[Sema] Make BreakContinueFinder handle nested loops.

We don't care about break or continue statements that aren't
associated with the current loop, so make sure the visitor
doesn't find them.

Fixes https://bugs.llvm.org/show_bug.cgi?id=32648 .

Differential Revision: https://reviews.llvm.org/D34568

llvm-svn: 307051

3 files changed, 121 insertions, 6 deletions

diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index eed10b077eb..2a38a1f8e1d 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -1544,23 +1544,78 @@ namespace {
 
   // A visitor to determine if a continue or break statement is a
   // subexpression.
-  class BreakContinueFinder : public EvaluatedExprVisitor<BreakContinueFinder> {
+  class BreakContinueFinder : public ConstEvaluatedExprVisitor<BreakContinueFinder> {
     SourceLocation BreakLoc;
     SourceLocation ContinueLoc;
+    bool InSwitch = false;
+
   public:
-    BreakContinueFinder(Sema &S, Stmt* Body) :
+    BreakContinueFinder(Sema &S, const Stmt* Body) :
         Inherited(S.Context) {
       Visit(Body);
     }
 
-    typedef EvaluatedExprVisitor<BreakContinueFinder> Inherited;
+    typedef ConstEvaluatedExprVisitor<BreakContinueFinder> Inherited;
 
-    void VisitContinueStmt(ContinueStmt* E) {
+    void VisitContinueStmt(const ContinueStmt* E) {
       ContinueLoc = E->getContinueLoc();
     }
 
-    void VisitBreakStmt(BreakStmt* E) {
-      BreakLoc = E->getBreakLoc();
+    void VisitBreakStmt(const BreakStmt* E) {
+      if (!InSwitch)
+        BreakLoc = E->getBreakLoc();
+    }
+
+    void VisitSwitchStmt(const SwitchStmt* S) {
+      if (const Stmt *Init = S->getInit())
+        Visit(Init);
+      if (const Stmt *CondVar = S->getConditionVariableDeclStmt())
+        Visit(CondVar);
+      if (const Stmt *Cond = S->getCond())
+        Visit(Cond);
+
+      // Don't return break statements from the body of a switch.
+      InSwitch = true;
+      if (const Stmt *Body = S->getBody())
+        Visit(Body);
+      InSwitch = false;
+    }
+
+    void VisitForStmt(const ForStmt *S) {
+      // Only visit the init statement of a for loop; the body
+      // has a different break/continue scope.
+      if (const Stmt *Init = S->getInit())
+        Visit(Init);
+    }
+
+    void VisitWhileStmt(const WhileStmt *) {
+      // Do nothing; the children of a while loop have a different
+      // break/continue scope.
+    }
+
+    void VisitDoStmt(const DoStmt *) {
+      // Do nothing; the children of a while loop have a different
+      // break/continue scope.
+    }
+
+    void VisitCXXForRangeStmt(const CXXForRangeStmt *S) {
+      // Only visit the initialization of a for loop; the body
+      // has a different break/continue scope.
+      if (const Stmt *Range = S->getRangeStmt())
+        Visit(Range);
+      if (const Stmt *Begin = S->getBeginStmt())
+        Visit(Begin);
+      if (const Stmt *End = S->getEndStmt())
+        Visit(End);
+    }
+
+    void VisitObjCForCollectionStmt(const ObjCForCollectionStmt *S) {
+      // Only visit the initialization of a for loop; the body
+      // has a different break/continue scope.
+      if (const Stmt *Element = S->getElement())
+        Visit(Element);
+      if (const Stmt *Collection = S->getCollection())
+        Visit(Collection);
     }
 
     bool ContinueFound() { return ContinueLoc.isValid(); }
diff --git a/clang/test/Sema/loop-control.c b/clang/test/Sema/loop-control.c
index 6c33e8437bd..1fc35d10218 100644
--- a/clang/test/Sema/loop-control.c
+++ b/clang/test/Sema/loop-control.c
@@ -119,3 +119,51 @@ void pr8880_23(int x, int y) {
     for ( ; ({ ++y; break; y;}); ++y) {} // expected-warning{{'break' is bound to loop, GCC binds it to switch}}
   }
 }
+
+void pr32648_1(int x, int y) {
+  switch(x) {
+  case 1:
+    for ( ; ({ ++y; switch (y) { case 0: break; } y;}); ++y) {} // no warning
+  }
+}
+
+void pr32648_2(int x, int y) {
+  while(x) {
+    for ( ; ({ ++y; switch (y) { case 0: continue; } y;}); ++y) {}  // expected-warning {{'continue' is bound to current loop, GCC binds it to the enclosing loop}}
+  }
+}
+
+void pr32648_3(int x, int y) {
+  switch(x) {
+  case 1:
+    for ( ; ({ ++y; for (; y; y++) { break; } y;}); ++y) {} // no warning
+  }
+}
+
+void pr32648_4(int x, int y) {
+  switch(x) {
+  case 1:
+    for ( ; ({ ++y; for (({ break; }); y; y++) { } y;}); ++y) {} // expected-warning{{'break' is bound to loop, GCC binds it to switch}}
+  }
+}
+
+void pr32648_5(int x, int y) {
+  switch(x) {
+  case 1:
+    for ( ; ({ ++y; while (({ break; y; })) {} y;}); ++y) {} // expected-warning{{'break' is bound to current loop, GCC binds it to the enclosing loop}}
+  }
+}
+
+void pr32648_6(int x, int y) {
+  switch(x) {
+  case 1:
+    for ( ; ({ ++y; do {} while (({ break; y; })); y;}); ++y) {} // expected-warning{{'break' is bound to current loop, GCC binds it to the enclosing loop}}
+  }
+}
+
+void pr32648_7(int x, int y) {
+  switch(x) {
+  case 1:
+    for ( ; ({ ++y; do { break; } while (y); y;}); ++y) {} // no warning
+  }
+}
diff --git a/clang/test/SemaCXX/warn-loop-analysis.cpp b/clang/test/SemaCXX/warn-loop-analysis.cpp
index 25ec7a7862e..2934003848a 100644
--- a/clang/test/SemaCXX/warn-loop-analysis.cpp
+++ b/clang/test/SemaCXX/warn-loop-analysis.cpp
@@ -202,6 +202,12 @@ void test7() {
     if (true) continue;
     i--;
   }
+
+  // But do warn if the continue is in a nested loop.
+  for (;;i--) { // expected-note{{decremented here}}
+    for (int j = 0; j < 10; ++j) continue;
+    i--; // expected-warning{{decremented both}}
+  }
 }
 
 struct iterator {
@@ -259,6 +265,12 @@ void test8() {
     if (true) continue;
     i--;
   }
+
+  // But do warn if the continue is in a nested loop.
+  for (;;i--) { // expected-note{{decremented here}}
+    for (int j = 0; j < 10; ++j) continue;
+    i--; // expected-warning{{decremented both}}
+  }
 }
 
 int f(int);