From d661d50118716e9695af5a893a2df45e87a6b3c8 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Tue, 22 Oct 2013 18:07:04 +0000 Subject: [PATCH] Revert r193073 and the attempt to fix it in r193170. This patch wasn't reviewed, and isn't correctly preserving the behaviors relied upon by QT. I don't have a direct example of fallout, but it should go through the standard code review process. For example, it should never have removed the QT test case that was added when fixing those users. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@193174 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 4 +- include/clang/Parse/Parser.h | 22 ------ include/clang/Sema/Scope.h | 10 --- lib/Parse/ParseStmt.cpp | 26 ------ lib/Parse/Parser.cpp | 9 --- lib/Sema/Scope.cpp | 37 --------- test/Analysis/dead-stores.c | 12 +++ test/Parser/bad-control.c | 92 ---------------------- test/Sema/statements.c | 2 +- 9 files changed, 15 insertions(+), 199 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 415c4b2963..92180e6051 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -6227,9 +6227,9 @@ def warn_cfstring_truncated : Warning< // Statements. def err_continue_not_in_loop : Error< - "'continue' statement not in loop statement body">; + "'continue' statement not in loop statement">; def err_break_not_in_loop_or_switch : Error< - "'break' statement not in loop or switch statement body">; + "'break' statement not in loop or switch statement">; 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">; diff --git a/include/clang/Parse/Parser.h b/include/clang/Parse/Parser.h index ece532f059..1fd1d866b2 100644 --- a/include/clang/Parse/Parser.h +++ b/include/clang/Parse/Parser.h @@ -696,22 +696,6 @@ public: } } - /// \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(); } @@ -723,12 +707,6 @@ public: /// 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 { diff --git a/include/clang/Sema/Scope.h b/include/clang/Sema/Scope.h index 4b0563ed54..249a4c7431 100644 --- a/include/clang/Sema/Scope.h +++ b/include/clang/Sema/Scope.h @@ -342,16 +342,6 @@ public: /// 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 diff --git a/lib/Parse/ParseStmt.cpp b/lib/Parse/ParseStmt.cpp index fe884148eb..f57ff97ceb 100644 --- a/lib/Parse/ParseStmt.cpp +++ b/lib/Parse/ParseStmt.cpp @@ -1135,9 +1135,6 @@ StmtResult Parser::ParseSwitchStatement(SourceLocation *TrailingElseLoc) { 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; @@ -1160,9 +1157,6 @@ StmtResult Parser::ParseSwitchStatement(SourceLocation *TrailingElseLoc) { 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. @@ -1233,9 +1227,6 @@ StmtResult Parser::ParseWhileStatement(SourceLocation *TrailingElseLoc) { 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; @@ -1244,9 +1235,6 @@ StmtResult Parser::ParseWhileStatement(SourceLocation *TrailingElseLoc) { FullExprArg FullCond(Actions.MakeFullExpr(Cond.get(), WhileLoc)); - // Allow 'break' and 'continue' in the body of the 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. @@ -1326,9 +1314,6 @@ StmtResult Parser::ParseDoStatement() { 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(); @@ -1406,10 +1391,6 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) { 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; @@ -1553,10 +1534,6 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) { // Parse the third part of the for specifier. if (Tok.isNot(tok::r_paren)) { // for (...;...;) - // This is needed to compile QT 4.8.4, which uses statement - // expression with 'break' in it. - ForScope.SetFlags(Scope::BreakScope); - ExprResult Third = ParseExpression(); // FIXME: The C++11 standard doesn't actually say that this is a // discarded-value expression, but it clearly should be. @@ -1589,9 +1566,6 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) { T.getCloseLocation()); } - // When parsing body of 'for' statement, 'break' and 'continue' may be used. - 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. diff --git a/lib/Parse/Parser.cpp b/lib/Parse/Parser.cpp index 2e1ace53ea..9b6c97ac19 100644 --- a/lib/Parse/Parser.cpp +++ b/lib/Parse/Parser.cpp @@ -390,15 +390,6 @@ void Parser::ExitScope() { 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, diff --git a/lib/Sema/Scope.cpp b/lib/Sema/Scope.cpp index 8c2a8b3773..10f12ce844 100644 --- a/lib/Sema/Scope.cpp +++ b/lib/Sema/Scope.cpp @@ -69,40 +69,3 @@ bool Scope::containedInPrototypeScope() const { } 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; -} - diff --git a/test/Analysis/dead-stores.c b/test/Analysis/dead-stores.c index 43f305bec6..067a0504f1 100644 --- a/test/Analysis/dead-stores.c +++ b/test/Analysis/dead-stores.c @@ -476,6 +476,18 @@ int f26_nestedblocks() { return y; } +// The FOREACH macro in QT uses 'break' statements within statement expressions +// placed within the increment code of for loops. +void rdar8014335() { + for (int i = 0 ; i != 10 ; ({ break; })) { + for ( ; ; ({ ++i; break; })) ; + // Note that the next value stored to 'i' is never executed + // because the next statement to be executed is the 'break' + // in the increment code of the first loop. + i = i * 3; // expected-warning{{Value stored to 'i' is never read}} expected-warning{{The left operand to '*' is always 1}} + } +} + // NullStmts followed by do...while() can lead to disconnected CFG // // This previously caused bogus dead-stores warnings because the body of the first do...while was diff --git a/test/Parser/bad-control.c b/test/Parser/bad-control.c index 9560af3259..480d81be0d 100644 --- a/test/Parser/bad-control.c +++ b/test/Parser/bad-control.c @@ -7,95 +7,3 @@ void foo() { 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 4.8 macro 'foreach' -void pr17649() { - 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}} - } -} - -// Moved from Analysis/dead-stores.c -void rdar8014335() { - for (int i = 0 ; i != 10 ; ({ break; })) { - for ( ; ; ({ ++i; break; })) ; - i = i * 3; - } -} - -void pr17649_2() { - for (int i = 0 ; i != 10 ; ({ continue; })) { // expected-error {{'continue' statement not in loop statement}} - for ( ; ; ({ ++i; continue; })) ; // expected-error {{'continue' statement not in loop statement}} - i = i * 3; - } -} diff --git a/test/Sema/statements.c b/test/Sema/statements.c index 1b84f4bf7c..f01ee40867 100644 --- a/test/Sema/statements.c +++ b/test/Sema/statements.c @@ -95,7 +95,7 @@ void foo(enum x X) { // was causing a crash in the CFG builder. int test_pr8880() { int first = 1; - for ( ; ({ if (first) { first = 0; continue; } 0; }); ) // expected-error {{'continue' statement not in loop statement}} + for ( ; ({ if (first) { first = 0; continue; } 0; }); ) return 0; return 1; } -- 2.40.0