GNU bug report logs -
#78704
[PATCH] Use `seq-do' instead of `seq-map' for side-effects
Previous Next
Full log
Message #17 received at 78704 <at> debbugs.gnu.org (full text, mbox):
"Zach Shaftel" <zach <at> shaf.tel> writes:
> Pip Cet <pipcet <at> protonmail.com> writes:
>
>> But I'm not sure adding important-return-value to functions which aren't
>> usually side-effect-free is a good idea given the warnings it currently
>> produces.
>
> my understanding was that important-return-value is meant to instruct
> the compiler to warn about discarded values, without also allowing it to
> optimize away those calls the way the side-effect-free property does
Usually, the byte compiler only optimizes away calls to functions whose
side-effect-free property is 'error-free, not those for which it is t.
(Bug#63288 was about unexpected failure to do so after evaluating
eieio-core.el, and the same code is causing trouble in bug#78685, so I'm
a bit skeptical that such conditional optimizations are worth it.)
> (for example, `mapcan' is declared important-return-value). so
> side-effect-free implies important-return-value. is that not the
> intended meaning?
I believe it was, but it seems we never complained about discarding, for
example, (not (beep)), even though the "not" is superfluous. See
bug#78716 for what happens when we enable such warnings (some false
positives, 5 places in the code base that look like potential bugs; and
there are places like x-dnd-handle-unsupported-drop which I cannot make
sense of at all).
>> The main reason is that it's not a helpful warning unless we tell the
>> user which function to use instead: mapc is a special case which is
>> handled by an explicit message, but without a "use `seq-do' instead"
>> in the message, fixing the warning requires looking up docstrings to
>> find the right alternative, which might not exist.
>
> very true. maybe important-return-value could accept a string argument
> to specify a helpful message? this could also be useful for functions
I think that'd be a good idea.
> like `plist-put', which are side-effect-full but whose return value
> should still be captured; we could notify the user that
> (plist-put plist :prop val) should be wrapped with `setq'.
Note that while it's traditional (and documented as a requirement) to
wrap plist-put in setq, Emacs always was very forgiving and modified any
non-empty plist passed to plist-put in a way that ensured the original
cons cell would be returned. At least on the C side of things, we rely
on this behavior.
plist-put (along with nconc, delq, and delete) is commented out in the
list of important-return-value functions in bytecomp.el, FWIW.
Uncommenting them produces lots of warnings.
>> Some other languages have chosen a different approach and provide a way
>> for functions to know, at compile time or run time, whether their return
>> value is used in a particular call.
>
> interesting, i wasn't aware of that approach. where could i see some
> examples?
Perl 5's wantarray, and the way GCC (incorrectly) rewrites printf
("%s\n", x) to puts (x) in void context. I'm surprised I haven't been
able to find more, to be honest.
>> Maybe we should do that instead? As an analogy, the byte compiler
>> won't complain about (equal x 3), but will silently replace it by (eq
>> x 3), so it's not like we always warn the user about code which can be
>> optimized in a similar fashion.
>
> that seems appropriate to me. (equal x 3) is still a valid usage of the
> `equal' function, and will do what the user intended.
The same is true of (mapcar #'message strings) and discarding the
result. It's perfectly valid and does what the user wants, it's just
inefficient. However, that's an implementation detail.
> on the other hand, we do complain about (eq x '(t)) in
> bytecomp--warn-dodgy-eq-arg, because that almost certainly will not do
> what the user intended.
I think that's a different class of warning altogether, to be honest.
Pip
This bug report was last modified 59 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.