[cfe-commits] r147202 - in /cfe/trunk: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticParseKinds.td include/clang/Parse/Parser.h lib/Parse/ParseStmt.cpp test/Parser/warn-dangling-else.cpp
Chad Rosier
mcrosier at apple.com
Mon Jan 16 19:03:30 PST 2012
Hi Nico,
I get a warning with the following test case.
int foo(int a, int b, int c) {
int rval = 0;
if (a != b)
if (b == c) {
rval = 0;
} else {
rval = 1;
}
return rval;
}
mcrosier$ clang -O3 t.c -Wall -c -o /dev/null
t.c:6:11: warning: add explicit braces to avoid dangling else [-Wdangling-else]
} else {
^
1 warning generated.
Is this expected behavior? This code seems rather explicit to me.
Chad
On Dec 22, 2011, at 3:26 PM, Nico Weber wrote:
> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list