Package: emacs;
Reported by: Pip Cet <pipcet <at> protonmail.com>
Date: Sat, 7 Jun 2025 14:27:02 UTC
Severity: normal
Found in version 31.0.50
Message #11 received at 78716 <at> debbugs.gnu.org (full text, mbox):
From: Stefan Monnier <monnier <at> iro.umontreal.ca> To: Pip Cet <pipcet <at> protonmail.com> Cc: 78716 <at> debbugs.gnu.org Subject: Re: bug#78716: 31.0.50; no byte compiler warnings for ignored side-effect-free function calls Date: Sat, 28 Jun 2025 13:38:56 -0400
> We do not currently warn consistently when discarding the value of a
> function call to a side-effect-free function, even though the return
> value is usually important. It's not clear that we should, but I tried
> enabling such warnings and found ~5 questionable places in the code in
> lisp/.
> 1. We don't ever warn about side-effect-and-error-free functions like
> "not"; this seems to me to be strange behavior on the part of the byte
> compiler, but it's probably been that way for a long time.
There are several macros which rely on the byte-compiler silently
erasing such "side-effect-and-error-free" if it's not used.
There is a warning for those that can signal errors *because* we can't
safely erase them.
> For example, this "not" in an implicit progn, in vc/vc.el:
>
> (insert-string (format "Changes to %s since last lock:\n\n"
> file))
> (not (beep))
> (yes-or-no-p
> (concat "File has unlocked changes, "
> "claim lock retaining changes? ")))
>
> That's commit bbf97570e5037b2417c9dd6353e1b2a9afadda7c, from 1993, moved
> around in 2000 but essentially untouched. Has the byte compiler been
> silent on this situation for all this time?
Probably.
> Is it worth changing?
Often, it's hard for macros to know if a value will be used or not, so
if we want to change it, we need a convenient way for macros to let the
compiler erase those operations silently. It's not completely trivial
because we can't just wrap them like (FOO (not (beep))) since the
annotation should apply to the `not` operation but not to its argument
(the `beep` operation).
> 2. We also don't warn about function calls which we optimize away; this
> also seems problematic if the form we optimize away was present in the
> original source code, but it's easier to write macros which generate
> intermediate forms which can then be optimized, and I don't think we can
> currently tell whether we're looking at macro-expanded code or not.
Exactly.
> For example, my naive experiment produced false warnings when
>
> (pop list2)
>
> appeared in a context in which its return value was ignored, which is a
> common idiom and probably accounts for the majority of the warnings
> produced.
Similarly, `pcase` destructuring was changed to use `c[ad]r-safe` (even
when we just did a `consp` check) rather than `c[ad]r` to avoid
such warnings.
> (if (not m)
> ;; Go to dependencies, and search there.
> nil)
> m))))
>
> probably should generate a warning, because the if form seems completely
> mysterious to me.
At the same time: is it an actual problem?
Other than clean up the code, what would be the benefit?
The above code shape is the kind of thing that can easily be produced by
macros, so again we'd need some way for macros to avoid/silence the
warnings without too much effort.
I'm not sure it's worth the trouble here.
> My suggestion is to fix the first issue and complain about such forms,
> by doing something like this:
>
> diff --git a/lisp/emacs-lisp/bytecomp.el b/lisp/emacs-lisp/bytecomp.el
> index 5fa65ff71a6..4716e150faf 100644
> --- a/lisp/emacs-lisp/bytecomp.el
> +++ b/lisp/emacs-lisp/bytecomp.el
> @@ -3470,26 +3470,25 @@ byte-compile-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 optimizer,
> - ;; 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 (function-get (car form) 'important-return-value))
> - ;; Don't warn for arguments to `ignore'.
> - (not (eq byte-compile--for-effect 'for-effect-no-warn))
> - (bytecomp--actually-important-return-value-p form)
> - (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 "")))))))
> + (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))
> + (bytecomp--actually-important-return-value-p form)
> + (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 ""))))
> + (and sef (or (eq sef 'error-free)
> + byte-compile-delete-errors)
> + ;; This transform is normally done in the Lisp optimizer,
> + ;; so maybe we don't need to bother about it here?
> + (setq form (cons 'progn (cdr form)))
> + (setq handler #'byte-compile-progn))))
IIUC you're swapping the two tests such that an `important-return-value`
is not missed just because we optimized it away according to its
`error-free` annotation?
I can go along with that.
> The second class of warnings should not be enabled by default, but maybe
> it can become an 'extra warning level which we run once in a while?
Adding a notion of "level" for warnings that aren't super-important
might make sense, yes. I'd still welcome some way to silence those
warnings locally rather than via some global setting.
Stefan
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.