Updated patch according to notes of Justin Bogner.
http://llvm-reviews.chandlerc.com/D2518
CHANGE SINCE LAST DIFF
http://llvm-reviews.chandlerc.com/D2518?vs=6380&id=6381#toc
Files:
include/clang/Sema/Scope.h
lib/Parse/ParseStmt.cpp
lib/Sema/Scope.cpp
test/CodeGen/PR8880.c
test/Parser/bad-control.c
test/Sema/statements.c
Index: include/clang/Sema/Scope.h
===================================================================
--- include/clang/Sema/Scope.h
+++ include/clang/Sema/Scope.h
@@ -354,6 +354,16 @@
/// Init - This is used by the parser to implement scope caching.
///
void Init(Scope *parent, unsigned flags);
+
+ /// \brief Sets up the specified scope flags and adjusts the scope state
+ /// variables accordingly.
+ ///
+ void AddFlags(unsigned Flags);
+
+ /// \brief Clears the specified scope flags and adjusts the scope state
+ /// variables accordingly.
+ ///
+ void RemoveFlags(unsigned Flags);
};
} // end namespace clang
Index: lib/Parse/ParseStmt.cpp
===================================================================
--- lib/Parse/ParseStmt.cpp
+++ lib/Parse/ParseStmt.cpp
@@ -1170,7 +1170,7 @@
// while, for, and switch statements are local to the if, while, for, or
// switch statement (including the controlled statement).
//
- unsigned ScopeFlags = Scope::BreakScope | Scope::SwitchScope;
+ unsigned ScopeFlags = Scope::SwitchScope;
if (C99orCXX)
ScopeFlags |= Scope::DeclScope | Scope::ControlScope;
ParseScope SwitchScope(this, ScopeFlags);
@@ -1208,6 +1208,7 @@
// See comments in ParseIfStatement for why we create a scope for the
// condition and a new scope for substatement in C++.
//
+ getCurScope()->AddFlags(Scope::BreakScope);
ParseScope InnerScope(this, Scope::DeclScope,
C99orCXX && Tok.isNot(tok::l_brace));
@@ -1259,12 +1260,9 @@
// while, for, and switch statements are local to the if, while, for, or
// switch statement (including the controlled statement).
//
- unsigned ScopeFlags;
+ unsigned ScopeFlags = 0;
if (C99orCXX)
- ScopeFlags = Scope::BreakScope | Scope::ContinueScope |
- Scope::DeclScope | Scope::ControlScope;
- else
- ScopeFlags = Scope::BreakScope | Scope::ContinueScope;
+ ScopeFlags = Scope::DeclScope | Scope::ControlScope;
ParseScope WhileScope(this, ScopeFlags);
// Parse the condition.
@@ -1286,6 +1284,7 @@
// See comments in ParseIfStatement for why we create a scope for the
// condition and a new scope for substatement in C++.
//
+ getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
ParseScope InnerScope(this, Scope::DeclScope,
C99orCXX && Tok.isNot(tok::l_brace));
@@ -1337,6 +1336,7 @@
// Pop the body scope if needed.
InnerScope.Exit();
+ getCurScope()->RemoveFlags(Scope::BreakScope | Scope::ContinueScope);
if (Tok.isNot(tok::kw_while)) {
if (!Body.isInvalid()) {
@@ -1419,12 +1419,9 @@
// Names declared in the for-init-statement are in the same declarative-region
// as those declared in the condition.
//
- unsigned ScopeFlags;
+ unsigned ScopeFlags = 0;
if (C99orCXXorObjC)
- ScopeFlags = Scope::BreakScope | Scope::ContinueScope |
- Scope::DeclScope | Scope::ControlScope;
- else
- ScopeFlags = Scope::BreakScope | Scope::ContinueScope;
+ ScopeFlags = Scope::DeclScope | Scope::ControlScope;
ParseScope ForScope(this, ScopeFlags);
@@ -1573,12 +1570,15 @@
}
// Parse the third part of the for specifier.
+ getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
if (Tok.isNot(tok::r_paren)) { // for (...;...;)
ExprResult Third = ParseExpression();
// FIXME: The C++11 standard doesn't actually say that this is a
// discarded-value expression, but it clearly should be.
ThirdPart = Actions.MakeFullDiscardedValueExpr(Third.take());
}
+ } else {
+ getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
}
// Match the ')'.
T.consumeClose();
Index: lib/Sema/Scope.cpp
===================================================================
--- lib/Sema/Scope.cpp
+++ lib/Sema/Scope.cpp
@@ -69,3 +69,31 @@
}
return false;
}
+
+void Scope::AddFlags(unsigned FlagsToSet) {
+ assert((FlagsToSet & ~(BreakScope | ContinueScope)) == 0 &&
+ "Unsupported scope flags");
+ if (FlagsToSet & BreakScope) {
+ assert((Flags & BreakScope) == 0 && "Already set");
+ BreakParent = this;
+ }
+ if (FlagsToSet & ContinueScope) {
+ assert((Flags & ContinueScope) == 0 && "Already set");
+ ContinueParent = this;
+ }
+ Flags |= FlagsToSet;
+}
+
+void Scope::RemoveFlags(unsigned FlagsToClear) {
+ assert((FlagsToClear & ~(BreakScope | ContinueScope)) == 0 &&
+ "Unsupported scope flags");
+ if (FlagsToClear & BreakScope) {
+ assert((Flags & BreakScope) != 0 && "Already cleared");
+ BreakParent = AnyParent->BreakParent;
+ }
+ if (FlagsToClear & ContinueScope) {
+ assert((Flags & ContinueScope) != 0 && "Already cleared");
+ ContinueParent = AnyParent->ContinueParent;
+ }
+ Flags &= ~FlagsToClear;
+}
Index: test/CodeGen/PR8880.c
===================================================================
--- /dev/null
+++ test/CodeGen/PR8880.c
@@ -0,0 +1,93 @@
+// RUN: %clang_cc1 -emit-llvm -o - %s | FileCheck %s
+
+void pr8880_cg_1(int *iptr) {
+// CHECK-LABEL: define void @pr8880_cg_1(
+// CHECK-NEXT: entry:
+ int i, j;
+ for (i = 2; i != 10 ; i++ )
+// CHECK: for.cond{{[0-9]*}}:
+// CHECK: for.body{{[0-9]*}}:
+ for (j = 3 ; j < 22; (void)({ ++j; break; j;})) {
+// CHECK: for.cond{{[0-9]*}}:
+// CHECK: for.body{{[0-9]*}}:
+ *iptr = 7;
+// CHECK: for.inc{{[0-9]*}}:
+
+// break in 3rd expression of inner loop causes branch to end of inner loop
+
+// CHECK: br label %for.end[[INNER_END:[0-9]*]]
+// CHECK: for.end[[INNER_END]]:
+ }
+// CHECK: for.inc{{[0-9]*}}:
+// CHECK: for.end{{[0-9]*}}:
+// CHECK-NEXT: ret void
+}
+
+void pr8880_cg_2(int *iptr) {
+// CHECK-LABEL: define void @pr8880_cg_2(
+// CHECK-NEXT: entry:
+ int i, j;
+ for (i = 2; i != 10 ; i++ )
+// CHECK: for.cond{{[0-9]*}}:
+// CHECK: for.body{{[0-9]*}}:
+ for (j = 3 ; j < 22; (void)({ ++j; continue; j;})) {
+// CHECK: for.cond{{[0-9]*}}:
+// CHECK: for.body{{[0-9]*}}:
+ *iptr = 7;
+// CHECK: for.inc[[INNER_INC:[0-9]*]]:
+
+// continue in 3rd expression of inner loop causes branch to inc of inner loop
+
+// CHECK: br label %for.inc[[INNER_INC]]
+// CHECK: for.end{{[0-9]*}}:
+ }
+// CHECK: for.inc{{[0-9]*}}:
+// CHECK: for.end{{[0-9]*}}:
+// CHECK-NEXT: ret void
+}
+
+void pr8880_cg_3(int *iptr) {
+// CHECK-LABEL: define void @pr8880_cg_3(
+// CHECK-NEXT: entry:
+ int i, j;
+ for (i = 2 ; i != 10 ; i++ )
+// CHECK: for.cond{{[0-9]*}}:
+// CHECK: for.body{{[0-9]*}}:
+ for (j = 3 ; ({break; j;}); j++) {
+// CHECK: for.cond{{[0-9]*}}:
+
+// break in 2nd expression of inner loop causes branch to end of outer loop
+
+// CHECK: br label %for.end[[OUTER_END:[0-9]*]]
+// CHECK: for.body{{[0-9]*}}:
+ *iptr = 7;
+// CHECK: for.inc{{[0-9]*}}:
+// CHECK: for.end{{[0-9]*}}:
+ }
+// CHECK: for.inc{{[0-9]*}}:
+// CHECK: for.end[[OUTER_END]]:
+// CHECK-NEXT: ret void
+}
+
+void pr8880_cg_4(int *iptr) {
+// CHECK-LABEL: define void @pr8880_cg_4(
+// CHECK-NEXT: entry:
+ int i, j;
+ for (i = 2 ; i != 10 ; i++ )
+// CHECK: for.cond{{[0-9]*}}:
+// CHECK: for.body{{[0-9]*}}:
+ for (j = 3 ; ({continue; j;}); j++) {
+// CHECK: for.cond{{[0-9]*}}:
+
+// continue in 2nd expression of inner loop causes branch to inc of outer loop
+
+// CHECK: br label %for.inc[[OUTER_INC:[0-9]*]]
+// CHECK: for.body{{[0-9]*}}:
+ *iptr = 7;
+// CHECK: for.inc{{[0-9]*}}:
+// CHECK: for.end{{[0-9]*}}:
+ }
+// CHECK: for.inc[[OUTER_INC:[0-9]*]]:
+// CHECK: for.end{{[0-9]*}}:
+// CHECK-NEXT: ret void
+}
Index: test/Parser/bad-control.c
===================================================================
--- test/Parser/bad-control.c
+++ test/Parser/bad-control.c
@@ -7,3 +7,135 @@
void foo2() {
continue; /* expected-error {{'continue' statement not in loop statement}} */
}
+
+int pr8880_1() {
+ int first = 1;
+ for ( ; ({ if (first) { first = 0; continue; } 0; }); ) /* expected-error {{'continue' statement not in loop statement}} */
+ return 0;
+ return 1;
+}
+
+void pr8880_2(int first) {
+ for ( ; ({ if (first) { first = 0; break; } 0; }); ) {} /* expected-error {{'break' statement not in loop or switch statement}} */
+}
+
+// GCC rejects this code
+void pr8880_3(int first) {
+ for ( ; ; (void)({ if (first) { first = 0; continue; } 0; })) {}
+}
+
+// GCC rejects this code (see also tests/Analysis/dead-stores.c rdar8014335()
+void pr8880_4(int first) {
+ for ( ; ; (void)({ if (first) { first = 0; break; } 0; })) {}
+}
+
+void pr8880_5 (int first) {
+ while(({ if (first) { first = 0; continue; } 0; })) {} /* expected-error {{'continue' statement not in loop statement}} */
+}
+
+void pr8880_6 (int first) {
+ while(({ if (first) { first = 0; break; } 0; })) {} /* expected-error {{'break' statement not in loop or switch statement}} */
+}
+
+void pr8880_7 (int first) {
+ do {} while(({ if (first) { first = 0; continue; } 0; })); /* expected-error {{'continue' statement not in loop statement}} */
+}
+
+void pr8880_8 (int first) {
+ do {} while(({ if (first) { first = 0; break; } 0; })); /* expected-error {{'break' statement not in loop or switch statement}} */
+}
+
+int pr8880_9 (int first) {
+ switch(({ if (first) { first = 0; break; } 1; })) { /* expected-error {{'break' statement not in loop or switch statement}} */
+ case 2: return 2;
+ default: return 0;
+ }
+}
+
+void pr8880_10(int i) {
+ for ( ; i != 10 ; i++ )
+ for ( ; ; (void)({ ++i; continue; i;})) {}
+}
+
+void pr8880_11(int i) {
+ for ( ; i != 10 ; i++ )
+ for ( ; ; (void)({ ++i; break; i;})) {}
+}
+
+void pr8880_12(int i, int j) {
+ for ( ; i != 10 ; i++ )
+ for ( ; ({if (i) continue; i;}); j++) {}
+}
+
+void pr8880_13(int i, int j) {
+ for ( ; i != 10 ; i++ )
+ for ( ; ({if (i) break; i;}); j++) {}
+}
+
+void pr8880_14(int i) {
+ for ( ; i != 10 ; i++ )
+ while(({if (i) break; i;})) {}
+}
+
+void pr8880_15(int i) {
+ while (--i)
+ while(({if (i) continue; i;})) {}
+}
+
+void pr8880_16(int i) {
+ for ( ; i != 10 ; i++ )
+ do {} while(({if (i) break; i;}));
+}
+
+void pr8880_17(int i) {
+ for ( ; i != 10 ; i++ )
+ do {} while(({if (i) continue; i;}));
+}
+
+void pr8880_18(int x, int y) {
+ while(x > 0)
+ switch(({if(y) break; y;})) {
+ case 2: x = 0;
+ }
+}
+
+void pr8880_19(int x, int y) {
+ switch(x) {
+ case 1:
+ switch(({if(y) break; y;})) {
+ case 2: x = 0;
+ }
+ }
+}
+
+void pr8880_20(int x, int y) {
+ switch(x) {
+ case 1:
+ while(({if (y) break; y;})) {}
+ }
+}
+
+void pr8880_21(int x, int y) {
+ switch(x) {
+ case 1:
+ do {} while(({if (y) break; y;}));
+ }
+}
+
+void pr8880_22(int x, int y) {
+ switch(x) {
+ case 1:
+ for ( ; ; (void)({ ++y; break; y;})) {}
+ }
+}
+
+void pr8880_23(int x, int y) {
+ switch(x) {
+ case 1:
+ for ( ; ({ ++y; break; y;}); ++y) {}
+ }
+}
+
+void pr8880_24() {
+ for (({break;});;); /* expected-error {{'break' statement not in loop or switch statement}} */
+}
Index: test/Sema/statements.c
===================================================================
--- test/Sema/statements.c
+++ test/Sema/statements.c
@@ -90,12 +90,9 @@
}
}
-// PR 8880
-// FIXME: Clang should reject this, since GCC does. Previously this
-// was causing a crash in the CFG builder.
int test_pr8880() {
int first = 1;
- for ( ; ({ if (first) { first = 0; continue; } 0; }); )
+ for ( ; ({ if (first) { first = 0; continue; } 0; }); ) // expected-error {{'continue' statement not in loop statement}}
return 0;
return 1;
}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
