From: Mattias Engdegård Date: Mon, 1 May 2023 13:55:32 +0000 (+0200) Subject: Add the function declaration and property `important-return-value` X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=7acb3f1c060872b1802e7548d991ca8a9f0fef01;p=emacs.git Add the function declaration and property `important-return-value` Now the declaration (declare (important-return-value t)) can be used to have the byte-compiler warn when the return value from a call is discarded (bug#61730). * lisp/emacs-lisp/bytecomp.el (byte-compile-form) (important-return-value-fns): Use the function property `important-return-value` instead of looking through a static list. * lisp/emacs-lisp/byte-run.el (byte-run--set-important-return-value) (defun-declarations-alist): New function declaration, setting the property of the same name. * lisp/emacs-lisp/cl-macs.el: * lisp/subr.el (assoc-default): Set the property. * doc/lispref/functions.texi (Declare Form): * doc/lispref/symbols.texi (Standard Properties): Document. * etc/NEWS: Announce. --- diff --git a/doc/lispref/functions.texi b/doc/lispref/functions.texi index 42441361fea..dc0d182d50d 100644 --- a/doc/lispref/functions.texi +++ b/doc/lispref/functions.texi @@ -2631,6 +2631,12 @@ so the byte compiler can ignore calls whose value is ignored. This is the same as the @code{side-effect-free} property of the function's symbol, @pxref{Standard Properties}. +@item (important-return-value @var{val}) +If @var{val} is non-@code{nil}, the byte compiler will warn about +calls to this function that do not use the returned value. This is the +same as the @code{important-return-value} property of the function's +symbol, @pxref{Standard Properties}. + @item (speed @var{n}) Specify the value of @code{native-comp-speed} in effect for native compilation of this function (@pxref{Native-Compilation Variables}). diff --git a/doc/lispref/symbols.texi b/doc/lispref/symbols.texi index c6a0408abd1..34db0caf3a8 100644 --- a/doc/lispref/symbols.texi +++ b/doc/lispref/symbols.texi @@ -643,6 +643,12 @@ ignore a call whose value is unused. If the property's value is calls. In addition to byte compiler optimizations, this property is also used for determining function safety (@pxref{Function Safety}). +@item important-return-value +@cindex @code{important-return-value} property +A non-@code{nil} value makes the byte compiler warn about code that +calls the named function without using its returned value. This is +useful for functions where doing so is likely to be a mistake. + @item undo-inhibit-region If non-@code{nil}, the named function prevents the @code{undo} operation from being restricted to the active region, if @code{undo} is invoked diff --git a/etc/NEWS b/etc/NEWS index 87d312596cd..b989f80f3c3 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -507,17 +507,26 @@ 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: +implicitly 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 ...)'. +'nreverse' on a list without using the returned value. -This warning can be suppressed using 'with-suppressed-warnings' with -the warning name 'ignored-return-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 ...)', or use 'with-suppressed-warnings' with the warning +name 'ignored-return-value'. + +The warning will only be issued for calls to functions declared +'important-return-value' or 'side-effect-free' (but not 'error-free'). + ++++ +** New function declaration and property 'important-return-value'. +The declaration '(important-return-value t)' sets the +'important-return-value' property which indicates that the function +return value should probably not be thrown away implicitly. +++ ** New function 'file-user-uid'. diff --git a/lisp/emacs-lisp/byte-run.el b/lisp/emacs-lisp/byte-run.el index fd9913d1be8..5b415c5e1f4 100644 --- a/lisp/emacs-lisp/byte-run.el +++ b/lisp/emacs-lisp/byte-run.el @@ -145,6 +145,11 @@ So far, FUNCTION can only be a symbol, not a lambda expression." (list 'function-put (list 'quote f) ''side-effect-free (list 'quote val)))) +(defalias 'byte-run--set-important-return-value + #'(lambda (f _args val) + (list 'function-put (list 'quote f) + ''important-return-value (list 'quote val)))) + (put 'compiler-macro 'edebug-declaration-spec '(&or symbolp ("lambda" &define lambda-list lambda-doc def-body))) @@ -226,6 +231,8 @@ This may shift errors from run-time to compile-time.") (list 'side-effect-free #'byte-run--set-side-effect-free "If non-nil, calls can be ignored if their value is unused. If `error-free', drop calls even if `byte-compile-delete-errors' is nil.") + (list 'important-return-value #'byte-run--set-important-return-value + "If non-nil, warn about calls not using the returned value.") (list 'compiler-macro #'byte-run--set-compiler-macro) (list 'doc-string #'byte-run--set-doc-string) (list 'indent #'byte-run--set-indent) diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el index c84c70971b3..6c804056ee7 100644 --- a/lisp/emacs-lisp/bytecomp.el +++ b/lisp/emacs-lisp/bytecomp.el @@ -3502,67 +3502,7 @@ 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 - (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 - ))) + ((and (or sef (function-get (car form) 'important-return-value)) ;; Don't warn for arguments to `ignore'. (not (eq byte-compile--for-effect 'for-effect-no-warn)) (byte-compile-warning-enabled-p @@ -3598,6 +3538,26 @@ lambda-expression." (byte-compile-discard)) (pop byte-compile-form-stack))) +(let ((important-return-value-fns + '( + ;; These functions are side-effect-free except for the + ;; behaviour of functions passed as argument. + mapcar mapcan mapconcat + assoc plist-get plist-member + + ;; It's safe to ignore the value of `sort' and `nreverse' + ;; when used on arrays, but most calls pass lists. + nreverse sort + + ;; Adding these functions causes many warnings; + ;; evaluate how many of them are false first. + ;;delq delete + ;;nconc plist-put + ))) + (dolist (fn important-return-value-fns) + (put fn 'important-return-value t))) + + (defun byte-compile-normal-call (form) (when (and (symbolp (car form)) (byte-compile-warning-enabled-p 'callargs (car form))) diff --git a/lisp/emacs-lisp/cl-macs.el b/lisp/emacs-lisp/cl-macs.el index 5382e0a0a52..04567917189 100644 --- a/lisp/emacs-lisp/cl-macs.el +++ b/lisp/emacs-lisp/cl-macs.el @@ -3657,6 +3657,46 @@ macro that returns its `&whole' argument." '(cl-list* cl-acons cl-equalp cl-random-state-p copy-tree)) +;;; Things whose return value should probably be used. +(mapc (lambda (x) (function-put x 'important-return-value t)) + '( + ;; Functions that are side-effect-free except for the + ;; behaviour of functions passed as argument. + cl-mapcar cl-mapcan cl-maplist cl-map cl-mapcon + cl-reduce + 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-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 + 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-merge + ;; It's safe to ignore the value of `cl-sort' and `cl-stable-sort' + ;; when used on arrays, but most calls pass lists. + cl-sort cl-stable-sort + )) + + ;;; Types and assertions. ;;;###autoload diff --git a/lisp/subr.el b/lisp/subr.el index 427014cedc3..1452a1117d3 100644 --- a/lisp/subr.el +++ b/lisp/subr.el @@ -862,6 +862,7 @@ If that is non-nil, the element matches; then `assoc-default' If no element matches, the value is nil. If TEST is omitted or nil, `equal' is used." + (declare (important-return-value t)) (let (found (tail alist) value) (while (and tail (not found)) (let ((elt (car tail)))