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.
This commit is contained in:
@@ -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}).
|
||||
|
||||
@@ -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
|
||||
|
||||
21
etc/NEWS
21
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'.
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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)))
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)))
|
||||
|
||||
Reference in New Issue
Block a user