r224465 - Adding a -Wunused-value warning for expressions with side effects used in an unevaluated expression context, such as sizeof(), or decltype(). Also adds a similar warning when the expression passed to typeid() *is* evaluated, since it is equally likely that the user would expect the expression operand to be unevaluated in that case.

[Aaron Ballman]

On Tue, Dec 23, 2014 at 11:01 AM, Joerg Sonnenberger
<joerg at britannica.bec.de> wrote:
> On Wed, Dec 17, 2014 at 09:57:17PM -0000, Aaron Ballman wrote:
>> Author: aaronballman
>> Date: Wed Dec 17 15:57:17 2014
>> New Revision: 224465
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=224465&view=rev
>> Log:
>> Adding a -Wunused-value warning for expressions with side effects used
>> in an unevaluated expression context, such as sizeof(), or decltype().
>
> I think in the case of sizeof, it is too aggressive. It triggered in
> NetBSD's mount on logic like the following:
>
>         char ** volatile argv;
>
>         argv = calloc(count, sizeof(*argv));
>
> because the volatile marker supposed makes the *argv have side effects.
> It is present in this case, because the function later on uses vfork and
> GCC complains about trashing local variables for a function that returns
> twice. setjmp would be slightly less obscure.

The original patch had volatile reads as not being side-effecting, but
Richard desired the current behavior. The specifications are pretty
clear in that reads of a volatile value *are* side-effecting, but I
originally believed as you did, the above code is pretty idiomatic.

> I think it should *not* trigger in this case for two important reasons:
>
> (1) The sizeof use is completely idiomatic.

Agreed. However, the dereference of a volatile value is still
side-effecting, and having a side-effecting operation appear in a
context where side effects are not evaluated is what this patch was
all about.

> (2) The only workaround for the warning introduces possible maintainance
> costs, as it would require duplicating the type of argv.

That strikes me as the bigger reason why the warning should be
silenced in this particular case -- it *is* idiomatic code, and the
way to silence the warning isn't particularly ideal.

> A C programmer should know and expect the memory access to not happen.
> I would say this is different from the case Aaron gave on IRC about
> sizeof(i++). That's a side effect most would expect to still happen.
> To keep the number of exceptions small, I propose the relax the warning
> to not trigger on dereference of volatile pointers.

I'm not opposed to this, but would like Richard to weigh in.

Thanks!

~Aaron