From cac27a5478831d8162cc81a06e33e8aa1f1e7a45 Mon Sep 17 00:00:00 2001 From: Chris Lattner Date: Fri, 31 Aug 2007 21:49:55 +0000 Subject: [PATCH] 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 | 5 ++- clang/Parse/ParseStmt.cpp | 8 ++--- clang/Sema/Sema.h | 3 +- clang/Sema/SemaStmt.cpp | 48 +++++++++++++++------------ clang/clang.xcodeproj/project.pbxproj | 1 - clang/include/clang/Parse/Action.h | 3 +- clang/include/clang/Parse/Parser.h | 4 +-- clang/test/Sema/unused-expr.c | 5 +++ 8 files changed, 44 insertions(+), 33 deletions(-) diff --git a/clang/Parse/ParseExpr.cpp b/clang/Parse/ParseExpr.cpp index 51ad217e3129..c2b54828556c 100644 --- a/clang/Parse/ParseExpr.cpp +++ b/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. diff --git a/clang/Parse/ParseStmt.cpp b/clang/Parse/ParseStmt.cpp index 80f7f0329e10..44caaf8c916a 100644 --- a/clang/Parse/ParseStmt.cpp +++ b/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 diff --git a/clang/Sema/Sema.h b/clang/Sema/Sema.h index a968dba88ad8..78b88b78a06b 100644 --- a/clang/Sema/Sema.h +++ b/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, diff --git a/clang/Sema/SemaStmt.cpp b/clang/Sema/SemaStmt.cpp index 300388cf33ae..bec7f892dfb5 100644 --- a/clang/Sema/SemaStmt.cpp +++ b/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(E)) - S.Diag(BO->getOperatorLoc(), diag::warn_unused_expr, - BO->getLHS()->getSourceRange(), BO->getRHS()->getSourceRange()); - else if (const UnaryOperator *UO = dyn_cast(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); 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(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(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(E)) + Diag(BO->getOperatorLoc(), diag::warn_unused_expr, + BO->getLHS()->getSourceRange(), BO->getRHS()->getSourceRange()); + else if (const UnaryOperator *UO = dyn_cast(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/clang.xcodeproj/project.pbxproj b/clang/clang.xcodeproj/project.pbxproj index 4beeaef7a6ba..ceddc1fe5137 100644 --- a/clang/clang.xcodeproj/project.pbxproj +++ b/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 = ""; diff --git a/clang/include/clang/Parse/Action.h b/clang/include/clang/Parse/Action.h index bc23b8121ee7..ec9a4d0dd261 100644 --- a/clang/include/clang/Parse/Action.h +++ b/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) { diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 65d1ff6431e8..f1157a331708 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/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(); diff --git a/clang/test/Sema/unused-expr.c b/clang/test/Sema/unused-expr.c index da3a21fcdd7f..e9e2992d0a2c 100644 --- a/clang/test/Sema/unused-expr.c +++ b/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; }); +} +