On Tue, Dec 23, 2014 at 11:01 AM, Joerg Sonnenberger <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
