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

llvm-svn: 129943

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

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

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

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

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

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

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