From 9330fda9a0ef108d03334f20319508e409bb356d Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Tue, 4 Jul 2017 00:52:24 +0000 Subject: [PATCH] [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 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@307051 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaStmt.cpp | 67 ++++++++++++++++++++++++++--- test/Sema/loop-control.c | 48 +++++++++++++++++++++ test/SemaCXX/warn-loop-analysis.cpp | 12 ++++++ 3 files changed, 121 insertions(+), 6 deletions(-) diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index eed10b077e..2a38a1f8e1 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/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 { + class BreakContinueFinder : public ConstEvaluatedExprVisitor { 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 Inherited; + typedef ConstEvaluatedExprVisitor 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/test/Sema/loop-control.c b/test/Sema/loop-control.c index 6c33e8437b..1fc35d1021 100644 --- a/test/Sema/loop-control.c +++ b/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/test/SemaCXX/warn-loop-analysis.cpp b/test/SemaCXX/warn-loop-analysis.cpp index 25ec7a7862..2934003848 100644 --- a/test/SemaCXX/warn-loop-analysis.cpp +++ b/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); -- 2.40.0