https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/152606
>From 467f72d8e2e7f3abbd6824433172fddcd24a771e Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Thu, 7 Aug 2025 15:29:51 -0700
Subject: [PATCH 1/3] [clang][Sema] Fix the continue and break scope for while
loops
Make sure we don't push the break and continue scope for a while
loop until after we have evaluated the condition.
---
clang/docs/ReleaseNotes.rst | 2 ++
clang/lib/Parse/ParseStmt.cpp | 3 ++-
clang/test/Sema/while-loop-condition-scope.c | 11 +++++++++++
3 files changed, 15 insertions(+), 1 deletion(-)
create mode 100644 clang/test/Sema/while-loop-condition-scope.c
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 0e9fcaa5fac6a..4062c4b7a6fdb 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -161,6 +161,8 @@ Bug Fixes in This Version
targets that treat ``_Float16``/``__fp16`` as native scalar types. Previously
the warning was silently lost because the operands differed only by an
implicit
cast chain. (#GH149967).
+- Correct the continue and break scope for while statements to be after the
+ condition is evaluated.
Bug Fixes to Compiler Builtins
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index bf1978c22ee9f..b1b700951231f 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1734,7 +1734,6 @@ StmtResult Parser::ParseWhileStatement(SourceLocation
*TrailingElseLoc) {
Scope::DeclScope | Scope::ControlScope;
else
ScopeFlags = Scope::BreakScope | Scope::ContinueScope;
- ParseScope WhileScope(this, ScopeFlags);
// Parse the condition.
Sema::ConditionResult Cond;
@@ -1744,6 +1743,8 @@ StmtResult Parser::ParseWhileStatement(SourceLocation
*TrailingElseLoc) {
Sema::ConditionKind::Boolean, LParen, RParen))
return StmtError();
+ ParseScope WhileScope(this, ScopeFlags);
+
// OpenACC Restricts a while-loop inside of certain construct/clause
// combinations, so diagnose that here in OpenACC mode.
SemaOpenACC::LoopInConstructRAII LCR{getActions().OpenACC()};
diff --git a/clang/test/Sema/while-loop-condition-scope.c
b/clang/test/Sema/while-loop-condition-scope.c
new file mode 100644
index 0000000000000..d87362bdc668d
--- /dev/null
+++ b/clang/test/Sema/while-loop-condition-scope.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void f() {
+ while (({ continue; 1; })) {
+ // expected-error@-1 {{'continue' statement not in loop statement}}
+
+ }
+ while (({ break; 1; })) {
+ // expected-error@-1 {{'break' statement not in loop or switch statement}}
+ }
+}
>From e2b0c23af1741f2d63634f76ac08bf02eea88a08 Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Fri, 8 Aug 2025 11:02:21 -0700
Subject: [PATCH 2/3] other loop kinds in progress
---
clang/lib/Parse/ParseExprCXX.cpp | 4 +--
clang/lib/Parse/ParseStmt.cpp | 24 ++++++----------
clang/test/CoverageMapping/break.c | 11 --------
.../Sema/loop-condition-continue-scopes.c | 28 +++++++++++++++++++
clang/test/Sema/while-loop-condition-scope.c | 11 --------
5 files changed, 38 insertions(+), 40 deletions(-)
create mode 100644 clang/test/Sema/loop-condition-continue-scopes.c
delete mode 100644 clang/test/Sema/while-loop-condition-scope.c
diff --git a/clang/lib/Parse/ParseExprCXX.cpp b/clang/lib/Parse/ParseExprCXX.cpp
index 8dce2268082d9..4aa4cbd383fed 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -1877,10 +1877,8 @@ Parser::ParseCXXCondition(StmtResult *InitStmt,
SourceLocation Loc,
struct ForConditionScopeRAII {
Scope *S;
void enter(bool IsConditionVariable) {
- if (S) {
- S->AddFlags(Scope::BreakScope | Scope::ContinueScope);
+ if (S)
S->setIsConditionVarScope(IsConditionVariable);
- }
}
~ForConditionScopeRAII() {
if (S)
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index b1b700951231f..f93ec67b36bcf 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1728,12 +1728,11 @@ StmtResult Parser::ParseWhileStatement(SourceLocation
*TrailingElseLoc) {
// while, for, and switch statements are local to the if, while, for, or
// switch statement (including the controlled statement).
//
- unsigned ScopeFlags;
+ unsigned ScopeFlags = 0;
if (C99orCXX)
- ScopeFlags = Scope::BreakScope | Scope::ContinueScope |
- Scope::DeclScope | Scope::ControlScope;
- else
- ScopeFlags = Scope::BreakScope | Scope::ContinueScope;
+ ScopeFlags = Scope::DeclScope | Scope::ControlScope;
+
+ ParseScope WhileScope(this, ScopeFlags);
// Parse the condition.
Sema::ConditionResult Cond;
@@ -1743,7 +1742,7 @@ StmtResult Parser::ParseWhileStatement(SourceLocation
*TrailingElseLoc) {
Sema::ConditionKind::Boolean, LParen, RParen))
return StmtError();
- ParseScope WhileScope(this, ScopeFlags);
+ getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
// OpenACC Restricts a while-loop inside of certain construct/clause
// combinations, so diagnose that here in OpenACC mode.
@@ -1839,6 +1838,8 @@ StmtResult Parser::ParseDoStatement() {
// A do-while expression is not a condition, so can't have attributes.
DiagnoseAndSkipCXX11Attributes();
+ DoScope.Exit();
+
SourceLocation Start = Tok.getLocation();
ExprResult Cond = ParseExpression();
if (!Cond.isUsable()) {
@@ -1849,7 +1850,6 @@ StmtResult Parser::ParseDoStatement() {
Actions.getASTContext().BoolTy);
}
T.consumeClose();
- DoScope.Exit();
if (Cond.isInvalid() || Body.isInvalid())
return StmtError();
@@ -2124,9 +2124,6 @@ StmtResult Parser::ParseForStatement(SourceLocation
*TrailingElseLoc) {
}
} else {
- // We permit 'continue' and 'break' in the condition of a for loop.
- getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
-
ExprResult SecondExpr = ParseExpression();
if (SecondExpr.isInvalid())
SecondPart = Sema::ConditionError();
@@ -2138,11 +2135,6 @@ StmtResult Parser::ParseForStatement(SourceLocation
*TrailingElseLoc) {
}
}
- // Enter a break / continue scope, if we didn't already enter one while
- // parsing the second part.
- if (!getCurScope()->isContinueScope())
- getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
-
// Parse the third part of the for statement.
if (!ForEach && !ForRangeInfo.ParsedForRangeDecl()) {
if (Tok.isNot(tok::semi)) {
@@ -2165,6 +2157,8 @@ StmtResult Parser::ParseForStatement(SourceLocation
*TrailingElseLoc) {
// Match the ')'.
T.consumeClose();
+ getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
+
// C++ Coroutines [stmt.iter]:
// 'co_await' can only be used for a range-based for statement.
if (CoawaitLoc.isValid() && !ForRangeInfo.ParsedForRangeDecl()) {
diff --git a/clang/test/CoverageMapping/break.c
b/clang/test/CoverageMapping/break.c
index 17083493a3403..09d0bd4a35708 100644
--- a/clang/test/CoverageMapping/break.c
+++ b/clang/test/CoverageMapping/break.c
@@ -31,14 +31,3 @@ int main(void) { // CHECK: File 0, [[@LINE]]:16 ->
{{[0-9]+}}:2 = #0
++cnt;
}
}
-
-// CHECK-LABEL: break_continue_in_increment:
-// CHECK: [[@LINE+6]]:11 -> [[@LINE+6]]:45 = #1
-// CHECK: [[@LINE+5]]:18 -> [[@LINE+5]]:19 = #1
-// CHECK: [[@LINE+4]]:21 -> [[@LINE+4]]:26 = #2
-// CHECK: [[@LINE+3]]:33 -> [[@LINE+3]]:41 = (#1 - #2)
-// CHECK: [[@LINE+3]]:5 -> [[@LINE+3]]:6 = #1
-void break_continue_in_increment(int x) {
- for (;; ({ if (x) break; else continue; }))
- ;
-}
diff --git a/clang/test/Sema/loop-condition-continue-scopes.c
b/clang/test/Sema/loop-condition-continue-scopes.c
new file mode 100644
index 0000000000000..11efa6dfb4d48
--- /dev/null
+++ b/clang/test/Sema/loop-condition-continue-scopes.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void f() {
+ while (({ continue; 1; })) {}
+ // expected-error@-1 {{'continue' statement not in loop statement}}
+ while (({ break; 1; })) {}
+ // expected-error@-1 {{'break' statement not in loop or switch statement}}
+ do {} while (({ break; 1; }));
+ // expected-error@-1 {{'break' statement not in loop or switch statement}}
+ do {} while (({ continue; 1;}));
+ // expected-error@-1 {{'continue' statement not in loop statement}}
+ for (({ continue; });;) {}
+ // expected-error@-1 {{'continue' statement not in loop statement}}
+ for (;({ continue; 1;});) {}
+ // expected-error@-1 {{'continue' statement not in loop statement}}
+ for (;;({ continue;})) {}
+ // expected-error@-1 {{'continue' statement not in loop statement}}
+ for (({ break;});;) {}
+ // expected-error@-1 {{'break' statement not in loop or switch statement}}
+ for (;({ break; 1;});) {}
+ // expected-error@-1 {{'break' statement not in loop or switch statement}}
+ for (;;({ break;})) {}
+ // expected-error@-1 {{'break' statement not in loop or switch statement}}
+ switch(({break;1;})){
+ // expected-error@-1 {{'break' statement not in loop or switch statement}}
+ case 1: break;
+ }
+}
diff --git a/clang/test/Sema/while-loop-condition-scope.c
b/clang/test/Sema/while-loop-condition-scope.c
deleted file mode 100644
index d87362bdc668d..0000000000000
--- a/clang/test/Sema/while-loop-condition-scope.c
+++ /dev/null
@@ -1,11 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-
-void f() {
- while (({ continue; 1; })) {
- // expected-error@-1 {{'continue' statement not in loop statement}}
-
- }
- while (({ break; 1; })) {
- // expected-error@-1 {{'break' statement not in loop or switch statement}}
- }
-}
>From 020147d9eaa0696b6d8d9ebbf4fa700d31c93959 Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Thu, 14 Aug 2025 01:52:18 -0700
Subject: [PATCH 3/3] Actually commit the codegen fixes this time
---
clang/lib/CodeGen/CGStmt.cpp | 25 +++++++++++--------------
clang/test/CoverageMapping/break.c | 24 ++++++++++++++++++++++++
2 files changed, 35 insertions(+), 14 deletions(-)
diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp
index 1a8c6f015bda1..051642bf5c057 100644
--- a/clang/lib/CodeGen/CGStmt.cpp
+++ b/clang/lib/CodeGen/CGStmt.cpp
@@ -1087,9 +1087,6 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,
// also become the break target.
JumpDest LoopExit = getJumpDestInCurrentScope("while.end");
- // Store the blocks to use for break and continue.
- BreakContinueStack.push_back(BreakContinue(LoopExit, LoopHeader));
-
// C++ [stmt.while]p2:
// When the condition of a while statement is a declaration, the
// scope of the variable that is declared extends from its point
@@ -1158,6 +1155,9 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,
<< SourceRange(S.getWhileLoc(), S.getRParenLoc());
}
+ // Store the blocks to use for break and continue.
+ BreakContinueStack.push_back(BreakContinue(LoopExit, LoopHeader));
+
// Emit the loop body. We have to emit this in a cleanup scope
// because it might be a singleton DeclStmt.
{
@@ -1206,8 +1206,6 @@ void CodeGenFunction::EmitDoStmt(const DoStmt &S,
uint64_t ParentCount = getCurrentProfileCount();
- // Store the blocks to use for break and continue.
- BreakContinueStack.push_back(BreakContinue(LoopExit, LoopCond));
// Emit the body of the loop.
llvm::BasicBlock *LoopBody = createBasicBlock("do.body");
@@ -1220,10 +1218,13 @@ void CodeGenFunction::EmitDoStmt(const DoStmt &S,
if (CGM.shouldEmitConvergenceTokens())
ConvergenceTokenStack.push_back(emitConvergenceLoopToken(LoopBody));
+ // Store the blocks to use for break and continue.
+ BreakContinueStack.push_back(BreakContinue(LoopExit, LoopCond));
{
RunCleanupsScope BodyScope(*this);
EmitStmt(S.getBody());
}
+ BreakContinueStack.pop_back();
EmitBlock(LoopCond.getBlock());
// When single byte coverage mode is enabled, add a counter to loop
condition.
@@ -1238,8 +1239,6 @@ void CodeGenFunction::EmitDoStmt(const DoStmt &S,
// 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.
llvm::ConstantInt *C = dyn_cast<llvm::ConstantInt>(BoolCondVal);
@@ -1328,7 +1327,6 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
Continue = CondDest;
else if (!S.getConditionVariable())
Continue = getJumpDestInCurrentScope("for.inc");
- BreakContinueStack.push_back(BreakContinue(LoopExit, Continue));
if (S.getCond()) {
// If the for statement has a condition scope, emit the local variable
@@ -1339,7 +1337,6 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
// We have entered the condition variable's scope, so we're now able to
// jump to the continue block.
Continue = S.getInc() ? getJumpDestInCurrentScope("for.inc") : CondDest;
- BreakContinueStack.back().ContinueBlock = Continue;
}
// When single byte coverage mode is enabled, add a counter to loop
@@ -1381,7 +1378,6 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
EmitBlock(ExitBlock);
EmitBranchThroughCleanup(LoopExit);
}
-
EmitBlock(ForBody);
} else {
// Treat it as a non-zero constant. Don't even create a new block for the
@@ -1393,12 +1389,15 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
incrementProfileCounter(S.getBody());
else
incrementProfileCounter(&S);
+
+ BreakContinueStack.push_back(BreakContinue(LoopExit, Continue));
{
// Create a separate cleanup scope for the body, in case it is not
// a compound statement.
RunCleanupsScope BodyScope(*this);
EmitStmt(S.getBody());
}
+ BreakContinueStack.pop_back();
// The last block in the loop's body (which unconditionally branches to the
// `inc` block if there is one).
@@ -1412,8 +1411,6 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
incrementProfileCounter(S.getInc());
}
- BreakContinueStack.pop_back();
-
ConditionScope.ForceCleanup();
EmitStopPoint(&S);
@@ -1518,6 +1515,8 @@ CodeGenFunction::EmitCXXForRangeStmt(const
CXXForRangeStmt &S,
EmitStmt(S.getLoopVarStmt());
EmitStmt(S.getBody());
}
+ BreakContinueStack.pop_back();
+
// The last block in the loop's body (which unconditionally branches to the
// `inc` block if there is one).
auto *FinalBodyBB = Builder.GetInsertBlock();
@@ -1527,8 +1526,6 @@ CodeGenFunction::EmitCXXForRangeStmt(const
CXXForRangeStmt &S,
EmitBlock(Continue.getBlock());
EmitStmt(S.getInc());
- BreakContinueStack.pop_back();
-
EmitBranch(CondBlock);
ForScope.ForceCleanup();
diff --git a/clang/test/CoverageMapping/break.c
b/clang/test/CoverageMapping/break.c
index 09d0bd4a35708..86aa2a0f4d6d6 100644
--- a/clang/test/CoverageMapping/break.c
+++ b/clang/test/CoverageMapping/break.c
@@ -31,3 +31,27 @@ int main(void) { // CHECK: File 0, [[@LINE]]:16 ->
{{[0-9]+}}:2 = #0
++cnt;
}
}
+
+// CHECK-LABEL: break_continue_in_increment:
+/*
+ 52:41 -> 57:2 = #0
+ 53:10 -> 53:11 = ((#0 + #1) + #2)
+ Branch,File 0, 53:10 -> 53:11 = #1, 0
+
+ 53:13 -> 56:4 = #1
+
+
+
+
+*/
+
+ // CHECK: [[@LINE+6]]:20 -> [[@LINE+6]]:21 = #2
+ // CHECK: [[@LINE+5]]:23 -> [[@LINE+5]]:28 = #3
+ // CHECK: [[@LINE+4]]:35 -> [[@LINE+4]]:43 = (#2 - #3)
+ // CHECK: [[@LINE+4]]:7 -> [[@LINE+4]]:8 = #2
+void break_continue_in_increment(int x) {
+ while (1) {
+ for (;; ({ if (x) break; else continue; }))
+ ;
+ }
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
