From 6157e3e4bc7e4e097e02c572379d1b1542e1d716 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Mattias=20Engdeg=C3=A5rd?= Date: Sun, 9 Apr 2023 15:57:31 +0200 Subject: [PATCH] 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. --- etc/NEWS | 15 +++++++++ lisp/emacs-lisp/bytecomp.el | 62 ++++++++++++++++++++++++++++++++++++- 2 files changed, 76 insertions(+), 1 deletion(-) diff --git a/etc/NEWS b/etc/NEWS index d20d9f65ac9..c61a9ec3c5f 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -443,6 +443,21 @@ simplified away. This warning can be suppressed using 'with-suppressed-warnings' with the warning name 'suspicious'. +--- +*** Warn about more ignored function return values. +The compiler now warns when the return value from certain functions is +ignored. Example: + + (progn (nreverse my-list) my-list) + +will elicit a warning because it is usually pointless to call +'nreverse' on a list without using the returned value. To silence the +warning, make use of the value in some way, such as assigning it to a +variable. You can also wrap the function call in '(ignore ...)'. + +This warning can be suppressed using 'with-suppressed-warnings' with +the warning name 'ignored-return-value'. + +++ ** New function 'file-user-uid'. 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." ;; so maybe we don't need to bother about it here? (setq form (cons 'progn (cdr form))) (setq handler #'byte-compile-progn)) - ((and (or sef (eq (car form) 'mapcar)) + ((and (or sef + (memq (car form) + ;; FIXME: Use a function property (declaration) + ;; instead of this list. + '( + ;; Functions that are side-effect-free + ;; except for the behaviour of + ;; functions passed as argument. + mapcar mapcan mapconcat + cl-mapcar cl-mapcan cl-maplist cl-map cl-mapcon + cl-reduce + assoc assoc-default plist-get plist-member + cl-assoc cl-assoc-if cl-assoc-if-not + cl-rassoc cl-rassoc-if cl-rassoc-if-not + cl-member cl-member-if cl-member-if-not + cl-adjoin + cl-mismatch cl-search + cl-find cl-find-if cl-find-if-not + cl-position cl-position-if cl-position-if-not + cl-count cl-count-if cl-count-if-not + cl-remove cl-remove-if cl-remove-if-not + cl-member cl-member-if cl-member-if-not + cl-remove-duplicates + cl-subst cl-subst-if cl-subst-if-not + cl-substitute cl-substitute-if + cl-substitute-if-not + cl-sublis + cl-union cl-intersection + cl-set-difference cl-set-exclusive-or + cl-subsetp + cl-every cl-some cl-notevery cl-notany + cl-tree-equal + + ;; Functions that mutate and return a list. + cl-delete-if cl-delete-if-not + ;; `delete-dups' and `delete-consecutive-dups' + ;; never delete the first element so it's + ;; safe to ignore their return value, but + ;; this isn't the case with + ;; `cl-delete-duplicates'. + cl-delete-duplicates + cl-nsubst cl-nsubst-if cl-nsubst-if-not + cl-nsubstitute cl-nsubstitute-if + cl-nsubstitute-if-not + cl-nunion cl-nintersection + cl-nset-difference cl-nset-exclusive-or + cl-nreconc cl-nsublis + cl-merge + ;; It's safe to ignore the value of `sort' + ;; and `nreverse' when used on arrays, + ;; but most calls pass lists. + nreverse + sort cl-sort cl-stable-sort + + ;; Adding the following functions yields many + ;; positives; evaluate how many of them are + ;; false first. + + ;;delq delete cl-delete + ;;nconc plist-put + ))) (byte-compile-warning-enabled-p 'ignored-return-value (car form))) (byte-compile-warn-x -- 2.39.2