Reject continue/break statements within members of local functions nested within

loop and switch statements, by teaching Scope that a function scope never has
a continue/break parent for the purposes of control flow. Remove the hack in
block and lambda expressions which worked around this by pretending that such
expressions were continue/break scopes.

Remove Scope::ControlParent, since it's unused.

In passing, teach default statements to recover properly from a missing ';', and
add a fixit for same to both default and case labels (the latter already
recovered correctly).

llvm-svn: 150776

clang/include/clang/Sema/Scope.h

@@ -57,7 +57,7 @@ public:
     /// BlockScope - This is a scope that corresponds to a block/closure object.
     /// Blocks serve as top-level scopes for some objects like labels, they
     /// also prevent things like break and continue.  BlockScopes always have
-    /// the FnScope, BreakScope, ContinueScope, and DeclScope flags set as well.
+    /// the FnScope and DeclScope flags set as well.
     BlockScope = 0x40,
 
     /// TemplateParamScope - This is a scope that corresponds to the
@@ -114,16 +114,12 @@ private:
   /// pointer is non-null and points to it.  This is used for label processing.
   Scope *FnParent;
 
-  /// BreakParent/ContinueParent - This is a direct link to the immediately
-  /// preceding BreakParent/ContinueParent if this scope is not one, or null if
-  /// there is no containing break/continue scope.
+  /// BreakParent/ContinueParent - This is a direct link to the innermost
+  /// BreakScope/ContinueScope which contains the contents of this scope
+  /// for control flow purposes (and might be this scope itself), or null
+  /// if there is no such scope.
   Scope *BreakParent, *ContinueParent;
 
-  /// ControlParent - This is a direct link to the immediately
-  /// preceding ControlParent if this scope is not one, or null if
-  /// there is no containing control scope.
-  Scope *ControlParent;
-
   /// BlockParent - This is a direct link to the immediately containing
   /// BlockScope if this scope is not one, or null if there is none.
   Scope *BlockParent;
@@ -180,12 +176,9 @@ public:
   Scope *getFnParent() { return FnParent; }
 
   /// getContinueParent - Return the closest scope that a continue statement
-  /// would be affected by.  If the closest scope is a closure scope, we know
-  /// that there is no loop *inside* the closure.
+  /// would be affected by.
   Scope *getContinueParent() {
-    if (ContinueParent && !ContinueParent->isBlockScope())
-      return ContinueParent;
-    return 0;
+    return ContinueParent;
   }
 
   const Scope *getContinueParent() const {
@@ -193,20 +186,14 @@ public:
   }
 
   /// getBreakParent - Return the closest scope that a break statement
-  /// would be affected by.  If the closest scope is a block scope, we know
-  /// that there is no loop *inside* the block.
+  /// would be affected by.
   Scope *getBreakParent() {
-    if (BreakParent && !BreakParent->isBlockScope())
-      return BreakParent;
-    return 0;
+    return BreakParent;
   }
   const Scope *getBreakParent() const {
     return const_cast<Scope*>(this)->getBreakParent();
   }
 
-  Scope *getControlParent() { return ControlParent; }
-  const Scope *getControlParent() const { return ControlParent; }
-
   Scope *getBlockParent() { return BlockParent; }
   const Scope *getBlockParent() const { return BlockParent; }
 

clang/lib/Parse/ParseExpr.cpp

@@ -2290,7 +2290,6 @@ ExprResult Parser::ParseBlockLiteralExpression() {
   // allows determining whether a variable reference inside the block is
   // within or outside of the block.
   ParseScope BlockScope(this, Scope::BlockScope | Scope::FnScope |
-                              Scope::BreakScope | Scope::ContinueScope |
                               Scope::DeclScope);
 
   // Inform sema that we are starting a block.

clang/lib/Parse/ParseExprCXX.cpp

@@ -871,7 +871,6 @@ ExprResult Parser::ParseLambdaExpressionAfterIntroducer(
   // FIXME: Rename BlockScope -> ClosureScope if we decide to continue using
   // it.
   ParseScope BodyScope(this, Scope::BlockScope | Scope::FnScope |
-                             Scope::BreakScope | Scope::ContinueScope |
                              Scope::DeclScope);
 
   Actions.ActOnStartOfLambdaDefinition(Intro, D, getCurScope());

clang/lib/Parse/ParseStmt.cpp

@@ -596,7 +596,8 @@ StmtResult Parser::ParseCaseStatement(ParsedAttributes &attrs, bool MissingCase,
     // 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);
+    Diag(AfterColonLoc, diag::err_label_end_of_compound_statement)
+      << FixItHint::CreateInsertion(AfterColonLoc, " ;");
     SubStmt = true;
   }
 
@@ -638,16 +639,22 @@ StmtResult Parser::ParseDefaultStatement(ParsedAttributes &attrs) {
     ColonLoc = ExpectedLoc;
   }
 
-  // Diagnose the common error "switch (X) {... default: }", which is not valid.
-  if (Tok.is(tok::r_brace)) {
+  StmtResult SubStmt;
+
+  if (Tok.isNot(tok::r_brace)) {
+    SubStmt = ParseStatement();
+  } else {
+    // Diagnose the common error "switch (X) {... default: }", which is
+    // not valid.
     SourceLocation AfterColonLoc = PP.getLocForEndOfToken(ColonLoc);
-    Diag(AfterColonLoc, diag::err_label_end_of_compound_statement);
-    return StmtError();
+    Diag(AfterColonLoc, diag::err_label_end_of_compound_statement)
+      << FixItHint::CreateInsertion(AfterColonLoc, " ;");
+    SubStmt = true;
   }
 
-  StmtResult SubStmt(ParseStatement());
+  // Broken sub-stmt shouldn't prevent forming the case statement properly.
   if (SubStmt.isInvalid())
-    return StmtError();
+    SubStmt = Actions.ActOnNullStmt(ColonLoc);
 
   return Actions.ActOnDefaultStmt(DefaultLoc, ColonLoc,
                                   SubStmt.get(), getCurScope());

clang/lib/Sema/Scope.cpp

@@ -19,23 +19,28 @@ using namespace clang;
 void Scope::Init(Scope *parent, unsigned flags) {
   AnyParent = parent;
   Flags = flags;
-    
+
+  if (parent && !(flags & FnScope)) {
+    BreakParent    = parent->BreakParent;
+    ContinueParent = parent->ContinueParent;
+  } else {
+    // Control scopes do not contain the contents of nested function scopes for
+    // control flow purposes.
+    BreakParent = ContinueParent = 0;
+  }
+
   if (parent) {
     Depth = parent->Depth + 1;
     PrototypeDepth = parent->PrototypeDepth;
     PrototypeIndex = 0;
     FnParent       = parent->FnParent;
-    BreakParent    = parent->BreakParent;
-    ContinueParent = parent->ContinueParent;
-    ControlParent  = parent->ControlParent;
     BlockParent    = parent->BlockParent;
     TemplateParamParent = parent->TemplateParamParent;
   } else {
     Depth = 0;
     PrototypeDepth = 0;
     PrototypeIndex = 0;
-    FnParent = BreakParent = ContinueParent = BlockParent = 0;
-    ControlParent = 0;
+    FnParent = BlockParent = 0;
     TemplateParamParent = 0;
   }
 
@@ -43,7 +48,6 @@ void Scope::Init(Scope *parent, unsigned flags) {
   if (flags & FnScope)            FnParent = this;
   if (flags & BreakScope)         BreakParent = this;
   if (flags & ContinueScope)      ContinueParent = this;
-  if (flags & ControlScope)       ControlParent = this;
   if (flags & BlockScope)         BlockParent = this;
   if (flags & TemplateParamScope) TemplateParamParent = this;
 

clang/test/FixIt/fixit.c

@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -pedantic -Wunused-label -verify -x c %s
 // RUN: cp %s %t
 // RUN: not %clang_cc1 -pedantic -Wunused-label -fixit -x c %t
 // RUN: grep -v CHECK %t > %t2
@@ -12,22 +13,24 @@
 // FIXME: FIX-IT should add #include <string.h>?
 int strcmp(const char *s1, const char *s2);
 
-void f0(void) { };
+void f0(void) { }; // expected-warning {{';'}}
 
 struct s {
-  int x, y;;
+  int x, y;; // expected-warning {{extra ';'}}
 };
 
 // CHECK: _Complex double cd;
-_Complex cd;
+_Complex cd; // expected-warning {{assuming '_Complex double'}}
 
 // CHECK: struct s s0 = { .y = 5 };
-struct s s0 = { y: 5 };
+struct s s0 = { y: 5 }; // expected-warning {{GNU old-style}}
 
 // CHECK: int array0[5] = { [3] = 3 };
-int array0[5] = { [3] 3 };
+int array0[5] = { [3] 3 }; // expected-warning {{GNU 'missing ='}}
 
-void f1(x, y)
+// CHECK: int x
+// CHECK: int y
+void f1(x, y) // expected-warning 2{{defaulting to type 'int'}}
 {
 }
 
@@ -36,16 +39,16 @@ int i0 = { 17 };
 #define ONE 1
 #define TWO 2
 
-int test_cond(int y, int fooBar) {
+int test_cond(int y, int fooBar) { // expected-note {{here}}
 // CHECK: int x = y ? 1 : 4+fooBar;
-  int x = y ? 1 4+foobar;
+  int x = y ? 1 4+foobar; // expected-error {{expected ':'}} expected-error {{undeclared identifier}} expected-note {{to match}}
 // CHECK: x = y ? ONE : TWO;
-  x = y ? ONE TWO;
+  x = y ? ONE TWO; // expected-error {{':'}} expected-note {{to match}}
   return x;
 }
 
-// CHECK: typedef int int_t;
-typedef typedef int int_t;
+// CHECK: const typedef int int_t;
+const typedef typedef int int_t; // expected-warning {{duplicate 'typedef'}}
 
 // <rdar://problem/7159693>
 enum Color { 
@@ -61,19 +64,39 @@ struct test_struct {
 };
 
 void removeUnusedLabels(char c) {
-  L0 /*removed comment*/:        c++;
+  L0 /*removed comment*/:        c++; // expected-warning {{unused label}}
   removeUnusedLabels(c);
-  L1:
+  L1: // expected-warning {{unused label}}
   c++;
-  /*preserved comment*/ L2  :        c++;
-  LL
+  /*preserved comment*/ L2  :        c++; // expected-warning {{unused label}}
+  LL // expected-warning {{unused label}}
   : c++;
-  c = c + 3; L4: return;
+  c = c + 3; L4: return; // expected-warning {{unused label}}
 }
 
-int oopsAComma = 0,
+int oopsAComma = 0, // expected-error {{';'}}
 void oopsMoreCommas() {
-  static int a[] = { 0, 1, 2 },
-  static int b[] = { 3, 4, 5 },
+  static int a[] = { 0, 1, 2 }, // expected-error {{';'}}
+  static int b[] = { 3, 4, 5 }, // expected-error {{';'}}
   &a == &b ? oopsMoreCommas() : removeUnusedLabels(a[0]);
 }
+
+int noSemiAfterLabel(int n) {
+  switch (n) {
+    default:
+      return n % 4;
+    case 0:
+    case 1:
+    case 2:
+    // CHECK: /*FOO*/ case 3: ;
+    /*FOO*/ case 3: // expected-error {{expected statement}}
+  }
+  switch (n) {
+    case 1:
+    case 2:
+      return 0;
+    // CHECK: /*BAR*/ default: ;
+    /*BAR*/ default: // expected-error {{expected statement}}
+  }
+  return 1;
+}

clang/test/SemaCXX/switch.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 
 void test() {
   bool x = true;
@@ -64,3 +64,24 @@ void click_check_header_sizes() {
   case 0: ;
   }
 }
+
+void local_class(int n) {
+  for (;;) switch (n) {
+  case 0:
+    struct S {
+      void f() {
+        case 1: // expected-error {{'case' statement not in switch statement}}
+        break; // expected-error {{'break' statement not in loop or switch statement}}
+        default: // expected-error {{'default' statement not in switch statement}}
+        continue; // expected-error {{'continue' statement not in loop statement}}
+      }
+    };
+    S().f();
+    []{
+      case 2: // expected-error {{'case' statement not in switch statement}}
+      break; // expected-error {{'break' statement not in loop or switch statement}}
+      default: // expected-error {{'default' statement not in switch statement}}
+      continue; // expected-error {{'continue' statement not in loop statement}}
+    }();
+  }
+}