rsmith 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);
----------------
Why "WarnOn"? Shouldn't this flag simply indicate whether the expression is a 
discarded-value expression?


================
Comment at: lib/Parse/ParseStmt.cpp:23
 #include "clang/Sema/Scope.h"
+#include "clang/Sema/ScopeInfo.h"
 #include "clang/Sema/TypoCorrection.h"
----------------
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?


================
Comment at: lib/Sema/SemaExprCXX.cpp:7785
 
-  if (DiscardedValue) {
+  if (WarnOnDiscardedValue) {
     // Top-level expressions default to 'id' when we're in a debugger.
----------------
It doesn't make sense for a WarnOn flag to control how we build the AST like 
this.


================
Comment at: lib/Sema/TreeTransform.h:3297
 
-      return getSema().ActOnExprStmt(E);
+      return getSema().ActOnExprStmt(E, SuppressWarnOnUnusedExpr == 0);
     }
----------------
What happens if the last statement in an expression statement contains a 
lambda? Will we consider all expression statements in it as not being 
discarded-value expressions? Alternate suggestion below.


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


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

https://reviews.llvm.org/D55955



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to