Fix a bug/missing-feature Ted noticed: the 'unused' warning should not

warn about the last stmt in a stmtexpr, f.e. there should be no warning for:

int maxval_stmt_expr(int x, int y) {
  return ({int _a = x, _b = y; _a > _b ? _a : _b; });
}

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@41655 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/Parse/ParseExpr.cpp b/Parse/ParseExpr.cpp
index 51ad217e31292502866c370e55307e2504e85123..c2b54828556c1d481ecc9f83b4689fda55807f0c 100644 (file)
--- a/Parse/ParseExpr.cpp
+++ b/Parse/ParseExpr.cpp
@@ -928,10 +928,9 @@ Parser::ExprResult Parser::ParseParenExpression(ParenParseOption &ExprType,
   ExprResult Result(true);
   CastTy = 0;
   
-  if (ExprType >= CompoundStmt && Tok.getKind() == tok::l_brace &&
-      !getLang().NoExtensions) {
+  if (ExprType >= CompoundStmt && Tok.getKind() == tok::l_brace) {
     Diag(Tok, diag::ext_gnu_statement_expr);
-    Parser::StmtResult Stmt = ParseCompoundStatement();
+    Parser::StmtResult Stmt = ParseCompoundStatement(true);
     ExprType = CompoundStmt;
     
     // If the substmt parsed correctly, build the AST node.

diff --git a/Parse/ParseStmt.cpp b/Parse/ParseStmt.cpp
index 80f7f0329e10a5a127b61a1722c92167afa9e973..44caaf8c916a9aeaa47fad016a533c73c331f2cd 100644 (file)
--- a/Parse/ParseStmt.cpp
+++ b/Parse/ParseStmt.cpp
@@ -366,7 +366,7 @@ Parser::StmtResult Parser::ParseDefaultStatement() {
 /// [OMP]   barrier-directive
 /// [OMP]   flush-directive
 ///
-Parser::StmtResult Parser::ParseCompoundStatement() {
+Parser::StmtResult Parser::ParseCompoundStatement(bool isStmtExpr) {
   assert(Tok.getKind() == tok::l_brace && "Not a compount stmt!");
   
   // Enter a scope to hold everything within the compound stmt.  Compound
@@ -374,7 +374,7 @@ Parser::StmtResult Parser::ParseCompoundStatement() {
   EnterScope(Scope::DeclScope);
 
   // Parse the statements in the body.
-  StmtResult Body = ParseCompoundStatementBody();
+  StmtResult Body = ParseCompoundStatementBody(isStmtExpr);
 
   ExitScope();
   return Body;
@@ -385,7 +385,7 @@ Parser::StmtResult Parser::ParseCompoundStatement() {
 /// ParseCompoundStmt action.  This expects the '{' to be the current token, and
 /// consume the '}' at the end of the block.  It does not manipulate the scope
 /// stack.
-Parser::StmtResult Parser::ParseCompoundStatementBody() {
+Parser::StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) {
   SourceLocation LBraceLoc = ConsumeBrace();  // eat the '{'.
 
   // TODO: "__label__ X, Y, Z;" is the GNU "Local Label" extension.  These are
@@ -442,7 +442,7 @@ Parser::StmtResult Parser::ParseCompoundStatementBody() {
   
   SourceLocation RBraceLoc = ConsumeBrace();
   return Actions.ParseCompoundStmt(LBraceLoc, RBraceLoc,
-                                   &Stmts[0], Stmts.size());
+                                   &Stmts[0], Stmts.size(), isStmtExpr);
 }
 
 /// ParseIfStatement

diff --git a/Sema/Sema.h b/Sema/Sema.h
index a968dba88ad887d2e3ab60d9acd59e4c8731ea07..78b88b78a06b3b19dcd61bd5ab9c39143f503c3a 100644 (file)
--- a/Sema/Sema.h
+++ b/Sema/Sema.h
@@ -202,7 +202,8 @@ public:
   
   virtual StmtResult ParseNullStmt(SourceLocation SemiLoc);
   virtual StmtResult ParseCompoundStmt(SourceLocation L, SourceLocation R,
-                                       StmtTy **Elts, unsigned NumElts);
+                                       StmtTy **Elts, unsigned NumElts,
+                                       bool isStmtExpr);
   virtual StmtResult ParseDeclStmt(DeclTy *Decl);
   virtual StmtResult ParseCaseStmt(SourceLocation CaseLoc, ExprTy *LHSVal,
                                    SourceLocation DotDotDotLoc, ExprTy *RHSVal,

diff --git a/Sema/SemaStmt.cpp b/Sema/SemaStmt.cpp
index 300388cf33ae13b876acce95ad6211ec87689461..bec7f892dfb5585706ad5295eb0cb2a20209700c 100644 (file)
--- a/Sema/SemaStmt.cpp
+++ b/Sema/SemaStmt.cpp
@@ -21,29 +21,9 @@
 #include "clang/Lex/IdentifierTable.h"
 using namespace clang;
 
-/// DiagnoseDeadExpr - The specified expression is side-effect free and
-/// evaluated in a context where the result is unused.  Emit a diagnostic to
-/// warn about this if appropriate.
-static void DiagnoseDeadExpr(Expr *E, Sema &S) {
-  if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(E))
-    S.Diag(BO->getOperatorLoc(), diag::warn_unused_expr,
-           BO->getLHS()->getSourceRange(), BO->getRHS()->getSourceRange());
-  else if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(E))
-    S.Diag(UO->getOperatorLoc(), diag::warn_unused_expr,
-           UO->getSubExpr()->getSourceRange());
-  else 
-    S.Diag(E->getExprLoc(), diag::warn_unused_expr, E->getSourceRange());
-}
-
 Sema::StmtResult Sema::ParseExprStmt(ExprTy *expr) {
   Expr *E = static_cast<Expr*>(expr);
   assert(E && "ParseExprStmt(): missing expression");
-  
-  // Exprs are statements, so there is no need to do a conversion here. However,
-  // diagnose some potentially bad code.
-  if (!E->hasLocalSideEffect() && !E->getType()->isVoidType())
-    DiagnoseDeadExpr(E, *this);
-  
   return E;
 }
 
@@ -61,7 +41,7 @@ Sema::StmtResult Sema::ParseDeclStmt(DeclTy *decl) {
 
 Action::StmtResult 
 Sema::ParseCompoundStmt(SourceLocation L, SourceLocation R,
-                        StmtTy **elts, unsigned NumElts) {
+                        StmtTy **elts, unsigned NumElts, bool isStmtExpr) {
   Stmt **Elts = reinterpret_cast<Stmt**>(elts);
   // If we're in C89 mode, check that we don't have any decls after stmts.  If
   // so, emit an extension diagnostic.
@@ -82,6 +62,32 @@ Sema::ParseCompoundStmt(SourceLocation L, SourceLocation R,
     }
   }
   
+  // Warn about unused expressions in statements.
+  for (unsigned i = 0; i != NumElts; ++i) {
+    Expr *E = dyn_cast<Expr>(Elts[i]);
+    if (!E) continue;
+    
+    // Warn about expressions with unused results.
+    if (E->hasLocalSideEffect() || E->getType()->isVoidType())
+      continue;
+    
+    // The last expr in a stmt expr really is used.
+    if (isStmtExpr && i == NumElts-1)
+      continue;
+    
+    /// DiagnoseDeadExpr - This expression is side-effect free and evaluated in
+    /// a context where the result is unused.  Emit a diagnostic to warn about
+    /// this.
+    if (const BinaryOperator *BO = dyn_cast<BinaryOperator>(E))
+      Diag(BO->getOperatorLoc(), diag::warn_unused_expr,
+           BO->getLHS()->getSourceRange(), BO->getRHS()->getSourceRange());
+    else if (const UnaryOperator *UO = dyn_cast<UnaryOperator>(E))
+      Diag(UO->getOperatorLoc(), diag::warn_unused_expr,
+           UO->getSubExpr()->getSourceRange());
+    else 
+      Diag(E->getExprLoc(), diag::warn_unused_expr, E->getSourceRange());
+  }
+  
   return new CompoundStmt(Elts, NumElts);
 }
 

diff --git a/clang.xcodeproj/project.pbxproj b/clang.xcodeproj/project.pbxproj
index 4beeaef7a6ba247c5c367d55649c8b312942ed37..ceddc1fe51371dbe08b56dff9d93b2afa896c564 100644 (file)
--- a/clang.xcodeproj/project.pbxproj
+++ b/clang.xcodeproj/project.pbxproj
@@ -637,7 +637,6 @@
                08FB7793FE84155DC02AAC07 /* Project object */ = {
                        isa = PBXProject;
                        buildConfigurationList = 1DEB923508733DC60010E9CD /* Build configuration list for PBXProject "clang" */;
-                       compatibilityVersion = "Xcode 2.4";
                        hasScannedForEncodings = 1;
                        mainGroup = 08FB7794FE84155DC02AAC07 /* clang */;
                        projectDirPath = "";

diff --git a/include/clang/Parse/Action.h b/include/clang/Parse/Action.h
index bc23b8121ee765c5e45080edf8810ecf1e10b74f..ec9a4d0dd2612ff9988cf3067ba02064a60bfea3 100644 (file)
--- a/include/clang/Parse/Action.h
+++ b/include/clang/Parse/Action.h
@@ -191,7 +191,8 @@ public:
   }
   
   virtual StmtResult ParseCompoundStmt(SourceLocation L, SourceLocation R,
-                                       StmtTy **Elts, unsigned NumElts) {
+                                       StmtTy **Elts, unsigned NumElts,
+                                       bool isStmtExpr) {
     return 0;
   }
   virtual StmtResult ParseDeclStmt(DeclTy *Decl) {

diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h
index 65d1ff6431e864bd1198176f417b46b9e90e918d..f1157a3317084faf86589047175cd496fd4448ea 100644 (file)
--- a/include/clang/Parse/Parser.h
+++ b/include/clang/Parse/Parser.h
@@ -361,8 +361,8 @@ private:
   StmtResult ParseIdentifierStatement(bool OnlyStatement);
   StmtResult ParseCaseStatement();
   StmtResult ParseDefaultStatement();
-  StmtResult ParseCompoundStatement();
-  StmtResult ParseCompoundStatementBody();
+  StmtResult ParseCompoundStatement(bool isStmtExpr = false);
+  StmtResult ParseCompoundStatementBody(bool isStmtExpr = false);
   StmtResult ParseIfStatement();
   StmtResult ParseSwitchStatement();
   StmtResult ParseWhileStatement();

diff --git a/test/Sema/unused-expr.c b/test/Sema/unused-expr.c
index da3a21fcdd7f815a0ae3f364e9ac13b92b94b9b8..e9e2992d0a2c95a9ecd881ba886040e1c444afb6 100644 (file)
--- a/test/Sema/unused-expr.c
+++ b/test/Sema/unused-expr.c
@@ -30,3 +30,8 @@ void t3(int c) {
   c ? t1() : t2();
 }
 
+// This shouldn't warn: the expr at the end of the stmtexpr really is used.
+int stmt_expr(int x, int y) {
+  return ({int _a = x, _b = y; _a > _b ? _a : _b; });
+}
+