<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Nov 25, 2014 at 6:51 AM, 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">The attached patch adds a -Wunused-value warning when an expression<br>
with side effects is used in an unevaluated expression context, such<br>
as sizeof(), or decltype(). It mostly reuses the logic from<br>
Expr::HasSideEffects, except with a flag that treats certain<br>
expressions as not having side effects -- mostly<br>
instantiation-dependent contexts, and function call-like operations.<br>
<br>
int f();<br>
int j = 0;<br>
<br>
(void)sizeof(f()); // Ok<br>
(void)sizeof(++j); // Warn<br>
(void)sizeof(int[++j]); // Ok<br>
<br>
I've added support for: sizeof, typeid, _Generic, _Alignof, noexcept,<br>
and decltype.<br></blockquote></div><br></div><div class="gmail_extra">It is a very common idiom to use arbitrary expressions in sizeof, decltype, or noexcept expressions; in the former two cases, for SFINAE, and in the last case to compute an exception specification for an arbitrary function. In all these cases, expressions with side-effects seem completely reasonable. How does your patch behave in those cases?</div><div class="gmail_extra"><br></div><div class="gmail_extra">Does this actually catch bugs in practice?</div><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_extra">+  /// call, volatile variable read, or throwing an exception. If ForUnevalDiag</div><div class="gmail_extra">+  /// is true, this call treats certain expressions as not having side effects</div><div class="gmail_extra">+  /// when they otherwise would (such as function call-like expressions, where</div><div class="gmail_extra">+  /// the intent isn't on the side effects of the call, but the signature; or</div><div class="gmail_extra">+  /// instantiation-dependent expressions).</div><div class="gmail_extra">+  bool HasSideEffects(const ASTContext &Ctx, bool ForUnevalDiag = false) const;</div><div class="gmail_extra"><br></div><div class="gmail_extra">This new flag seems very special-case. I think what you're really trying to express here is what value the function should return if it doesn't know. (That is, it's choosing between determining "might possibly have side effects" and "definitely has side effects".)</div></div></div>