[cfe-commits] r129943 - in /cfe/trunk: include/clang/Basic/DiagnosticParseKinds.td include/clang/Parse/Parser.h include/clang/Sema/Scope.h include/clang/Sema/Sema.h lib/Parse/ParseStmt.cpp lib/Sema/SemaExpr.cpp test/Parser/switch-recovery.cpp

Richard Trieu rtrieu at google.com
Thu Apr 21 14:44:26 PDT 2011


Author: rtrieu
Date: Thu Apr 21 16:44:26 2011
New Revision: 129943

URL: http://llvm.org/viewvc/llvm-project?rev=129943&view=rev
Log:
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;
}


Modified:
    cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
    cfe/trunk/include/clang/Parse/Parser.h
    cfe/trunk/include/clang/Sema/Scope.h
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Parse/ParseStmt.cpp
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/Parser/switch-recovery.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=129943&r1=129942&r2=129943&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Thu Apr 21 16:44:26 2011
@@ -204,6 +204,9 @@
 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">;

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=129943&r1=129942&r2=129943&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Thu Apr 21 16:44:26 2011
@@ -1238,7 +1238,9 @@
   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);

Modified: cfe/trunk/include/clang/Sema/Scope.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Scope.h?rev=129943&r1=129942&r2=129943&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Scope.h (original)
+++ cfe/trunk/include/clang/Sema/Scope.h Thu Apr 21 16:44:26 2011
@@ -75,7 +75,10 @@
     
     /// 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 @@
     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;
 

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=129943&r1=129942&r2=129943&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Apr 21 16:44:26 2011
@@ -2226,6 +2226,8 @@
   // __null
   ExprResult ActOnGNUNullExpr(SourceLocation TokenLoc);
 
+  bool CheckCaseExpression(Expr *expr);
+
   //===------------------------- "Block" Extension ------------------------===//
 
   /// ActOnBlockStart - This callback is invoked when a block literal is

Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=129943&r1=129942&r2=129943&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Thu Apr 21 16:44:26 2011
@@ -121,6 +121,9 @@
       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 @@
         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 @@
 ///         '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 @@
 
   // 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 @@
     /// 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 @@
   // 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);

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=129943&r1=129942&r2=129943&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Apr 21 16:44:26 2011
@@ -10734,3 +10734,11 @@
   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;
+}

Modified: cfe/trunk/test/Parser/switch-recovery.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/switch-recovery.cpp?rev=129943&r1=129942&r2=129943&view=diff
==============================================================================
--- cfe/trunk/test/Parser/switch-recovery.cpp (original)
+++ cfe/trunk/test/Parser/switch-recovery.cpp Thu Apr 21 16:44:26 2011
@@ -31,4 +31,128 @@
       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;
+        }
+      }
+    }
+  }
+}





More information about the cfe-commits mailing list