Improved recovery of switch statement

Make better diagnostic produced by erroneous switch statement.
It fixes PR19022.

Differential Revision: http://reviews.llvm.org/D3137

llvm-svn: 209302

clang/lib/Parse/ParseStmt.cpp

@@ -612,6 +612,7 @@ StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) {
   do {
     SourceLocation CaseLoc = MissingCase ? Expr.get()->getExprLoc() :
                                            ConsumeToken();  // eat the 'case'.
+    ColonLoc = SourceLocation();
 
     if (Tok.is(tok::code_completion)) {
       Actions.CodeCompleteCase(getCurScope());
@@ -624,11 +625,21 @@ StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) {
     /// expression.
     ColonProtectionRAIIObject ColonProtection(*this);
 
-    ExprResult LHS(MissingCase ? Expr : ParseConstantExpression());
-    MissingCase = false;
-    if (LHS.isInvalid()) {
-      SkipUntil(tok::colon, StopAtSemi);
-      return StmtError();
+    ExprResult LHS;
+    if (!MissingCase) {
+      LHS = ParseConstantExpression();
+      if (LHS.isInvalid()) {
+        // If constant-expression is parsed unsuccessfully, recover by skipping
+        // current case statement (moving to the colon that ends it).
+        if (SkipUntil(tok::colon, tok::r_brace, StopAtSemi | StopBeforeMatch)) {
+          TryConsumeToken(tok::colon, ColonLoc);
+          continue;
+        }
+        return StmtError();
+      }
+    } else {
+      LHS = Expr;
+      MissingCase = false;
     }
 
     // GNU case range extension.
@@ -638,7 +649,10 @@ StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) {
       Diag(DotDotDotLoc, diag::ext_gnu_case_range);
       RHS = ParseConstantExpression();
       if (RHS.isInvalid()) {
-        SkipUntil(tok::colon, StopAtSemi);
+        if (SkipUntil(tok::colon, tok::r_brace, StopAtSemi | StopBeforeMatch)) {
+          TryConsumeToken(tok::colon, ColonLoc);
+          continue;
+        }
         return StmtError();
       }
     }
@@ -684,28 +698,30 @@ StmtResult Parser::ParseCaseStatement(bool MissingCase, ExprResult Expr) {
     // Handle all case statements.
   } while (Tok.is(tok::kw_case));
 
-  assert(!TopLevelCase.isInvalid() && "Should have parsed at least one case!");
-
   // If we found a non-case statement, start by parsing it.
   StmtResult SubStmt;
 
   if (Tok.isNot(tok::r_brace)) {
     SubStmt = ParseStatement();
   } else {
     // Nicely diagnose the common error "switch (X) { case 4: }", which is
-    // not valid.
-    SourceLocation AfterColonLoc = PP.getLocForEndOfToken(ColonLoc);
-    Diag(AfterColonLoc, diag::err_label_end_of_compound_statement)
-      << FixItHint::CreateInsertion(AfterColonLoc, " ;");
-    SubStmt = true;
+    // not valid.  If ColonLoc doesn't point to a valid text location, there was
+    // another parsing error, so avoid producing extra diagnostics.
+    if (ColonLoc.isValid()) {
+      SourceLocation AfterColonLoc = PP.getLocForEndOfToken(ColonLoc);
+      Diag(AfterColonLoc, diag::err_label_end_of_compound_statement)
+        << FixItHint::CreateInsertion(AfterColonLoc, " ;");
+    }
+    SubStmt = StmtError();
   }
 
-  // Broken sub-stmt shouldn't prevent forming the case statement properly.
-  if (SubStmt.isInvalid())
-    SubStmt = Actions.ActOnNullStmt(SourceLocation());
-
   // Install the body into the most deeply-nested case.
-  Actions.ActOnCaseStmtBody(DeepestParsedCaseStmt, SubStmt.get());
+  if (DeepestParsedCaseStmt) {
+    // Broken sub-stmt shouldn't prevent forming the case statement properly.
+    if (SubStmt.isInvalid())
+      SubStmt = Actions.ActOnNullStmt(SourceLocation());
+    Actions.ActOnCaseStmtBody(DeepestParsedCaseStmt, SubStmt.get());
+  }
 
   // Return the top level parsed statement tree.
   return TopLevelCase;
@@ -1234,15 +1250,6 @@ StmtResult Parser::ParseSwitchStatement(SourceLocation *TrailingElseLoc) {
   InnerScope.Exit();
   SwitchScope.Exit();
 
-  if (Body.isInvalid()) {
-    // FIXME: Remove the case statement list from the Switch statement.
-
-    // Put the synthesized null statement on the same line as the end of switch
-    // condition.
-    SourceLocation SynthesizedNullStmtLocation = Cond.get()->getLocEnd();
-    Body = Actions.ActOnNullStmt(SynthesizedNullStmtLocation);
-  }
-
   return Actions.ActOnFinishSwitchStmt(SwitchLoc, Switch.get(), Body.get());
 }
 

clang/lib/Sema/SemaStmt.cpp

@@ -702,6 +702,7 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
   assert(SS == getCurFunction()->SwitchStack.back() &&
          "switch stack missing push/pop!");
 
+  if (!BodyStmt) return StmtError();
   SS->setBody(BodyStmt, SwitchLoc);
   getCurFunction()->SwitchStack.pop_back();
 
@@ -1141,8 +1142,9 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
     }
   }
 
-  DiagnoseEmptyStmtBody(CondExpr->getLocEnd(), BodyStmt,
-                        diag::warn_empty_switch_body);
+  if (BodyStmt)
+    DiagnoseEmptyStmtBody(CondExpr->getLocEnd(), BodyStmt,
+                          diag::warn_empty_switch_body);
 
   // FIXME: If the case list was broken is some way, we don't have a good system
   // to patch it up.  Instead, just return the whole substmt as broken.

clang/test/Parser/switch-recovery.cpp

@@ -170,3 +170,53 @@ void missing_statement_default(int x) {
     default: // expected-error {{label at end of compound statement: expected statement}}
   }
 }
+
+void pr19022_1() {
+  switch (int x)  // expected-error {{variable declaration in condition must have an initializer}}
+  case v: ;  // expected-error {{use of undeclared identifier 'v'}}
+}
+
+void pr19022_1a(int x) {
+  switch(x) {
+  case 1  // expected-error{{expected ':' after 'case'}} \
+          // expected-error{{label at end of compound statement: expected statement}}
+  }
+}
+
+void pr19022_1b(int x) {
+  switch(x) {
+  case v  // expected-error{{use of undeclared identifier 'v'}}
+  }
+ }
+
+void pr19022_2() {
+  switch (int x)  // expected-error {{variable declaration in condition must have an initializer}}
+  case v1: case v2: ;  // expected-error {{use of undeclared identifier 'v1'}} \
+                       // expected-error {{use of undeclared identifier 'v2'}}
+}
+
+void pr19022_3(int x) {
+  switch (x)
+  case 1: case v2: ;  // expected-error {{use of undeclared identifier 'v2'}}
+}
+
+int pr19022_4(int x) {
+  switch(x) {
+  case 1  // expected-error{{expected ':' after 'case'}} expected-note{{previous case defined here}}
+  case 1 : return x;  // expected-error{{duplicate case value '1'}}
+  }
+}
+
+void pr19022_5(int x) {
+  switch(x) {
+  case 1: case
+  }  // expected-error{{expected expression}}
+}
+
+namespace pr19022 {
+int baz5() {}
+bool bar0() {
+  switch (int foo0)  //expected-error{{variable declaration in condition must have an initializer}}
+  case bar5: ;  // expected-error{{use of undeclared identifier 'bar5'}}
+}
+}

clang/test/Sema/statements.c

@@ -36,7 +36,7 @@ void *test10() {
 
 // PR6034
 void test11(int bit) {
-  switch (bit) // expected-warning {{switch statement has empty body}} expected-note {{put the semicolon on a separate line to silence this warning}}
+  switch (bit)
   switch (env->fpscr)  // expected-error {{use of undeclared identifier 'env'}}
   {
   }