bug#81493: 30.2; Incorrect byte-compiler warning about unused value

"Drew Adams" <[email protected]> writes:

> And I think I understand what you say about the byte-compiler
> optimizing away `(when TRUE ...)' to just `...' for this use
> of the macro.  It makes sense for the compiler to perform that
> (minor) optimization.
>
> But it doesn't make sense from a user point of view, I think.
>
> It's normal that the macro code should only call `message',
> passing the value of SINGULAR (after formatting) to `concat',
> when SINGULAR isn't nil (otherwise the result of `message'
> would just emit `s' for format's `%s' instead of something
> like `files').
>
> So this is a case, I think, where the byte-compiler warning is
> bound to confuse more than help - anyone, regardless of how
> the warning might be worded.
>
> To understand the warning you have to (a) understand what the
> compiler did (removed an `when' test in that macro's code,
> (b) why it did that, and that it was right/OK to do it (it
> wasn't needed in this case), and (c) why its doing that makes
> no difference to the behavior/logic.
>
> IOW, the code shouldn't be changed - it's correct as is, and
> it should do what it does.  And the compiler does what it can
> and should do, for an optimization (very, very, VERY minor,
> in this case).
>
> But ideally there would be NO warning in this case, as it
> doesn't help in any way.  It can only confuse, and waste a
> user's time trying to figure out what's being "warned" about.
>
> This is not what _warnings_ should be for, IMO.  Byte-compiler
> warnings should be about code that's likely to be incorrect.
>
> Maybe there's no easy way for the compiler to be able to tell
> the difference in a case like this between code that's likely
> to be incorrect or nonkosher and code that's not problematic.
> But ideally it would, and ideally it wouldn't warn about
> anything here.
>
> Please let me know, if you think I'm missing something.

I anticipated your answer ;-)

Ok.  I agree that the thing that the byte compiler finds is not trivial
to locate in this case.  But it's surely possible.

OTOH I think the warning is founded: if you look at the expansion, that
code is hard to read because it is confusing to do a calculation and
throw away the result (confusing for the reader of such code).  This is
a potential mistake to warn about.

And it is a programming mistake, even if it doesn't result in an error
or serious problem in this case:
you evaluate one argument of the macro four different times.  The
compiler warning is a hint to fix that (by computing the result once and
binding it to a local variable, for example).

This is one of the issues documented in

  (info "(elisp) Argument Evaluation")

I think it is valuable to get warnings for such gotchas... the above
chapter contains the most evil gotchas in whole Elisp.  Don't you think
it would be better to fix your macro definition?

If you don't like this warning nonetheless, and still want to use this
style, binding byte-optimize to nil should turn these optimizations off,
along with the warnings.


Michael.