From: Richard Trieu Date: Thu, 21 Apr 2011 21:44:26 +0000 (+0000) Subject: Add a fixit suggest for missing case keywords inside a switch scope. For instance... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=bb9b80c308d7d3f758886243c7145404a50c66cf;p=clang Add a fixit suggest for missing case keywords inside a switch scope. For instance, in the following code, 'case ' will be suggested before the '1:' switch (x) { 1: return 0; default: return 1; } git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@129943 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticParseKinds.td b/include/clang/Basic/DiagnosticParseKinds.td index 09ac5fe02f..abc3808dea 100644 --- a/include/clang/Basic/DiagnosticParseKinds.td +++ b/include/clang/Basic/DiagnosticParseKinds.td @@ -204,6 +204,9 @@ def err_expected_class_name : Error<"expected class name">; def err_unspecified_vla_size_with_static : Error< "'static' may not be used with an unspecified variable length array size">; +def err_expected_case_before_expression: Error< + "expected 'case' keyword before expression">; + // Declarations. def err_typename_requires_specqual : Error< "type name requires a specifier or qualifier">; diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index 9be3d4d1ff..efc622cc0d 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -1238,7 +1238,9 @@ private: StmtResult ParseStatementOrDeclaration(StmtVector& Stmts, bool OnlyStatement = false); StmtResult ParseLabeledStatement(ParsedAttributes &Attr); - StmtResult ParseCaseStatement(ParsedAttributes &Attr); + StmtResult ParseCaseStatement(ParsedAttributes &Attr, + bool MissingCase = false, + ExprResult Expr = ExprResult()); StmtResult ParseDefaultStatement(ParsedAttributes &Attr); StmtResult ParseCompoundStatement(ParsedAttributes &Attr, bool isStmtExpr = false); diff --git a/include/clang/Sema/Scope.h b/include/clang/Sema/Scope.h index 82f527e4be..f70016cbfd 100644 --- a/include/clang/Sema/Scope.h +++ b/include/clang/Sema/Scope.h @@ -75,7 +75,10 @@ public: /// ObjCMethodScope - This scope corresponds to an Objective-C method body. /// It always has FnScope and DeclScope set as well. - ObjCMethodScope = 0x400 + ObjCMethodScope = 0x400, + + /// SwitchScope - This is a scope that corresponds to a switch statement. + SwitchScope = 0x800 }; private: /// The parent scope for this scope. This is null for the translation-unit @@ -260,6 +263,20 @@ public: return getFlags() & Scope::AtCatchScope; } + /// isSwitchScope - Return true if this scope is a switch scope. + bool isSwitchScope() const { + for (const Scope *S = this; S; S = S->getParent()) { + if (S->getFlags() & Scope::SwitchScope) + return true; + else if (S->getFlags() & (Scope::FnScope | Scope::ClassScope | + Scope::BlockScope | Scope::TemplateParamScope | + Scope::FunctionPrototypeScope | + Scope::AtCatchScope | Scope::ObjCMethodScope)) + return false; + } + return false; + } + typedef UsingDirectivesTy::iterator udir_iterator; typedef UsingDirectivesTy::const_iterator const_udir_iterator; diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index df3c6643e3..f42fe3d610 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -2226,6 +2226,8 @@ public: // __null ExprResult ActOnGNUNullExpr(SourceLocation TokenLoc); + bool CheckCaseExpression(Expr *expr); + //===------------------------- "Block" Extension ------------------------===// /// ActOnBlockStart - This callback is invoked when a block literal is diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index d70a745314..aaf79acf4f 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/lib/Parse/ParseStmt.cpp @@ -121,6 +121,9 @@ Parser::ParseStatementOrDeclaration(StmtVector &Stmts, bool OnlyStatement) { return StmtError(); } + // If a case keyword is missing, this is where it should be inserted. + Token OldToken = Tok; + // FIXME: Use the attributes // expression[opt] ';' ExprResult Expr(ParseExpression()); @@ -133,6 +136,18 @@ Parser::ParseStatementOrDeclaration(StmtVector &Stmts, bool OnlyStatement) { ConsumeToken(); return StmtError(); } + + if (Tok.is(tok::colon) && getCurScope()->isSwitchScope() && + Actions.CheckCaseExpression(Expr.get())) { + // If a constant expression is followed by a colon inside a switch block, + // suggest a missing case keywork. + Diag(OldToken, diag::err_expected_case_before_expression) + << FixItHint::CreateInsertion(OldToken.getLocation(), "case "); + + // Recover parsing as a case statement. + return ParseCaseStatement(attrs, /*MissingCase=*/true, Expr); + } + // Otherwise, eat the semicolon. ExpectAndConsumeSemi(diag::err_expected_semi_after_expr); return Actions.ActOnExprStmt(Actions.MakeFullExpr(Expr.get())); @@ -251,8 +266,9 @@ StmtResult Parser::ParseLabeledStatement(ParsedAttributes &attrs) { /// 'case' constant-expression ':' statement /// [GNU] 'case' constant-expression '...' constant-expression ':' statement /// -StmtResult Parser::ParseCaseStatement(ParsedAttributes &attrs) { - assert(Tok.is(tok::kw_case) && "Not a case stmt!"); +StmtResult Parser::ParseCaseStatement(ParsedAttributes &attrs, bool MissingCase, + ExprResult Expr) { + assert(MissingCase || Tok.is(tok::kw_case) && "Not a case stmt!"); // FIXME: Use attributes? // It is very very common for code to contain many case statements recursively @@ -280,7 +296,8 @@ StmtResult Parser::ParseCaseStatement(ParsedAttributes &attrs) { // While we have case statements, eat and stack them. do { - SourceLocation CaseLoc = ConsumeToken(); // eat the 'case'. + SourceLocation CaseLoc = MissingCase ? Expr.get()->getExprLoc() : + ConsumeToken(); // eat the 'case'. if (Tok.is(tok::code_completion)) { Actions.CodeCompleteCase(getCurScope()); @@ -292,7 +309,8 @@ StmtResult Parser::ParseCaseStatement(ParsedAttributes &attrs) { /// expression. ColonProtectionRAIIObject ColonProtection(*this); - ExprResult LHS(ParseConstantExpression()); + ExprResult LHS(MissingCase ? Expr : ParseConstantExpression()); + MissingCase = false; if (LHS.isInvalid()) { SkipUntil(tok::colon); return StmtError(); @@ -775,7 +793,7 @@ StmtResult Parser::ParseSwitchStatement(ParsedAttributes &attrs) { // while, for, and switch statements are local to the if, while, for, or // switch statement (including the controlled statement). // - unsigned ScopeFlags = Scope::BreakScope; + unsigned ScopeFlags = Scope::BreakScope | Scope::SwitchScope; if (C99orCXX) ScopeFlags |= Scope::DeclScope | Scope::ControlScope; ParseScope SwitchScope(this, ScopeFlags); diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 8eb0177046..0e0bf435b2 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -10734,3 +10734,11 @@ ExprResult Sema::CheckPlaceholderExpr(Expr *E) { assert(!type->isPlaceholderType()); return Owned(E); } + +bool Sema::CheckCaseExpression(Expr *expr) { + if (expr->isTypeDependent()) + return true; + if (expr->isValueDependent() || expr->isIntegerConstantExpr(Context)) + return expr->getType()->isIntegralOrEnumerationType(); + return false; +} diff --git a/test/Parser/switch-recovery.cpp b/test/Parser/switch-recovery.cpp index f11babc5b6..0e4dcfa3c6 100644 --- a/test/Parser/switch-recovery.cpp +++ b/test/Parser/switch-recovery.cpp @@ -31,4 +31,128 @@ struct B { break; } } + + int test3(int i) { + switch (i) { + case 1: return 0; + 2: return 1; // expected-error {{expected 'case' keyword before expression}} + default: return 5; + } + } }; + +int test4(int i) { + switch (i) + 1: return -1; // expected-error {{expected 'case' keyword before expression}} + return 0; +} + +int test5(int i) { + switch (i) { + case 1: case 2: case 3: return 1; + { + 4:5:6:7: return 2; // expected-error 4{{expected 'case' keyword before expression}} + } + default: return -1; + } +} + +int test6(int i) { + switch (i) { + case 1: + case 4: + // This class provides extra single colon tokens. Make sure no + // errors are seen here. + class foo{ + public: + protected: + private: + }; + case 2: + 5: // expected-error {{expected 'case' keyword before expression}} + default: return 1; + } +} + +int test7(int i) { + switch (i) { + case false ? 1 : 2: + true ? 1 : 2: // expected-error {{expected 'case' keyword before expression}} + case 10: + 14 ? 3 : 4; + default: + return 1; + } +} + +enum foo { A, B, C}; +int test8( foo x ) { + switch (x) { + A: return 0; // FIXME: give a warning for unused labels that could also be + // a case expression. + default: return 1; + } +} + +// Stress test to make sure Clang doesn't crash. +void test9(int x) { + switch(x) { + case 1: return; + 2: case; // expected-error {{expected 'case' keyword before expression}} \ + expected-error {{expected expression}} + 4:5:6: return; // expected-error 3{{expected 'case' keyword before expression}} + 7: :x; // expected-error {{expected 'case' keyword before expression}} \ + expected-error {{expected expression}} + 8:: x; // expected-error {{expected ';' after expression}} \ + expected-error {{no member named 'x' in the global namespace}} \ + expected-warning {{expression result unused}} + 9:: :y; // expected-error {{expected ';' after expression}} \ + expected-error {{expected unqualified-id}} \ + expected-warning {{expression result unused}} + :; // expected-error {{expected expression}} + ::; // expected-error {{expected unqualified-id}} + } +} + +void test10(int x) { + switch (x) { + case 1: { + struct Inner { + void g(int y) { + 2: y++; // expected-error {{expected ';' after expression}} \ + // expected-warning {{expression result unused}} + } + }; + break; + } + } +} + +template +struct test11 { + enum { E }; + + void f(int x) { + switch (x) { + E: break; // FIXME: give a 'case' fix-it for unused labels that + // could also be an expression an a case label. + E+1: break; // expected-error {{expected 'case' keyword before expression}} + } + } +}; + +void test12(int x) { + switch (x) { + 0: // expected-error {{expected 'case' keyword before expression}} + while (x) { + 1: // expected-error {{expected 'case' keyword before expression}} + for (;x;) { + 2: // expected-error {{expected 'case' keyword before expression}} + if (x > 0) { + 3: // expected-error {{expected 'case' keyword before expression}} + --x; + } + } + } + } +}