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).
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@150776 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/include/clang/Sema/Scope.h b/include/clang/Sema/Scope.h
index cff8b33..9b3dd92 100644
--- a/include/clang/Sema/Scope.h
+++ b/include/clang/Sema/Scope.h
@@ -57,7 +57,7 @@
/// 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 @@
/// 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 @@
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 @@
}
/// 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; }
diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
index 642fc2a..75510ad 100644
--- a/lib/Parse/ParseExpr.cpp
+++ b/lib/Parse/ParseExpr.cpp
@@ -2290,7 +2290,6 @@
// 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.
diff --git a/lib/Parse/ParseExprCXX.cpp b/lib/Parse/ParseExprCXX.cpp
index d09dba2..76cc392 100644
--- a/lib/Parse/ParseExprCXX.cpp
+++ b/lib/Parse/ParseExprCXX.cpp
@@ -871,7 +871,6 @@
// 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());
diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp
index 8af4da6..c256f78 100644
--- a/lib/Parse/ParseStmt.cpp
+++ b/lib/Parse/ParseStmt.cpp
@@ -596,7 +596,8 @@
// 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 @@
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());
diff --git a/lib/Sema/Scope.cpp b/lib/Sema/Scope.cpp
index 833a59f..c76f61a 100644
--- a/lib/Sema/Scope.cpp
+++ b/lib/Sema/Scope.cpp
@@ -19,23 +19,28 @@
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 @@
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;
diff --git a/test/FixIt/fixit.c b/test/FixIt/fixit.c
index 967ae23..5e4947b 100644
--- a/test/FixIt/fixit.c
+++ b/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 @@
#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 @@
};
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;
+}
diff --git a/test/SemaCXX/switch.cpp b/test/SemaCXX/switch.cpp
index 5fd6b1a..517faa9 100644
--- a/test/SemaCXX/switch.cpp
+++ b/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 @@
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}}
+ }();
+ }
+}