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.

diff --git a/etc/NEWS b/etc/NEWS
index d20d9f65ac9daecab651dacb9fdb65208e4acb3c..c61a9ec3c5f63a00620d9455f167fcb21fa9b216 100644 (file)
--- 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 4a10ae298044f5fed12776f9cd0998aa02281c09..1b28fcd509396993b78525f19cd580511d0f68a2 100644 (file)
--- 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