From: Mattias Engdegård Date: Sat, 8 Apr 2023 17:17:17 +0000 (+0200) Subject: Consolidate existing warnings about unused return values X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=aef996cd34f421da6540cccb9cc3ac2153458e36;p=emacs.git 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. --- 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 ()