[PATCH] Warning on unevaluated expression contexts with side effects

[Richard Smith]

On Tue, Nov 25, 2014 at 6:51 AM, Aaron Ballman <aaron at aaronballman.com>
wrote:

> The attached patch adds a -Wunused-value warning when an expression
> with side effects is used in an unevaluated expression context, such
> as sizeof(), or decltype(). It mostly reuses the logic from
> Expr::HasSideEffects, except with a flag that treats certain
> expressions as not having side effects -- mostly
> instantiation-dependent contexts, and function call-like operations.
>
> int f();
> int j = 0;
>
> (void)sizeof(f()); // Ok
> (void)sizeof(++j); // Warn
> (void)sizeof(int[++j]); // Ok
>
> I've added support for: sizeof, typeid, _Generic, _Alignof, noexcept,
> and decltype.
>

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?

Does this actually catch bugs in practice?

+  /// call, volatile variable read, or throwing an exception. If
ForUnevalDiag
+  /// is true, this call treats certain expressions as not having side
effects
+  /// when they otherwise would (such as function call-like expressions,
where
+  /// the intent isn't on the side effects of the call, but the signature;
or
+  /// instantiation-dependent expressions).
+  bool HasSideEffects(const ASTContext &Ctx, bool ForUnevalDiag = false)
const;

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".)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141126/80b9641e/attachment.html>