[PATCH] D55955: Properly diagnose [[nodiscard]] on the body of a range-based for loop

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked 3 inline comments as done.
aaron.ballman added inline comments.


================
Comment at: include/clang/Sema/Sema.h:5337
   ExprResult ActOnFinishFullExpr(Expr *Expr, SourceLocation CC,
-                                 bool DiscardedValue = false,
+                                 bool WarnOnDiscardedValue = false,
                                  bool IsConstexpr = false);
----------------
rsmith wrote:
> Why "WarnOn"? Shouldn't this flag simply indicate whether the expression is a discarded-value expression?
It probably can; but then it feels like the logic is backwards from the suggested changes as I understood them. If it's a discarded value expression, then the value being unused should *not* be diagnosed because the expression only exists for its side effects (not its value computations), correct?


================
Comment at: lib/Parse/ParseStmt.cpp:23
 #include "clang/Sema/Scope.h"
+#include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/TypoCorrection.h"
----------------
rsmith wrote:
> The parser shouldn't be looking inside Sema's data structures. Can you add a method to the Sema interface for the query you make below instead?
Can do.


================
Comment at: lib/Sema/TreeTransform.h:6529
+      --SuppressWarnOnUnusedExpr;
+    
     if (Result.isInvalid()) {
----------------
rsmith wrote:
> Instead of this counter mechanism (which doesn't seem reliable), can you just pass a flag to TransformStmt?
I'll give it a shot, it seems plausible.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55955/new/

https://reviews.llvm.org/D55955