Author: nico Date: Thu Dec 22 17:26:17 2011 New Revision: 147202 URL: http://llvm.org/viewvc/llvm-project?rev=147202&view=rev Log: Add -Wdangling-else.
This works like described in http://drdobbs.com/blogs/cpp/231602010 Fixes http://llvm.org/PR11609 Added: cfe/trunk/test/Parser/warn-dangling-else.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td cfe/trunk/include/clang/Parse/Parser.h cfe/trunk/lib/Parse/ParseStmt.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=147202&r1=147201&r2=147202&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Dec 22 17:26:17 2011 @@ -85,6 +85,7 @@ def : DiagGroup<"idiomatic-parentheses">; def BitwiseOpParentheses: DiagGroup<"bitwise-op-parentheses">; def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">; +def DanglingElse: DiagGroup<"dangling-else">; def IgnoredQualifiers : DiagGroup<"ignored-qualifiers">; def : DiagGroup<"import">; def IncompatiblePointerTypes : DiagGroup<"incompatible-pointer-types">; @@ -322,8 +323,8 @@ // Thread Safety warnings def ThreadSafety : DiagGroup<"thread-safety">; -// -Wall is -Wmost -Wparentheses -Wtop-level-comparison -def : DiagGroup<"all", [Most, Parentheses]>; +// -Wall is -Wmost -Wparentheses -Wdangling-else +def : DiagGroup<"all", [DanglingElse, Most, Parentheses]>; // Aliases. def : DiagGroup<"", [Extra]>; // -W = -Wextra Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=147202&r1=147201&r2=147202&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Thu Dec 22 17:26:17 2011 @@ -387,6 +387,9 @@ "expected '=' after declarator">; def warn_parens_disambiguated_as_function_decl : Warning< "parentheses were disambiguated as a function declarator">; +def warn_dangling_else : Warning< + "add explicit braces to avoid dangling else">, + InGroup<DanglingElse>; def err_expected_member_or_base_name : Error< "expected class member or base class name">; def err_expected_lbrace_after_base_specifiers : Error< Modified: cfe/trunk/include/clang/Parse/Parser.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=147202&r1=147201&r2=147202&view=diff ============================================================================== --- cfe/trunk/include/clang/Parse/Parser.h (original) +++ cfe/trunk/include/clang/Parse/Parser.h Thu Dec 22 17:26:17 2011 @@ -1466,9 +1466,9 @@ bool isSimpleObjCMessageExpression(); ExprResult ParseObjCMessageExpression(); ExprResult ParseObjCMessageExpressionBody(SourceLocation LBracloc, - SourceLocation SuperLoc, - ParsedType ReceiverType, - ExprArg ReceiverExpr); + SourceLocation SuperLoc, + ParsedType ReceiverType, + ExprArg ReceiverExpr); ExprResult ParseAssignmentExprWithObjCMessageExprStart( SourceLocation LBracloc, SourceLocation SuperLoc, ParsedType ReceiverType, ExprArg ReceiverExpr); @@ -1477,12 +1477,13 @@ //===--------------------------------------------------------------------===// // C99 6.8: Statements and Blocks. - StmtResult ParseStatement() { + StmtResult ParseStatement(SourceLocation *TrailingElseLoc = NULL) { StmtVector Stmts(Actions); - return ParseStatementOrDeclaration(Stmts, true); + return ParseStatementOrDeclaration(Stmts, true, TrailingElseLoc); } StmtResult ParseStatementOrDeclaration(StmtVector& Stmts, - bool OnlyStatement = false); + bool OnlyStatement, + SourceLocation *TrailingElseLoc = NULL); StmtResult ParseExprStatement(ParsedAttributes &Attrs); StmtResult ParseLabeledStatement(ParsedAttributes &Attr); StmtResult ParseCaseStatement(ParsedAttributes &Attr, @@ -1499,11 +1500,15 @@ Decl *&DeclResult, SourceLocation Loc, bool ConvertToBoolean); - StmtResult ParseIfStatement(ParsedAttributes &Attr); - StmtResult ParseSwitchStatement(ParsedAttributes &Attr); - StmtResult ParseWhileStatement(ParsedAttributes &Attr); + StmtResult ParseIfStatement(ParsedAttributes &Attr, + SourceLocation *TrailingElseLoc); + StmtResult ParseSwitchStatement(ParsedAttributes &Attr, + SourceLocation *TrailingElseLoc); + StmtResult ParseWhileStatement(ParsedAttributes &Attr, + SourceLocation *TrailingElseLoc); StmtResult ParseDoStatement(ParsedAttributes &Attr); - StmtResult ParseForStatement(ParsedAttributes &Attr); + StmtResult ParseForStatement(ParsedAttributes &Attr, + SourceLocation *TrailingElseLoc); StmtResult ParseGotoStatement(ParsedAttributes &Attr); StmtResult ParseContinueStatement(ParsedAttributes &Attr); StmtResult ParseBreakStatement(ParsedAttributes &Attr); Modified: cfe/trunk/lib/Parse/ParseStmt.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=147202&r1=147201&r2=147202&view=diff ============================================================================== --- cfe/trunk/lib/Parse/ParseStmt.cpp (original) +++ cfe/trunk/lib/Parse/ParseStmt.cpp Thu Dec 22 17:26:17 2011 @@ -76,7 +76,8 @@ /// [OBC] '@' 'throw' ';' /// StmtResult -Parser::ParseStatementOrDeclaration(StmtVector &Stmts, bool OnlyStatement) { +Parser::ParseStatementOrDeclaration(StmtVector &Stmts, bool OnlyStatement, + SourceLocation *TrailingElseLoc) { const char *SemiError = 0; StmtResult Res; @@ -234,18 +235,18 @@ } case tok::kw_if: // C99 6.8.4.1: if-statement - return ParseIfStatement(attrs); + return ParseIfStatement(attrs, TrailingElseLoc); case tok::kw_switch: // C99 6.8.4.2: switch-statement - return ParseSwitchStatement(attrs); + return ParseSwitchStatement(attrs, TrailingElseLoc); case tok::kw_while: // C99 6.8.5.1: while-statement - return ParseWhileStatement(attrs); + return ParseWhileStatement(attrs, TrailingElseLoc); case tok::kw_do: // C99 6.8.5.2: do-statement Res = ParseDoStatement(attrs); SemiError = "do/while"; break; case tok::kw_for: // C99 6.8.5.3: for-statement - return ParseForStatement(attrs); + return ParseForStatement(attrs, TrailingElseLoc); case tok::kw_goto: // C99 6.8.6.1: goto-statement Res = ParseGotoStatement(attrs); @@ -874,7 +875,8 @@ /// [C++] 'if' '(' condition ')' statement /// [C++] 'if' '(' condition ')' statement 'else' statement /// -StmtResult Parser::ParseIfStatement(ParsedAttributes &attrs) { +StmtResult Parser::ParseIfStatement(ParsedAttributes &attrs, + SourceLocation *TrailingElseLoc) { // FIXME: Use attributes? assert(Tok.is(tok::kw_if) && "Not an if stmt!"); @@ -933,7 +935,9 @@ // Read the 'then' stmt. SourceLocation ThenStmtLoc = Tok.getLocation(); - StmtResult ThenStmt(ParseStatement()); + + SourceLocation InnerStatementTrailingElseLoc; + StmtResult ThenStmt(ParseStatement(&InnerStatementTrailingElseLoc)); // Pop the 'if' scope if needed. InnerScope.Exit(); @@ -944,6 +948,9 @@ StmtResult ElseStmt; if (Tok.is(tok::kw_else)) { + if (TrailingElseLoc) + *TrailingElseLoc = Tok.getLocation(); + ElseLoc = ConsumeToken(); ElseStmtLoc = Tok.getLocation(); @@ -967,6 +974,8 @@ Actions.CodeCompleteAfterIf(getCurScope()); cutOffParsing(); return StmtError(); + } else if (InnerStatementTrailingElseLoc.isValid()) { + Diag(InnerStatementTrailingElseLoc, diag::warn_dangling_else); } IfScope.Exit(); @@ -1000,7 +1009,8 @@ /// switch-statement: /// 'switch' '(' expression ')' statement /// [C++] 'switch' '(' condition ')' statement -StmtResult Parser::ParseSwitchStatement(ParsedAttributes &attrs) { +StmtResult Parser::ParseSwitchStatement(ParsedAttributes &attrs, + SourceLocation *TrailingElseLoc) { // FIXME: Use attributes? assert(Tok.is(tok::kw_switch) && "Not a switch stmt!"); @@ -1068,7 +1078,7 @@ C99orCXX && Tok.isNot(tok::l_brace)); // Read the body statement. - StmtResult Body(ParseStatement()); + StmtResult Body(ParseStatement(TrailingElseLoc)); // Pop the scopes. InnerScope.Exit(); @@ -1085,7 +1095,8 @@ /// while-statement: [C99 6.8.5.1] /// 'while' '(' expression ')' statement /// [C++] 'while' '(' condition ')' statement -StmtResult Parser::ParseWhileStatement(ParsedAttributes &attrs) { +StmtResult Parser::ParseWhileStatement(ParsedAttributes &attrs, + SourceLocation *TrailingElseLoc) { // FIXME: Use attributes? assert(Tok.is(tok::kw_while) && "Not a while stmt!"); @@ -1143,7 +1154,7 @@ C99orCXX && Tok.isNot(tok::l_brace)); // Read the body statement. - StmtResult Body(ParseStatement()); + StmtResult Body(ParseStatement(TrailingElseLoc)); // Pop the body scope if needed. InnerScope.Exit(); @@ -1242,7 +1253,8 @@ /// [C++0x] for-range-initializer: /// [C++0x] expression /// [C++0x] braced-init-list [TODO] -StmtResult Parser::ParseForStatement(ParsedAttributes &attrs) { +StmtResult Parser::ParseForStatement(ParsedAttributes &attrs, + SourceLocation *TrailingElseLoc) { // FIXME: Use attributes? assert(Tok.is(tok::kw_for) && "Not a for stmt!"); @@ -1467,7 +1479,7 @@ C99orCXXorObjC && Tok.isNot(tok::l_brace)); // Read the body statement. - StmtResult Body(ParseStatement()); + StmtResult Body(ParseStatement(TrailingElseLoc)); // Pop the body scope if needed. InnerScope.Exit(); Added: cfe/trunk/test/Parser/warn-dangling-else.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/warn-dangling-else.cpp?rev=147202&view=auto ============================================================================== --- cfe/trunk/test/Parser/warn-dangling-else.cpp (added) +++ cfe/trunk/test/Parser/warn-dangling-else.cpp Thu Dec 22 17:26:17 2011 @@ -0,0 +1,55 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wdangling-else %s + +void f(int a, int b, int c, int d, int e) { + + // should warn + { if (a) if (b) d++; else e++; } // expected-warning {{add explicit braces to avoid dangling else}} + { if (a) while (b) if (c) d++; else e++; } // expected-warning {{add explicit braces to avoid dangling else}} + { if (a) switch (b) if (c) d++; else e++; } // expected-warning {{add explicit braces to avoid dangling else}} + { if (a) for (;;) if (c) d++; else e++; } // expected-warning {{add explicit braces to avoid dangling else}} + { if (a) if (b) if (d) d++; else e++; else d--; } // expected-warning {{add explicit braces to avoid dangling else}} + + if (a) + if (b) { + d++; + } else e++; // expected-warning {{add explicit braces to avoid dangling else}} + + // shouldn't + { if (a) if (b) d++; } + { if (a) if (b) if (c) d++; } + { if (a) if (b) d++; else e++; else d--; } + { if (a) if (b) if (d) d++; else e++; else d--; else e--; } + { if (a) do if (b) d++; else e++; while (c); } + + if (a) { + if (b) d++; + else e++; + } + + if (a) { + if (b) d++; + } else e++; +} + +// Somewhat more elaborate case that shouldn't warn. +class A { + public: + void operator<<(const char* s) {} +}; + +void HandleDisabledThing() {} +A GetThing() { return A(); } + +#define FOO(X) \ + switch (0) default: \ + if (!(X)) \ + HandleDisabledThing(); \ + else \ + GetThing() + +void f(bool cond) { + int x = 0; + if (cond) + FOO(x) << "hello"; // no warning +} + _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
