master aef996cd34f: Consolidate existing warnings about unused return values

[Mattias Engdegård]

branch: master
commit aef996cd34f421da6540cccb9cc3ac2153458e36
Author: Mattias Engdegård <mattiase@acm.org>
Commit: Mattias Engdegård <mattiase@acm.org>

    Consolidate existing warnings about unused return values
    
    Move the warning about unused return values from calls to
    side-effect-free functions from the source-level optimiser to the code
    generator, where it can be unified with the special-purpose warning
    about unused values from `mapcar`.  This change also cures spurious
    duplicate warnings about the same code, makes the warnings amenable to
    suppression through `with-suppressed-warnings`, and now warns about
    some unused values that weren't caught before.
    
    * lisp/emacs-lisp/byte-opt.el (byte-optimize-form-code-walker):
    Move warning away from here.
    * lisp/emacs-lisp/byte-run.el (with-suppressed-warnings):
    * lisp/emacs-lisp/bytecomp.el (byte-compile-warnings):
    Doc string updates.
    (byte-compile-form): Put the new warnings here.
    (byte-compile-normal-call): Move mapcar warning away from here.
    * lisp/emacs-lisp/bytecomp.el (byte-compile-ignore):
    Compile args to `ignore` for value to avoid unused-value warnings, and
    then discard the generated values immediately thereafter.  Mostly this
    does not affect the generated code but in rare cases it might result
    in slightly worse code.
    * test/lisp/emacs-lisp/bytecomp-tests.el
    (bytecomp-test--with-suppressed-warnings): Adapt test.
---
 lisp/emacs-lisp/byte-opt.el            |  8 +------
 lisp/emacs-lisp/byte-run.el            |  7 ++----
 lisp/emacs-lisp/bytecomp.el            | 39 +++++++++++++++++++++++++++-------
 test/lisp/emacs-lisp/bytecomp-tests.el |  4 ++--
 4 files changed, 36 insertions(+), 22 deletions(-)

diff --git a/lisp/emacs-lisp/byte-opt.el b/lisp/emacs-lisp/byte-opt.el
index 0891ec80beb..70317e2365d 100644
--- a/lisp/emacs-lisp/byte-opt.el
+++ b/lisp/emacs-lisp/byte-opt.el
@@ -506,13 +506,7 @@ for speeding up processing.")
       ((guard (when for-effect
                (if-let ((tmp (byte-opt--fget fn 'side-effect-free)))
                    (or byte-compile-delete-errors
-                       (eq tmp 'error-free)
-                       (progn
-                         (byte-compile-warn-x
-                           form
-                           "value returned from %s is unused"
-                          form)
-                         nil)))))
+                       (eq tmp 'error-free)))))
        (byte-compile-log "  %s called for effect; deleted" fn)
        (byte-optimize-form (cons 'progn (cdr form)) t))
 
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 @@ in `byte-compile-warning-types'; see the variable
 `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 a122e81ba3c..4a10ae29804 100644
--- a/lisp/emacs-lisp/bytecomp.el
+++ b/lisp/emacs-lisp/bytecomp.el
@@ -317,7 +317,9 @@ Elements of the list may be:
   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 @@ Elements of the list may be:
   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'.
@@ -3490,6 +3492,27 @@ lambda-expression."
             (byte-compile-report-error
              (format-message "`%s' defined after use in %S (missing `require' 
of a library file?)"
                      (car form) form)))
+
+        (when byte-compile--for-effect
+          (let ((sef (function-get (car form) 'side-effect-free)))
+            (cond
+             ((and sef (or (eq sef 'error-free)
+                           byte-compile-delete-errors))
+              ;; This transform is normally done in the Lisp optimiser,
+              ;; 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))
+                   (byte-compile-warning-enabled-p
+                    'ignored-return-value (car form)))
+              (byte-compile-warn-x
+               (car form)
+               "value from call to `%s' is unused%s"
+               (car form)
+               (cond ((eq (car form) 'mapcar)
+                      "; use `mapc' or `dolist' instead")
+                     (t "")))))))
+
         (if (and handler
                  ;; Make sure that function exists.
                  (and (functionp handler)
@@ -3523,11 +3546,7 @@ lambda-expression."
     (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))
-    (byte-compile-warn-x
-     (car form)
-     "`mapcar' called for effect; use `mapc' or `dolist' instead"))
+
   (byte-compile-push-constant (car form))
   (mapc 'byte-compile-form (cdr form)) ; wasteful, but faster.
   (byte-compile-out 'byte-call (length (cdr form))))
@@ -4367,7 +4386,11 @@ This function is never called when `lexical-binding' is 
nil."
 
 (defun byte-compile-ignore (form)
   (dolist (arg (cdr form))
-    (byte-compile-form arg t))
+    ;; Compile args for value (to avoid warnings about unused values),
+    ;; emit a discard after each, and trust the LAP peephole optimiser
+    ;; to annihilate useless ops.
+    (byte-compile-form arg)
+    (byte-compile-discard))
   (byte-compile-form nil))
 
 ;; Return the list of items in CONDITION-PARAM that match PRED-LIST.
diff --git a/test/lisp/emacs-lisp/bytecomp-tests.el 
b/test/lisp/emacs-lisp/bytecomp-tests.el
index 5bad1ce41a8..9ade47331df 100644
--- a/test/lisp/emacs-lisp/bytecomp-tests.el
+++ b/test/lisp/emacs-lisp/bytecomp-tests.el
@@ -1438,8 +1438,8 @@ literals (Bug#20852)."
    '(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 ()