<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Jan 2, 2015 at 2:56 PM, Aaron Ballman <span dir="ltr"><<a href="mailto:aaron@aaronballman.com" target="_blank">aaron@aaronballman.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class="">On Tue, Dec 23, 2014 at 11:01 AM, Joerg Sonnenberger<br>
<<a href="mailto:joerg@britannica.bec.de">joerg@britannica.bec.de</a>> wrote:<br>
> On Wed, Dec 17, 2014 at 09:57:17PM -0000, Aaron Ballman wrote:<br>
>> Author: aaronballman<br>
>> Date: Wed Dec 17 15:57:17 2014<br>
>> New Revision: 224465<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=224465&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=224465&view=rev</a><br>
>> Log:<br>
>> Adding a -Wunused-value warning for expressions with side effects used<br>
>> in an unevaluated expression context, such as sizeof(), or decltype().<br>
><br>
> I think in the case of sizeof, it is too aggressive. It triggered in<br>
> NetBSD's mount on logic like the following:<br>
><br>
>         char ** volatile argv;<br>
><br>
>         argv = calloc(count, sizeof(*argv));<br>
><br>
> because the volatile marker supposed makes the *argv have side effects.<br>
> It is present in this case, because the function later on uses vfork and<br>
> GCC complains about trashing local variables for a function that returns<br>
> twice. setjmp would be slightly less obscure.<br>
<br>
</span>The original patch had volatile reads as not being side-effecting, but<br>
Richard desired the current behavior. The specifications are pretty<br>
clear in that reads of a volatile value *are* side-effecting, but I<br>
originally believed as you did, the above code is pretty idiomatic.<br>
<span class=""><br>
> I think it should *not* trigger in this case for two important reasons:<br>
><br>
> (1) The sizeof use is completely idiomatic.<br>
<br>
</span>Agreed. However, the dereference of a volatile value is still<br>
side-effecting, and having a side-effecting operation appear in a<br>
context where side effects are not evaluated is what this patch was<br>
all about.<br>
<span class=""><br>
> (2) The only workaround for the warning introduces possible maintainance<br>
> costs, as it would require duplicating the type of argv.<br>
<br>
</span>That strikes me as the bigger reason why the warning should be<br>
silenced in this particular case -- it *is* idiomatic code, and the<br>
way to silence the warning isn't particularly ideal.<br>
<span class=""><br>
> A C programmer should know and expect the memory access to not happen.<br>
> I would say this is different from the case Aaron gave on IRC about<br>
> sizeof(i++). That's a side effect most would expect to still happen.<br>
> To keep the number of exceptions small, I propose the relax the warning<br>
> to not trigger on dereference of volatile pointers.<br>
<br>
</span>I'm not opposed to this, but would like Richard to weigh in.</blockquote><div><br></div><div>Well, my position on this was "let's try warning on it and see what happens" =)</div><div><br></div><div>But I don't buy the argument here: a C programmer should know and expect side-effects inside sizeof to not happen, whether they're due to 'volatile' or an increment  (this warning is *supposed* to have false-positives if the expression always has a side-effect that the programmer doesn't expect to actually happen...).</div><div><br></div><div>It seems like the issue is that 'volatile' is sometimes used for reasons other than to ensure a side-effect (the example code above isn't a great case for this, where it's apparently been used as an attempt to provide atomicity/thread safety across a vfork, but there are other reasonable cases where it's used as a compiler optimization barrier), so we shouldn't assume a volatile load or store is *always* a side-effect. Also, in the general case of an access through a volatile-qualified type, we don't actually know whether the object itself is volatile-qualified, which affects whether there is actually a side-effect. On that basis, I think we should unconditionally treat volatile access as the maybe-side-effect case rather than the always-side-effect case.</div></div></div></div>