sepavloff added you to the CC list for the revision "Fix to PR8880 (clang dies
processing a for loop).".
Due to statement expressions supported as GCC extension, it is possible
to put 'break' or 'continue' into a loop/switch statement but outside
its body, for example:
for ( ; ({ if (first) { first = 0; continue; } 0; }); )
Such usage must be diagnosed as an error, GCC rejects it.
To recognize such patterns flags BreakScope and ContinueScope are
temporarily turned off while parsing condition expression.
http://llvm-reviews.chandlerc.com/D2018
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Parse/Parser.h
include/clang/Sema/Scope.h
lib/Parse/ParseStmt.cpp
lib/Parse/Parser.cpp
lib/Sema/Scope.cpp
test/Parser/bad-control.c
test/Sema/statements.c
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -6238,9 +6238,9 @@
// Statements.
def err_continue_not_in_loop : Error<
- "'continue' statement not in loop statement">;
+ "'continue' statement not in loop statement body">;
def err_break_not_in_loop_or_switch : Error<
- "'break' statement not in loop or switch statement">;
+ "'break' statement not in loop or switch statement body">;
def err_default_not_in_switch : Error<
"'default' statement not in switch statement">;
def err_case_not_in_switch : Error<"'case' statement not in switch statement">;
Index: include/clang/Parse/Parser.h
===================================================================
--- include/clang/Parse/Parser.h
+++ include/clang/Parse/Parser.h
@@ -696,6 +696,22 @@
}
}
+ /// \brief Sets the specified flags in this scope.
+ /// \param Flags Set of or'd flags (specified in Scope::ScopeFlags) that
+ /// must be set.
+ void SetFlags(unsigned Flags) {
+ if (Self)
+ Self->SetScopeFlags(Flags);
+ }
+
+ /// \brief Clear the specified flags in this scope.
+ /// \param Flags Set of or'd flags (specified in Scope::ScopeFlags) that
+ /// must be cleared.
+ void ClearFlags(unsigned Flags) {
+ if (Self)
+ Self->ClearScopeFlags(Flags);
+ }
+
~ParseScope() {
Exit();
}
@@ -707,6 +723,12 @@
/// ExitScope - Pop a scope off the scope stack.
void ExitScope();
+ /// \brief Sets the specified flags in the current scope.
+ void SetScopeFlags(unsigned Flags);
+
+ /// \brief Clears the specified flags in the current scope.
+ void ClearScopeFlags(unsigned Flags);
+
private:
/// \brief RAII object used to modify the scope flags for the current scope.
class ParseScopeFlags {
Index: include/clang/Sema/Scope.h
===================================================================
--- include/clang/Sema/Scope.h
+++ include/clang/Sema/Scope.h
@@ -342,6 +342,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 SetFlags(unsigned Flags);
+
+ /// \brief Clears the specified scope flags and adjusts the scope state
+ /// variables accordingly.
+ ///
+ void ClearFlags(unsigned Flags);
};
} // end namespace clang
Index: lib/Parse/ParseStmt.cpp
===================================================================
--- lib/Parse/ParseStmt.cpp
+++ lib/Parse/ParseStmt.cpp
@@ -1135,6 +1135,9 @@
ScopeFlags |= Scope::DeclScope | Scope::ControlScope;
ParseScope SwitchScope(this, ScopeFlags);
+ // Temporarily disable 'break' while parsing condition.
+ SwitchScope.ClearFlags(Scope::BreakScope);
+
// Parse the condition.
ExprResult Cond;
Decl *CondVar = 0;
@@ -1157,6 +1160,9 @@
return Switch;
}
+ // Enable 'break' in the body of switch statement.
+ SwitchScope.SetFlags(Scope::BreakScope);
+
// C99 6.8.4p3 - In C99, the body of the switch statement is a scope, even if
// there is no compound stmt. C90 does not have this clause. We only do this
// if the body isn't a compound statement to avoid push/pop in common cases.
@@ -1227,14 +1233,20 @@
ScopeFlags = Scope::BreakScope | Scope::ContinueScope;
ParseScope WhileScope(this, ScopeFlags);
+ // Disable 'break' and 'continue' while parsing condition.
+ WhileScope.ClearFlags(Scope::BreakScope | Scope::ContinueScope);
+
// Parse the condition.
ExprResult Cond;
Decl *CondVar = 0;
if (ParseParenExprOrCondition(Cond, CondVar, WhileLoc, true))
return StmtError();
FullExprArg FullCond(Actions.MakeFullExpr(Cond.get(), WhileLoc));
+ // Allow 'break' and 'continue' in the body of statement.
+ WhileScope.SetFlags(Scope::BreakScope | Scope::ContinueScope);
+
// C99 6.8.5p5 - In C99, the body of the if statement is a scope, even if
// there is no compound stmt. C90 does not have this clause. We only do this
// if the body isn't a compound statement to avoid push/pop in common cases.
@@ -1314,6 +1326,9 @@
return StmtError();
}
+ // Do not allow 'break' and 'continue' in 'while' condition expression.
+ DoScope.ClearFlags(Scope::BreakScope | Scope::ContinueScope);
+
// Parse the parenthesized expression.
BalancedDelimiterTracker T(*this, tok::l_paren);
T.consumeOpen();
@@ -1391,6 +1406,10 @@
BalancedDelimiterTracker T(*this, tok::l_paren);
T.consumeOpen();
+ // Until loop body starts, statements 'break' and 'continue' cannot
+ // be used.
+ ForScope.ClearFlags(Scope::BreakScope | Scope::ContinueScope);
+
ExprResult Value;
bool ForEach = false, ForRange = false;
@@ -1533,6 +1552,7 @@
}
// Parse the third part of the for specifier.
+ ForScope.SetFlags(Scope::BreakScope);
if (Tok.isNot(tok::r_paren)) { // for (...;...;)
ExprResult Third = ParseExpression();
// FIXME: The C++11 standard doesn't actually say that this is a
@@ -1566,6 +1586,9 @@
T.getCloseLocation());
}
+ // Allow 'break' and 'continue' in the loop body.
+ ForScope.SetFlags(Scope::BreakScope | Scope::ContinueScope);
+
// C99 6.8.5p5 - In C99, the body of the if statement is a scope, even if
// there is no compound stmt. C90 does not have this clause. We only do this
// if the body isn't a compound statement to avoid push/pop in common cases.
Index: lib/Parse/Parser.cpp
===================================================================
--- lib/Parse/Parser.cpp
+++ lib/Parse/Parser.cpp
@@ -390,6 +390,15 @@
ScopeCache[NumCachedScopes++] = OldScope;
}
+void Parser::SetScopeFlags(unsigned Flags) {
+ Actions.CurScope->SetFlags(Flags);
+}
+
+void Parser::ClearScopeFlags(unsigned Flags) {
+ Actions.CurScope->ClearFlags(Flags);
+}
+
+
/// Set the flags for the current scope to ScopeFlags. If ManageFlags is false,
/// this object does nothing.
Parser::ParseScopeFlags::ParseScopeFlags(Parser *Self, unsigned ScopeFlags,
Index: lib/Sema/Scope.cpp
===================================================================
--- lib/Sema/Scope.cpp
+++ lib/Sema/Scope.cpp
@@ -69,3 +69,40 @@
}
return false;
}
+
+void Scope::SetFlags(unsigned FlagsToSet) {
+ assert((FlagsToSet & ~(BreakScope | ContinueScope)) == 0 ||
+ "Unsupported scope flags");
+ assert ((Flags & ControlScope) != 0 || "Must be control scope");
+ 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::ClearFlags(unsigned FlagsToClear) {
+ assert((FlagsToClear & ~(BreakScope | ContinueScope)) == 0 ||
+ "Unsupported scope flags");
+ if (FlagsToClear & BreakScope) {
+ assert((Flags & ControlScope) != 0 || "Must be control scope");
+ assert((Flags & BreakScope) != 0 || "Already cleared");
+ // This is a loop or switch scope. Flag BreakScope is removed temporarily
+ // when parsing the loop or switch header, to prevent constructs like this:
+ // \code
+ // while (({ if(a>N) break; a}))
+ // \endcode
+ BreakParent = 0;
+ }
+ if (FlagsToClear & ContinueScope) {
+ assert ((Flags & ControlScope) != 0 || "Must be control scope");
+ assert((Flags & ContinueScope) != 0 || "Already cleared");
+ ContinueParent = 0;
+ }
+ Flags &= ~FlagsToClear;
+}
+
Index: test/Parser/bad-control.c
===================================================================
--- test/Parser/bad-control.c
+++ test/Parser/bad-control.c
@@ -7,3 +7,80 @@
void foo2() {
continue; /* expected-error {{'continue' statement not in loop statement}} */
}
+
+int pr8880() {
+ int first = 1;
+ for ( ; ({ if (first) { first = 0; continue; } 0; }); ) /* expected-error {{'continue' statement not in loop statement}} */
+ return 0;
+ return 1;
+}
+
+int pr8880_2 (int a) {
+ int first = a;
+ while(({ if (first) { first = 0; continue; } 0; })) /* expected-error {{'continue' statement not in loop statement}} */
+ return a;
+}
+
+int pr8880_3 (int a) {
+ int first = a;
+ while(({ if (first) { first = 0; break; } 0; })) /* expected-error {{'break' statement not in loop or switch statement}} */
+ return a;
+}
+
+int pr8880_4 (int a) {
+ int first = a;
+ do {
+ return a;
+ } while(({ if (first) { first = 0; continue; } 0; })); /* expected-error {{'continue' statement not in loop statement}} */
+}
+
+int pr8880_5 (int a) {
+ int first = a;
+ do {
+ return a;
+ } while(({ if (first) { first = 0; break; } 0; })); /* expected-error {{'break' statement not in loop or switch statement}} */
+}
+
+int pr8880_6 (int a) {
+ int first = a;
+ switch(({ if (first) { first = 0; break; } a; })) { /* expected-error {{'break' statement not in loop or switch statement}} */
+ case 2: return a;
+ default: return 0;
+ }
+ return 1;
+}
+
+void pr8880_7() {
+ for (int i = 0 ; i != 10 ; i++ ) {
+ for ( ; ; ({ ++i; continue; })) { // expected-error {{'continue' statement not in loop statement}}
+ }
+ }
+}
+
+// Have to allow 'break' in the third part of 'for' specifier to enable compilation of QT macro 'foreach'
+void pr8880_8() {
+ for (int i = 0 ; i != 10 ; i++ )
+ for ( ; ; ({ ++i; break; })) {
+ }
+}
+
+void pr8880_9(int x, int y) {
+ switch(x) {
+ case 1:
+ while(({if (y) break; y;})) {} // expected-error {{'break' statement not in loop or switch statement}}
+ }
+}
+
+void pr8880_10(int x, int y) {
+ while(x > 0) {
+ switch(({if(y) break; y;})) { // expected-error {{'break' statement not in loop or switch statement}}
+ case 2: x=0;
+ }
+ }
+}
+
+void pr8880_11() {
+ for (int i = 0 ; i != 10 ; i++ ) {
+ while(({if (i) break; i;})) {} // expected-error {{'break' statement not in loop or switch statement}}
+ }
+}
Index: test/Sema/statements.c
===================================================================
--- test/Sema/statements.c
+++ test/Sema/statements.c
@@ -95,7 +95,7 @@
// 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
