bug#61730: 30.0.50; Compiler warnings for delq and delete

["Mattias Engdegård" <mattias.engdegard@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 93 bytes --]

As promised, here is a first draft. The false positive rate seems indeed to be quite low.


[-- Attachment #2: warn-ignored-return-value.diff --]
[-- Type: application/octet-stream, Size: 4612 bytes --]

diff --git a/lisp/emacs-lisp/byte-run.el b/lisp/emacs-lisp/byte-run.el
index 9345665eea8..fd9913d1be8 100644
--- a/lisp/emacs-lisp/byte-run.el
+++ b/lisp/emacs-lisp/byte-run.el
@@ -650,11 +650,8 @@ with-suppressed-warnings
 `byte-compile-warnings' for a fuller explanation of the warning
 types.  The types that can be suppressed with this macro are
 `free-vars', `callargs', `redefine', `obsolete',
-`interactive-only', `lexical', `mapcar', `constants',
-`suspicious' and `empty-body'.
-
-For the `mapcar' case, only the `mapcar' function can be used in
-the symbol list."
+`interactive-only', `lexical', `ignored-return-value', `constants',
+`suspicious' and `empty-body'."
   ;; Note: during compilation, this definition is overridden by the one in
   ;; byte-compile-initial-macro-environment.
   (declare (debug (sexp body)) (indent 1))
diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
index 095468ad978..5f428b8e1f9 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -317,7 +317,9 @@ byte-compile-warnings
   lexical-dynamic
               lexically bound variable declared dynamic elsewhere
   make-local  calls to `make-variable-buffer-local' that may be incorrect.
-  mapcar      mapcar called for effect.
+  ignored-return-value
+              function called without using the return value where this
+              is likely to be a mistake
   not-unused  warning about using variables with symbol names starting with _.
   constants   let-binding of, or assignment to, constants/nonvariables.
   docstrings  docstrings that are too wide (longer than
@@ -330,7 +332,7 @@ byte-compile-warnings
   empty-body  body argument to a special form or macro is empty.
 
 If the list begins with `not', then the remaining elements specify warnings to
-suppress.  For example, (not mapcar) will suppress warnings about mapcar.
+suppress.  For example, (not free-vars) will suppress the `free-vars' warning.
 
 The t value means \"all non experimental warning types\", and
 excludes the types in `byte-compile--emacs-build-warning-types'.
@@ -3485,11 +3487,35 @@ byte-compile-normal-call
     (byte-compile-callargs-warn form))
   (if byte-compile-generate-call-tree
       (byte-compile-annotate-call-tree form))
-  (when (and byte-compile--for-effect (eq (car form) 'mapcar)
-             (byte-compile-warning-enabled-p 'mapcar 'mapcar))
+
+  ;; We warn about ignored return values for two categories of functions:
+  ;; * Ones like `mapcar' that are side-effect-free themselves but call a
+  ;;   user-supplied function.
+  ;; * Ones like `delq' that mutate a list but whose return argument is
+  ;;   essential to use in case the start of the list has changed.
+  (when (and byte-compile--for-effect
+             (memq (car form)
+                   ;; FIXME: Maybe we should use a property instead of
+                   ;; this list.
+                   '( mapcar mapcan mapconcat
+                      assoc assoc-default
+                      delq delete delete-dups delete-consecutive-dups
+                      nconc sort
+                      cl-delete cl-delete-if cl-delete-if-not
+                      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-sort))
+             (byte-compile-warning-enabled-p 'ignored-return-value (car form)))
     (byte-compile-warn-x
      (car form)
-     "`mapcar' called for effect; use `mapc' or `dolist' instead"))
+     "value from call to `%s' is unused%s"
+     (car form)
+     (cond ((eq (car form) 'mapcar) "; use `mapc' or `dolist' instead")
+           (t ""))))
   (byte-compile-push-constant (car form))
   (mapc 'byte-compile-form (cdr form))	; wasteful, but faster.
   (byte-compile-out 'byte-call (length (cdr form))))
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el b/test/lisp/emacs-lisp/bytecomp-tests.el
index 4b0a714e52d..6dce3087e40 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -1420,8 +1420,8 @@ bytecomp-test--with-suppressed-warnings
    '(defun zot ()
       (mapcar #'list '(1 2 3))
       nil)
-   '((mapcar mapcar))
-   "Warning: .mapcar. called for effect")
+   '((ignored-return-value mapcar))
+   "Warning: value from call to `mapcar' is unused; use `mapc' or `dolist' instead")
 
   (test-suppression
    '(defun zot ()