master 7acb3f1c060 1/2: Add the function declaration and property `important-return-value`

[Mattias Engdegård]

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

    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.
---
 doc/lispref/functions.texi  |  6 ++++
 doc/lispref/symbols.texi    |  6 ++++
 etc/NEWS                    | 21 ++++++++----
 lisp/emacs-lisp/byte-run.el |  7 ++++
 lisp/emacs-lisp/bytecomp.el | 82 ++++++++++++---------------------------------
 lisp/emacs-lisp/cl-macs.el  | 40 ++++++++++++++++++++++
 lisp/subr.el                |  1 +
 7 files changed, 96 insertions(+), 67 deletions(-)

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)))