diff options
| author | Mattias EngdegÄrd | 2023-04-09 15:57:31 +0200 |
|---|---|---|
| committer | Mattias EngdegÄrd | 2023-04-09 18:23:05 +0200 |
| commit | 6157e3e4bc7e4e097e02c572379d1b1542e1d716 (patch) | |
| tree | eccda38427b4e8e7d2a42f2211f3f59d2a9331d0 | |
| parent | 4f0849a9e6d29e25d23e061bd81bacce9468856d (diff) | |
| download | emacs-6157e3e4bc7e4e097e02c572379d1b1542e1d716.tar.gz emacs-6157e3e4bc7e4e097e02c572379d1b1542e1d716.zip | |
Extend ignored-return-value warning to more functions (bug#61730)
Warn when the return value of certain functions is unused. Previously
this was only done for side-effect-free functions, and for `mapcar`.
These are functions where the return value is important for correct
usage or where ignoring it is likely to indicate a mistake. The exact
set of functions is tentative and will be modified as we gain a better
understanding of which ones to include.
The current set comprises higher order functions such as `mapcar`
which are not primarily called for the effects of their function
arguments, and list-mutating functions like `nreverse` whose return
value is essential.
* lisp/emacs-lisp/bytecomp.el (byte-compile-form): Add list of
functions to warn about when their value is ignored.
* etc/NEWS: Announce.
| -rw-r--r-- | etc/NEWS | 15 | ||||
| -rw-r--r-- | lisp/emacs-lisp/bytecomp.el | 62 |
2 files changed, 76 insertions, 1 deletions
| @@ -443,6 +443,21 @@ simplified away. | |||
| 443 | This warning can be suppressed using 'with-suppressed-warnings' with | 443 | This warning can be suppressed using 'with-suppressed-warnings' with |
| 444 | the warning name 'suspicious'. | 444 | the warning name 'suspicious'. |
| 445 | 445 | ||
| 446 | --- | ||
| 447 | *** Warn about more ignored function return values. | ||
| 448 | The compiler now warns when the return value from certain functions is | ||
| 449 | ignored. Example: | ||
| 450 | |||
| 451 | (progn (nreverse my-list) my-list) | ||
| 452 | |||
| 453 | will elicit a warning because it is usually pointless to call | ||
| 454 | 'nreverse' on a list without using the returned value. To silence the | ||
| 455 | warning, make use of the value in some way, such as assigning it to a | ||
| 456 | variable. You can also wrap the function call in '(ignore ...)'. | ||
| 457 | |||
| 458 | This warning can be suppressed using 'with-suppressed-warnings' with | ||
| 459 | the warning name 'ignored-return-value'. | ||
| 460 | |||
| 446 | +++ | 461 | +++ |
| 447 | ** New function 'file-user-uid'. | 462 | ** New function 'file-user-uid'. |
| 448 | This function is like 'user-uid', but is aware of file name handlers, | 463 | This function is like 'user-uid', but is aware of file name handlers, |
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index 4a10ae29804..1b28fcd5093 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el | |||
| @@ -3502,7 +3502,67 @@ lambda-expression." | |||
| 3502 | ;; so maybe we don't need to bother about it here? | 3502 | ;; so maybe we don't need to bother about it here? |
| 3503 | (setq form (cons 'progn (cdr form))) | 3503 | (setq form (cons 'progn (cdr form))) |
| 3504 | (setq handler #'byte-compile-progn)) | 3504 | (setq handler #'byte-compile-progn)) |
| 3505 | ((and (or sef (eq (car form) 'mapcar)) | 3505 | ((and (or sef |
| 3506 | (memq (car form) | ||
| 3507 | ;; FIXME: Use a function property (declaration) | ||
| 3508 | ;; instead of this list. | ||
| 3509 | '( | ||
| 3510 | ;; Functions that are side-effect-free | ||
| 3511 | ;; except for the behaviour of | ||
| 3512 | ;; functions passed as argument. | ||
| 3513 | mapcar mapcan mapconcat | ||
| 3514 | cl-mapcar cl-mapcan cl-maplist cl-map cl-mapcon | ||
| 3515 | cl-reduce | ||
| 3516 | assoc assoc-default plist-get plist-member | ||
| 3517 | cl-assoc cl-assoc-if cl-assoc-if-not | ||
| 3518 | cl-rassoc cl-rassoc-if cl-rassoc-if-not | ||
| 3519 | cl-member cl-member-if cl-member-if-not | ||
| 3520 | cl-adjoin | ||
| 3521 | cl-mismatch cl-search | ||
| 3522 | cl-find cl-find-if cl-find-if-not | ||
| 3523 | cl-position cl-position-if cl-position-if-not | ||
| 3524 | cl-count cl-count-if cl-count-if-not | ||
| 3525 | cl-remove cl-remove-if cl-remove-if-not | ||
| 3526 | cl-member cl-member-if cl-member-if-not | ||
| 3527 | cl-remove-duplicates | ||
| 3528 | cl-subst cl-subst-if cl-subst-if-not | ||
| 3529 | cl-substitute cl-substitute-if | ||
| 3530 | cl-substitute-if-not | ||
| 3531 | cl-sublis | ||
| 3532 | cl-union cl-intersection | ||
| 3533 | cl-set-difference cl-set-exclusive-or | ||
| 3534 | cl-subsetp | ||
| 3535 | cl-every cl-some cl-notevery cl-notany | ||
| 3536 | cl-tree-equal | ||
| 3537 | |||
| 3538 | ;; Functions that mutate and return a list. | ||
| 3539 | cl-delete-if cl-delete-if-not | ||
| 3540 | ;; `delete-dups' and `delete-consecutive-dups' | ||
| 3541 | ;; never delete the first element so it's | ||
| 3542 | ;; safe to ignore their return value, but | ||
| 3543 | ;; this isn't the case with | ||
| 3544 | ;; `cl-delete-duplicates'. | ||
| 3545 | cl-delete-duplicates | ||
| 3546 | cl-nsubst cl-nsubst-if cl-nsubst-if-not | ||
| 3547 | cl-nsubstitute cl-nsubstitute-if | ||
| 3548 | cl-nsubstitute-if-not | ||
| 3549 | cl-nunion cl-nintersection | ||
| 3550 | cl-nset-difference cl-nset-exclusive-or | ||
| 3551 | cl-nreconc cl-nsublis | ||
| 3552 | cl-merge | ||
| 3553 | ;; It's safe to ignore the value of `sort' | ||
| 3554 | ;; and `nreverse' when used on arrays, | ||
| 3555 | ;; but most calls pass lists. | ||
| 3556 | nreverse | ||
| 3557 | sort cl-sort cl-stable-sort | ||
| 3558 | |||
| 3559 | ;; Adding the following functions yields many | ||
| 3560 | ;; positives; evaluate how many of them are | ||
| 3561 | ;; false first. | ||
| 3562 | |||
| 3563 | ;;delq delete cl-delete | ||
| 3564 | ;;nconc plist-put | ||
| 3565 | ))) | ||
| 3506 | (byte-compile-warning-enabled-p | 3566 | (byte-compile-warning-enabled-p |
| 3507 | 'ignored-return-value (car form))) | 3567 | 'ignored-return-value (car form))) |
| 3508 | (byte-compile-warn-x | 3568 | (byte-compile-warn-x |