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; });
}

llvm-svn: 41655

clang/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.

clang/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

clang/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,

clang/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);
 }
 

clang/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 = "";

clang/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) {

clang/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();

clang/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; });
+}
+