Fix to PR8880 (clang dies processing a for loop).

[sepavloff]

Summary

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. With this
change clang interprets such constructs as GCC in C++ mode: 'break'
and 'continue' in the 3rd expression refer to the loop itself, in
the 1st and 2nd expressions - to outer loop.

This revision is reincarnation of http://llvm-reviews.chandlerc.com/D2018, which was accidentally closed.

[sepavloff]

Updated patch according to notes of Justin Bogner.

[sepavloff]

Updated test for code generation.

[sepavloff]

Changed implementation according to reviewer's notes.

Code generation is changed so that condition and increment expressions
in loop header are considered as if they are inside break/continue
scope of the loop. Such behavior is more close to GCC in C++ mode.
In C mode such usage causes GCC compatibility warning.

[rsmith]

Thanks, this looks good.

include/clang/Basic/DiagnosticSemaKinds.td

6336 ↗

(On Diff #6518)

... "to the enclosing loop" might be clearer.

test/CodeGen/PR8880.c

8 ↗

(On Diff #6518)

This test will not work with a Clang built without asserts, because label names are not emitted into the text form of the IR. If you need to match label names, run the IR through opt -instnamer -S before piping it into FileCheck (see test/CodeGenCXX/for-range.cpp for an example).

test/Sema/loop-control.cpp

1 ↗

(On Diff #6518)

Can you fold this and loop-control.c into the same source file? (Maybe use -verify for the C test and -Werror for this one.)

[sepavloff]

Closed by commit rL199897 (authored by @sepavloff).

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def warn_cfstring_truncated : Warning<
   "belong to the input codeset UTF-8">,
   InGroup<DiagGroup<"CFString-literal">>;
 
 // Statements.
 def err_continue_not_in_loop : Error<
   "'continue' statement not in loop statement">;
 def err_break_not_in_loop_or_switch : Error<
   "'break' statement not in loop or switch statement">;
+def warn_loop_ctrl_binds_to_inner : Warning<
+  "'%0' is bound to current loop, GCC binds it to the enclosing loop">,
+  InGroup<GccCompat>;
+def warn_break_binds_to_switch : Warning<
+  "'break' is bound to loop, GCC binds it to switch">,
+  InGroup<GccCompat>;
 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">;
 def warn_bool_switch_condition : Warning<
   "switch condition has boolean value">;
 def warn_case_value_overflow : Warning<
   "overflow converting case value to switch condition type (%0 to %1)">,
   InGroup<Switch>;

cfe/trunk/include/clang/Sema/Scope.h

public:
 
   const_udir_iterator using_directives_end() const {
     return UsingDirectives.end();
   }
 
   /// 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);
 };
 
 }  // end namespace clang
 
 #endif

cfe/trunk/include/clang/Sema/Sema.h

private:
   /// \brief Perform semantic checks on a completed expression. This will either
   /// be a full-expression or a default argument expression.
   void CheckCompletedExpr(Expr *E, SourceLocation CheckLoc = SourceLocation(),
                           bool IsConstexpr = false);
 
   void CheckBitFieldInitialization(SourceLocation InitLoc, FieldDecl *Field,
                                    Expr *Init);
 
+  /// \brief Check if the given expression contains 'break' or 'continue'
+  /// statement that produces control flow different from GCC.
+  void CheckBreakContinueBinding(Expr *E);
+
 public:
   /// \brief Register a magic integral constant to be used as a type tag.
   void RegisterTypeTagForDatatype(const IdentifierInfo *ArgumentKind,
                                   uint64_t MagicValue, QualType Type,
                                   bool LayoutCompatible, bool MustBeNull);
 
   struct TypeTagData {
     TypeTagData() {}

cfe/trunk/lib/CodeGen/CGStmt.cpp

void CodeGenFunction::EmitDoStmt(const DoStmt &S) {
   EmitBlock(LoopBody);
   Cnt.beginRegion(Builder);
   {
     RunCleanupsScope BodyScope(*this);
     EmitStmt(S.getBody());
   }
   Cnt.adjustForControlFlow();
 
-  BreakContinueStack.pop_back();
-
   EmitBlock(LoopCond.getBlock());
 
   // C99 6.8.5.2: "The evaluation of the controlling expression takes place
   // after each execution of the loop body."
 
   // Evaluate the conditional in the while header.
   // C99 6.8.5p2/p4: The first substatement is executed if the expression
   // compares unequal to 0.  The condition must be a scalar type.
   llvm::Value *BoolCondVal = EvaluateExprAsBool(S.getCond());
 
+  BreakContinueStack.pop_back();
+
   // "do {} while (0)" is common in macros, avoid extra blocks.  Be sure
   // to correctly handle break/continue though.
   bool EmitBoolCondBranch = true;
   if (llvm::ConstantInt *C = dyn_cast<llvm::ConstantInt>(BoolCondVal))
     if (C->isZero())
       EmitBoolCondBranch = false;
 
   uint64_t LoopCount = Cnt.getCount() - Cnt.getParentCount();
void CodeGenFunction::EmitForStmt(const ForStmt &S) {
 
   // Start the loop with a block that tests the condition.
   // If there's an increment, the continue scope will be overwritten
   // later.
   JumpDest Continue = getJumpDestInCurrentScope("for.cond");
   llvm::BasicBlock *CondBlock = Continue.getBlock();
   EmitBlock(CondBlock);
 
+  // If the for loop doesn't have an increment we can just use the
+  // condition as the continue block.  Otherwise we'll need to create
+  // a block for it (in the current scope, i.e. in the scope of the
+  // condition), and that we will become our continue block.
+  if (S.getInc())
+    Continue = getJumpDestInCurrentScope("for.inc");
+
+  // Store the blocks to use for break and continue.
+  BreakContinueStack.push_back(BreakContinue(LoopExit, Continue, &Cnt));
+
   // Create a cleanup scope for the condition variable cleanups.
   RunCleanupsScope ConditionScope(*this);
 
   llvm::BranchInst *CondBr = NULL;
   if (S.getCond()) {
     // If the for statement has a condition scope, emit the local variable
     // declaration.
     if (S.getConditionVariable()) {
if (S.getCond()) {
 
     EmitBlock(ForBody);
   } else {
     // Treat it as a non-zero constant.  Don't even create a new block for the
     // body, just fall into it.
   }
   Cnt.beginRegion(Builder);
 
-  // If the for loop doesn't have an increment we can just use the
-  // condition as the continue block.  Otherwise we'll need to create
-  // a block for it (in the current scope, i.e. in the scope of the
-  // condition), and that we will become our continue block.
-  if (S.getInc())
-    Continue = getJumpDestInCurrentScope("for.inc");
-
-  // Store the blocks to use for break and continue.
-  BreakContinueStack.push_back(BreakContinue(LoopExit, Continue, &Cnt));
-
   {
     // Create a separate cleanup scope for the body, in case it is not
     // a compound statement.
     RunCleanupsScope BodyScope(*this);
     EmitStmt(S.getBody());
   }
 
   // If there is an increment, emit it next.

cfe/trunk/lib/Parse/ParseStmt.cpp

StmtResult Parser::ParseSwitchStatement(SourceLocation *TrailingElseLoc) {
   // A name introduced by a declaration in a condition is in scope from its
   // point of declaration until the end of the substatements controlled by the
   // condition.
   // C++ 3.3.2p4:
   // Names declared in the for-init-statement, and in the condition of if,
   // 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);
 
   // Parse the condition.
   ExprResult Cond;
   Decl *CondVar = 0;
   if (ParseParenExprOrCondition(Cond, CondVar, SwitchLoc, false))
StmtResult Parser::ParseSwitchStatement(SourceLocation *TrailingElseLoc) {
   //
   // C++ 6.4p1:
   // The substatement in a selection-statement (each substatement, in the else
   // form of the if statement) implicitly defines a local scope.
   //
   // 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));
 
   // Read the body statement.
   StmtResult Body(ParseStatement(TrailingElseLoc));
 
   // Pop the scopes.
   InnerScope.Exit();
StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
   // C++ 3.3.2p4:
   // Names declared in the for-init-statement, and in the condition of if,
   // while, for, and switch statements are local to the if, while, for, or
   // switch statement (including the controlled statement).
   // C++ 6.5.3p1:
   // 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);
 
   BalancedDelimiterTracker T(*this, tok::l_paren);
   T.consumeOpen();
 
   ExprResult Value;
 
if (Tok.is(tok::semi)) {
       } else {
         // Skip until semicolon or rparen, don't consume it.
         SkipUntil(tok::r_paren, StopAtSemi | StopBeforeMatch);
         if (Tok.is(tok::semi))
           ConsumeToken();
       }
     }
   }
+
+  // Parse the second part of the for specifier.
+  getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
   if (!ForEach && !ForRange) {
     assert(!SecondPart.get() && "Shouldn't have a second expression yet.");
     // Parse the second part of the for specifier.
     if (Tok.is(tok::semi)) {  // for (...;;
       // no second part.
     } else if (Tok.is(tok::r_paren)) {
       // missing both semicolons.
     } else {

cfe/trunk/lib/Sema/Scope.cpp

bool Scope::containedInPrototypeScope() const {
   const Scope *S = this;
   while (S) {
     if (S->isFunctionPrototypeScope())
       return true;
     S = S->getParent();
   }
   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;
+}
+

cfe/trunk/lib/Sema/SemaStmt.cpp

if (CondVar) {
     ConditionVar = cast<VarDecl>(CondVar);
     CondResult = CheckConditionVariable(ConditionVar, WhileLoc, true);
     if (CondResult.isInvalid())
       return StmtError();
   }
   Expr *ConditionExpr = CondResult.take();
   if (!ConditionExpr)
     return StmtError();
+  CheckBreakContinueBinding(ConditionExpr);
 
   DiagnoseUnusedExprResult(Body);
 
   if (isa<NullStmt>(Body))
     getCurCompoundScope().setHasEmptyLoopBodies();
 
   return Owned(new (Context) WhileStmt(Context, ConditionVar, ConditionExpr,
                                        Body, WhileLoc));
 }
 
 StmtResult
 Sema::ActOnDoStmt(SourceLocation DoLoc, Stmt *Body,
                   SourceLocation WhileLoc, SourceLocation CondLParen,
                   Expr *Cond, SourceLocation CondRParen) {
   assert(Cond && "ActOnDoStmt(): missing expression");
 
+  CheckBreakContinueBinding(Cond);
   ExprResult CondResult = CheckBooleanCondition(Cond, DoLoc);
   if (CondResult.isInvalid())
     return StmtError();
   Cond = CondResult.take();
 
   CondResult = ActOnFinishFullExpr(Cond, DoLoc);
   if (CondResult.isInvalid())
     return StmtError();
if (CXXOperatorCallExpr *Call = dyn_cast<CXXOperatorCallExpr>(Statement)) {
       }
       DRE = dyn_cast<DeclRefExpr>(Call->getArg(0));
       return DRE;
     }
 
     return false;
   }
 
-  // A visitor to determine if a continue statement is a subexpression.
-  class ContinueFinder : public EvaluatedExprVisitor<ContinueFinder> {
-    bool Found;
+  // A visitor to determine if a continue or break statement is a
+  // subexpression.
+  class BreakContinueFinder : public EvaluatedExprVisitor<BreakContinueFinder> {
+    SourceLocation BreakLoc;
+    SourceLocation ContinueLoc;
   public:
-    ContinueFinder(Sema &S, Stmt* Body) :
-        Inherited(S.Context),
-        Found(false) {
+    BreakContinueFinder(Sema &S, Stmt* Body) :
+        Inherited(S.Context) {
       Visit(Body);
     }
 
-    typedef EvaluatedExprVisitor<ContinueFinder> Inherited;
+    typedef EvaluatedExprVisitor<BreakContinueFinder> Inherited;
 
     void VisitContinueStmt(ContinueStmt* E) {
-      Found = true;
+      ContinueLoc = E->getContinueLoc();
+    }
+
+    void VisitBreakStmt(BreakStmt* E) {
+      BreakLoc = E->getBreakLoc();
     }
 
-    bool ContinueFound() { return Found; }
+    bool ContinueFound() { return ContinueLoc.isValid(); }
+    bool BreakFound() { return BreakLoc.isValid(); }
+    SourceLocation GetContinueLoc() { return ContinueLoc; }
+    SourceLocation GetBreakLoc() { return BreakLoc; }
 
-  };  // end class ContinueFinder
+  };  // end class BreakContinueFinder
 
   // Emit a warning when a loop increment/decrement appears twice per loop
   // iteration.  The conditions which trigger this warning are:
   // 1) The last statement in the loop body and the third expression in the
   //    for loop are both increment or both decrement of the same variable
   // 2) No continue statements in the loop body.
   void CheckForRedundantIteration(Sema &S, Expr *Third, Stmt *Body) {
     // Return when there is nothing to check.
void CheckForRedundantIteration(Sema &S, Expr *Third, Stmt *Body) {
 
     bool LoopIncrement, LastIncrement;
     DeclRefExpr *LoopDRE, *LastDRE;
 
     if (!ProcessIterationStmt(S, Third, LoopIncrement, LoopDRE)) return;
     if (!ProcessIterationStmt(S, LastStmt, LastIncrement, LastDRE)) return;
 
     // Check that the two statements are both increments or both decrements
-    // on the same varaible.
+    // on the same variable.
     if (LoopIncrement != LastIncrement ||
         LoopDRE->getDecl() != LastDRE->getDecl()) return;
 
-    if (ContinueFinder(S, Body).ContinueFound()) return;
+    if (BreakContinueFinder(S, Body).ContinueFound()) return;
 
     S.Diag(LastDRE->getLocation(), diag::warn_redundant_loop_iteration)
          << LastDRE->getDecl() << LastIncrement;
     S.Diag(LoopDRE->getLocation(), diag::note_loop_iteration_here)
          << LoopIncrement;
   }
 
 } // end namespace
 
+
+void Sema::CheckBreakContinueBinding(Expr *E) {
+  if (!E || getLangOpts().CPlusPlus)
+    return;
+  BreakContinueFinder BCFinder(*this, E);
+  Scope *BreakParent = CurScope->getBreakParent();
+  if (BCFinder.BreakFound() && BreakParent) {
+    if (BreakParent->getFlags() & Scope::SwitchScope) {
+      Diag(BCFinder.GetBreakLoc(), diag::warn_break_binds_to_switch);
+    } else {
+      Diag(BCFinder.GetBreakLoc(), diag::warn_loop_ctrl_binds_to_inner)
+          << "break";
+    }
+  } else if (BCFinder.ContinueFound() && CurScope->getContinueParent()) {
+    Diag(BCFinder.GetContinueLoc(), diag::warn_loop_ctrl_binds_to_inner)
+        << "continue";
+  }
+}
+
 StmtResult
 Sema::ActOnForStmt(SourceLocation ForLoc, SourceLocation LParenLoc,
                    Stmt *First, FullExprArg second, Decl *secondVar,
                    FullExprArg third,
                    SourceLocation RParenLoc, Stmt *Body) {
   if (!getLangOpts().CPlusPlus) {
     if (DeclStmt *DS = dyn_cast_or_null<DeclStmt>(First)) {
       // C99 6.8.5p3: The declaration part of a 'for' statement shall only
       // declare identifiers for objects having storage class 'auto' or
       // 'register'.
       for (DeclStmt::decl_iterator DI=DS->decl_begin(), DE=DS->decl_end();
            DI!=DE; ++DI) {
         VarDecl *VD = dyn_cast<VarDecl>(*DI);
         if (VD && VD->isLocalVarDecl() && !VD->hasLocalStorage())
           VD = 0;
         if (VD == 0) {
           Diag((*DI)->getLocation(), diag::err_non_local_variable_decl_in_for);
           (*DI)->setInvalidDecl();
         }
       }
     }
   }
 
+  CheckBreakContinueBinding(second.get());
+  CheckBreakContinueBinding(third.get());
+
   CheckForLoopConditionalStatement(*this, second.get(), third.get(), Body);
   CheckForRedundantIteration(*this, third.get(), Body);
 
   ExprResult SecondResult(second.release());
   VarDecl *ConditionVar = 0;
   if (secondVar) {
     ConditionVar = cast<VarDecl>(secondVar);
     SecondResult = CheckConditionVariable(ConditionVar, ForLoc, true);

cfe/trunk/test/Analysis/dead-stores.c

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; })) ;
+    for ( ; ; ({ ++i; break; })) ; // expected-warning {{'break' is bound to current loop, GCC binds it to the enclosing loop}}
     // 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}}
   }
 }
 
 // <rdar://problem/8320674> NullStmts followed by do...while() can lead to disconnected CFG

cfe/trunk/test/CodeGen/PR8880.c

+// RUN: %clang_cc1 -Wno-gcc-compat -emit-llvm -o - %s | FileCheck %s
+
+void pr8880_cg_1(int *iptr) {
+// CHECK-LABEL: define void @pr8880_cg_1(
+  int i, j;
+// CHECK: br label %[[OUTER_COND:[0-9A-Za-z$._]+]]
+  for (i = 2; i != 10 ; i++ )
+// CHECK: [[OUTER_COND]]
+// CHECK: label %[[OUTER_BODY:[0-9A-Za-z$._]+]], label %[[OUTER_END:[0-9A-Za-z$._]+]]
+// CHECK: [[OUTER_BODY]]
+// CHECK: br label %[[INNER_COND:[0-9A-Za-z$._]+]]
+    for (j = 3 ; j < 22; (void)({ ++j; break; j;})) {
+// CHECK: [[INNER_COND]]
+// CHECK: label %[[INNER_BODY:[0-9A-Za-z$._]+]], label %[[INNER_END:[0-9A-Za-z$._]+]]
+// CHECK: [[INNER_BODY]]
+      *iptr = 7;
+// CHECK: store i32 7,
+// CHECK: br label %[[INNER_INC:[0-9A-Za-z$._]+]]
+// CHECK: [[INNER_INC]]
+
+// break in 3rd expression of inner loop causes branch to end of inner loop
+
+// CHECK: br label %[[INNER_END:[0-9A-Za-z$._]+]]
+// CHECK: [[INNER_END]]
+    }
+// CHECK: br label %[[OUTER_INC:[0-9A-Za-z$._]+]]
+// CHECK: [[OUTER_INC]]
+// CHECK: br label %[[OUTER_COND]]
+// CHECK: [[OUTER_END]]
+// CHECK: ret
+}
+
+void pr8880_cg_2(int *iptr) {
+// CHECK-LABEL: define void @pr8880_cg_2(
+  int i, j;
+// CHECK: br label %[[OUTER_COND:[0-9A-Za-z$._]+]]
+  for (i = 2; i != 10 ; i++ )
+// CHECK: [[OUTER_COND]]
+// CHECK: label %[[OUTER_BODY:[0-9A-Za-z$._]+]], label %[[OUTER_END:[0-9A-Za-z$._]+]]
+// CHECK: [[OUTER_BODY]]
+// CHECK: br label %[[INNER_COND:[0-9A-Za-z$._]+]]
+    for (j = 3 ; j < 22; (void)({ ++j; continue; j;})) {
+// CHECK: [[INNER_COND]]
+// CHECK: label %[[INNER_BODY:[0-9A-Za-z$._]+]], label %[[INNER_END:[0-9A-Za-z$._]+]]
+// CHECK: [[INNER_BODY]]
+      *iptr = 7;
+// CHECK: store i32 7,
+// CHECK: br label %[[INNER_INC:[0-9A-Za-z$._]+]]
+// CHECK: [[INNER_INC]]
+
+// continue in 3rd expression of inner loop causes branch to inc of inner loop
+
+// CHECK: br label %[[INNER_INC]]
+// CHECK: [[INNER_END]]
+    }
+// CHECK: br label %[[OUTER_INC:[0-9A-Za-z$._]+]]
+// CHECK: [[OUTER_INC]]
+// CHECK: br label %[[OUTER_COND]]
+// CHECK: [[OUTER_END]]
+// CHECK: ret
+}
+
+void pr8880_cg_3(int *iptr) {
+// CHECK-LABEL: define void @pr8880_cg_3(
+  int i, j;
+// CHECK: br label %[[OUTER_COND:[0-9A-Za-z$._]+]]
+  for (i = 2 ; i != 10 ; i++ )
+// CHECK: [[OUTER_COND]]
+// CHECK: label %[[OUTER_BODY:[0-9A-Za-z$._]+]], label %[[OUTER_END:[0-9A-Za-z$._]+]]
+// CHECK: [[OUTER_BODY]]
+// CHECK: br label %[[INNER_COND:[0-9A-Za-z$._]+]]
+    for (j = 3 ; ({break; j;}); j++) {
+
+// break in 2nd expression of inner loop causes branch to end of inner loop
+
+// CHECK: [[INNER_COND]]
+// CHECK: br label %[[INNER_END:[0-9A-Za-z$._]+]]
+// CHECK: label %[[INNER_BODY:[0-9A-Za-z$._]+]], label %[[INNER_END:[0-9A-Za-z$._]+]]
+// CHECK: [[INNER_BODY]]
+      *iptr = 7;
+// CHECK: store i32 7,
+// CHECK: br label %[[INNER_INC:[0-9A-Za-z$._]+]]
+// CHECK: [[INNER_INC]]
+// CHECK: br label %[[INNER_COND]]
+    }
+// CHECK: [[INNER_END]]
+// CHECK: br label %[[OUTER_INC:[0-9A-Za-z$._]+]]
+// CHECK: [[OUTER_INC]]
+// CHECK: br label %[[OUTER_COND]]
+// CHECK: [[OUTER_END]]
+// CHECK: ret
+}
+
+void pr8880_cg_4(int *iptr) {
+// CHECK-LABEL: define void @pr8880_cg_4(
+  int i, j;
+// CHECK: br label %[[OUTER_COND:[0-9A-Za-z$._]+]]
+  for (i = 2 ; i != 10 ; i++ )
+// CHECK: [[OUTER_COND]]
+// CHECK: label %[[OUTER_BODY:[0-9A-Za-z$._]+]], label %[[OUTER_END:[0-9A-Za-z$._]+]]
+// CHECK: [[OUTER_BODY]]
+// CHECK: br label %[[INNER_COND:[0-9A-Za-z$._]+]]
+    for (j = 3 ; ({continue; j;}); j++) {
+
+// continue in 2nd expression of inner loop causes branch to inc of inner loop
+
+// CHECK: [[INNER_COND]]
+// CHECK: br label %[[INNER_INC:[0-9A-Za-z$._]+]]
+// CHECK: label %[[INNER_BODY:[0-9A-Za-z$._]+]], label %[[INNER_END:[0-9A-Za-z$._]+]]
+// CHECK: [[INNER_BODY]]
+      *iptr = 7;
+// CHECK: store i32 7,
+// CHECK: br label %[[INNER_INC]]
+// CHECK: [[INNER_INC]]
+// CHECK: br label %[[INNER_COND]]
+    }
+// CHECK: [[INNER_END]]
+// CHECK: br label %[[OUTER_INC:[0-9A-Za-z$._]+]]
+// CHECK: [[OUTER_INC]]
+// CHECK: br label %[[OUTER_COND]]
+// CHECK: [[OUTER_END]]
+// CHECK: ret
+}
+
+void pr8880_cg_5(int x, int *iptr) {
+// CHECK-LABEL: define void @pr8880_cg_5(
+  int y = 5;
+// CHECK: br label %[[OUTER_COND:[0-9A-Za-z$._]+]]
+// CHECK: [[OUTER_COND]]
+  while(--x) {
+// CHECK: label %[[OUTER_BODY:[0-9A-Za-z$._]+]], label %[[OUTER_END:[0-9A-Za-z$._]+]]
+// CHECK: [[OUTER_BODY]]
+// CHECK: br label %[[INNER_COND:[0-9A-Za-z$._]+]]
+    while(({ break; --y; })) {
+// CHECK: [[INNER_COND]]
+// CHECK: br label %[[INNER_END:[0-9A-Za-z$._]+]]
+// CHECK: label %[[INNER_BODY:[0-9A-Za-z$._]+]], label %[[INNER_END:[0-9A-Za-z$._]+]]
+// CHECK: [[INNER_BODY]]
+      *iptr = 7;
+// CHECK: store i32 7,
+    }
+// CHECK: br label %[[INNER_COND]]
+  }
+// CHECK: [[INNER_END]]
+// CHECK: br label %[[OUTER_COND]]
+// CHECK: [[OUTER_END]]
+// CHECK: ret void
+}
+
+void pr8880_cg_6(int x, int *iptr) {
+// CHECK-LABEL: define void @pr8880_cg_6(
+  int y = 5;
+// CHECK: br label %[[OUTER_COND:[0-9A-Za-z$._]+]]
+// CHECK: [[OUTER_COND]]
+  while(--x) {
+// CHECK: label %[[OUTER_BODY:[0-9A-Za-z$._]+]], label %[[OUTER_END:[0-9A-Za-z$._]+]]
+// CHECK: [[OUTER_BODY]]
+// CHECK: br label %[[INNER_BODY:[0-9A-Za-z$._]+]]
+// CHECK: [[INNER_BODY]]
+    do {
+// CHECK: store i32 7,
+      *iptr = 7;
+// CHECK: br label %[[INNER_COND:[0-9A-Za-z$._]+]]
+    } while(({ break; --y; }));
+// CHECK: [[INNER_COND]]
+// CHECK: br label %[[INNER_END:[0-9A-Za-z$._]+]]
+// CHECK: label %[[INNER_BODY:[0-9A-Za-z$._]+]], label %[[INNER_END]]
+  }
+// CHECK: [[INNER_END]]
+// CHECK: br label %[[OUTER_COND]]
+// CHECK: [[OUTER_END]]
+// CHECK: ret void
+}

cfe/trunk/test/Parser/bad-control.c

 /* RUN: %clang_cc1 -fsyntax-only -verify %s
 */
 void foo() { 
   break; /* expected-error {{'break' statement not in loop or switch statement}} */
 }
 
 void foo2() { 
   continue; /* expected-error {{'continue' statement not in loop 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_24() {
+  for (({break;});;); // expected-error {{'break' statement not in loop or switch statement}}
+}
+
+void pr8880_25() {
+  for (({continue;});;); // expected-error {{'continue' statement not in loop statement}}
+}

cfe/trunk/test/Sema/loop-control.c

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -x c++ -Werror %s
+
+int pr8880_1() {
+  int first = 1;
+  for ( ; ({ if (first) { first = 0; continue; } 0; }); )
+    return 0;
+  return 1;
+}
+
+void pr8880_2(int first) {
+  for ( ; ({ if (first) { first = 0; break; } 0; }); ) {}
+}
+
+void pr8880_3(int first) {
+  for ( ; ; (void)({ if (first) { first = 0; continue; } 0; })) {}
+}
+
+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; })) {}
+}
+
+void pr8880_6 (int first) {
+  while(({ if (first) { first = 0; break; } 0; })) {}
+}
+
+void pr8880_7 (int first) {
+  do {} while(({ if (first) { first = 0; continue; } 0; }));
+}
+
+void pr8880_8 (int first) {
+  do {} while(({ if (first) { first = 0; break; } 0; }));
+}
+
+void pr8880_10(int i) {
+  for ( ; i != 10 ; i++ )
+    for ( ; ; (void)({ ++i; continue; i;})) {} // expected-warning{{'continue' is bound to current loop, GCC binds it to the enclosing loop}}
+}
+
+void pr8880_11(int i) {
+  for ( ; i != 10 ; i++ )
+    for ( ; ; (void)({ ++i; break; i;})) {} // expected-warning{{'break' is bound to current loop, GCC binds it to the enclosing loop}}
+}
+
+void pr8880_12(int i, int j) {
+  for ( ; i != 10 ; i++ )
+    for ( ; ({if (i) continue; i;}); j++) {} // expected-warning {{'continue' is bound to current loop, GCC binds it to the enclosing loop}}
+}
+
+void pr8880_13(int i, int j) {
+  for ( ; i != 10 ; i++ )
+    for ( ; ({if (i) break; i;}); j++) {} // expected-warning{{'break' is bound to current loop, GCC binds it to the enclosing loop}}
+}
+
+void pr8880_14(int i) {
+  for ( ; i != 10 ; i++ )
+    while(({if (i) break; i;})) {} // expected-warning {{'break' is bound to current loop, GCC binds it to the enclosing loop}}
+}
+
+void pr8880_15(int i) {
+  while (--i)
+    while(({if (i) continue; i;})) {} // expected-warning {{'continue' is bound to current loop, GCC binds it to the enclosing loop}}
+}
+
+void pr8880_16(int i) {
+  for ( ; i != 10 ; i++ )
+    do {} while(({if (i) break; i;})); // expected-warning {{'break' is bound to current loop, GCC binds it to the enclosing loop}}
+}
+
+void pr8880_17(int i) {
+  for ( ; i != 10 ; i++ )
+    do {} while(({if (i) continue; i;})); // expected-warning {{'continue' is bound to current loop, GCC binds it to the enclosing loop}}
+}
+
+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;})) {} //expected-warning {{'break' is bound to loop, GCC binds it to switch}}
+  }
+}
+
+void pr8880_21(int x, int y) {
+  switch(x) {
+  case 1:
+    do {} while(({if (y) break; y;})); //expected-warning {{'break' is bound to loop, GCC binds it to switch}}
+  }
+}
+
+void pr8880_22(int x, int y) {
+  switch(x) {
+  case 1:
+    for ( ; ; (void)({ ++y; break; y;})) {} // expected-warning{{'break' is bound to loop, GCC binds it to switc}}
+  }
+}
+
+void pr8880_23(int x, int y) {
+  switch(x) {
+  case 1:
+    for ( ; ({ ++y; break; y;}); ++y) {} // expected-warning{{'break' is bound to loop, GCC binds it to switch}}
+  }
+}

cfe/trunk/test/Sema/statements.c

void foo(enum x X) {
 
   switch (X) { // expected-warning {{5 enumeration values not handled in switch: 'c', 'd', 'e'...}}
   case a:
   case b:
     break;
   }
 }
 
-// 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; }); )
     return 0;
   return 1;
 }