r193174 - Revert r193073 and the attempt to fix it in r193170.

Chandler Carruth chandlerc at gmail.com
Tue Oct 22 11:07:05 PDT 2013


Author: chandlerc
Date: Tue Oct 22 13:07:04 2013
New Revision: 193174

URL: http://llvm.org/viewvc/llvm-project?rev=193174&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/include/clang/Parse/Parser.h
    cfe/trunk/include/clang/Sema/Scope.h
    cfe/trunk/lib/Parse/ParseStmt.cpp
    cfe/trunk/lib/Parse/Parser.cpp
    cfe/trunk/lib/Sema/Scope.cpp
    cfe/trunk/test/Analysis/dead-stores.c
    cfe/trunk/test/Parser/bad-control.c
    cfe/trunk/test/Sema/statements.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=193174&r1=193173&r2=193174&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Oct 22 13:07:04 2013
@@ -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">;

Modified: cfe/trunk/include/clang/Parse/Parser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=193174&r1=193173&r2=193174&view=diff
==============================================================================
--- cfe/trunk/include/clang/Parse/Parser.h (original)
+++ cfe/trunk/include/clang/Parse/Parser.h Tue Oct 22 13:07:04 2013
@@ -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 {

Modified: cfe/trunk/include/clang/Sema/Scope.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Scope.h?rev=193174&r1=193173&r2=193174&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Scope.h (original)
+++ cfe/trunk/include/clang/Sema/Scope.h Tue Oct 22 13:07:04 2013
@@ -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

Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=193174&r1=193173&r2=193174&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Tue Oct 22 13:07:04 2013
@@ -1135,9 +1135,6 @@ StmtResult Parser::ParseSwitchStatement(
     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(
     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(S
     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(S
 
   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(Sou
   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(Sou
 
     // 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(Sou
                                                      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.

Modified: cfe/trunk/lib/Parse/Parser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=193174&r1=193173&r2=193174&view=diff
==============================================================================
--- cfe/trunk/lib/Parse/Parser.cpp (original)
+++ cfe/trunk/lib/Parse/Parser.cpp Tue Oct 22 13:07:04 2013
@@ -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,

Modified: cfe/trunk/lib/Sema/Scope.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Scope.cpp?rev=193174&r1=193173&r2=193174&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Scope.cpp (original)
+++ cfe/trunk/lib/Sema/Scope.cpp Tue Oct 22 13:07:04 2013
@@ -69,40 +69,3 @@ bool Scope::containedInPrototypeScope()
   }
   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;
-}
-

Modified: cfe/trunk/test/Analysis/dead-stores.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dead-stores.c?rev=193174&r1=193173&r2=193174&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/dead-stores.c (original)
+++ cfe/trunk/test/Analysis/dead-stores.c Tue Oct 22 13:07:04 2013
@@ -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}}
+  }
+}
+
 // <rdar://problem/8320674> 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

Modified: cfe/trunk/test/Parser/bad-control.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/bad-control.c?rev=193174&r1=193173&r2=193174&view=diff
==============================================================================
--- cfe/trunk/test/Parser/bad-control.c (original)
+++ cfe/trunk/test/Parser/bad-control.c Tue Oct 22 13:07:04 2013
@@ -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;
-  }
-}

Modified: cfe/trunk/test/Sema/statements.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/statements.c?rev=193174&r1=193173&r2=193174&view=diff
==============================================================================
--- cfe/trunk/test/Sema/statements.c (original)
+++ cfe/trunk/test/Sema/statements.c Tue Oct 22 13:07:04 2013
@@ -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;
 }





More information about the cfe-commits mailing list