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 8 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.