From fea85e5fcb3578439d01f61c1ea78e816ca403e7 Mon Sep 17 00:00:00 2001 From: Alessio Giacobbe Date: Fri, 28 Aug 2026 11:55:16 +0200 Subject: [PATCH] fix(parser): propagate multi-level break/continue before trailing statements The flag checks that translate `break N` / `continue N` were emitted only at the end of each enclosing loop body. After the inner construct exited with the countdown flag set, every trailing statement of the enclosing body still executed before the check ran: foreach ([1] as $x) { foreach ([1] as $y) { break 2; } echo "leaked"; // ran in compiled output, not in PHP } The native (int-typed) switch path was worse: its check sat inside the do-while(0) wrapper, decrementing the flag a second time for the switch level the C++ `break` had already exited. A `break 2` from a native switch inside a loop therefore never exited the loop at all. Emit the propagation check immediately after every nested loop / switch statement instead, from the statement dispatcher, and drop the dead end-of-body emissions. The check now also distinguishes the enclosing construct: when it sits inside a switch, a continue that lands on the switch level lowers to `break`, matching PHP's continue-targets-switch semantics. parseBreak/parseContinue now reject levels exceeding the number of enclosing breakable constructs - the same compile-time validation PHP performs (`Cannot 'break' 2 levels`) - which the countdown scheme relies on to terminate at an enclosing construct. The continue-2-while scenario in break-continue-level.phpt encoded the old leaked behavior: its `$i++` after the inner loop only ran because of the misplaced check; standard PHP loops forever on it. The counter now advances before the inner loop. --- src/CompilerBase.php | 50 +++--- src/Context/FunctionContext.php | 4 + src/Parser/ForeachTrait.php | 2 +- src/Parser/LoopControlTrait.php | 39 ++++- src/Parser/SwitchTrait.php | 4 +- .../break-continue-level-placement.phpt | 155 ++++++++++++++++++ .../control_flow/break-continue-level.phpt | 7 +- 7 files changed, 223 insertions(+), 38 deletions(-) create mode 100644 tests/compiler/control_flow/break-continue-level-placement.phpt diff --git a/src/CompilerBase.php b/src/CompilerBase.php index c5655dd5..09256358 100644 --- a/src/CompilerBase.php +++ b/src/CompilerBase.php @@ -1684,6 +1684,8 @@ class CompilerBase implements PropertyAccessContext $lines = []; $inLoopTop = $this->context->inLoop; $inContinuableLoopTop = $this->context->inContinuableLoop; + $breakableIsSwitchTop = $this->context->breakableIsSwitch; + $breakableDepthTop = $this->context->breakableDepth; $last = array_key_last($stmts); foreach ($stmts as $i => $v) { $class = $v->getType(); @@ -1715,37 +1717,37 @@ class CompilerBase implements PropertyAccessContext $result = $this->parseReturn($v); break; case 'Stmt_For': - $this->context->inLoop = true; - $this->context->inContinuableLoop = true; - $result = $this->parseFor($v); - $this->context->inLoop = $inLoopTop; - $this->context->inContinuableLoop = $inContinuableLoopTop; - break; case 'Stmt_Foreach': - $this->context->inLoop = true; - $this->context->inContinuableLoop = true; - $result = $this->parseForeach($v); - $this->context->inLoop = $inLoopTop; - $this->context->inContinuableLoop = $inContinuableLoopTop; - break; case 'Stmt_Switch': - $this->context->inLoop = true; - $result = $this->parseSwitch($v); - $this->context->inLoop = $inLoopTop; - break; case 'Stmt_While': - $this->context->inLoop = true; - $this->context->inContinuableLoop = true; - $result = $this->parseWhile($v); - $this->context->inLoop = $inLoopTop; - $this->context->inContinuableLoop = $inContinuableLoopTop; - break; case 'Stmt_Do': + $isSwitch = $class === 'Stmt_Switch'; $this->context->inLoop = true; - $this->context->inContinuableLoop = true; - $result = $this->parseDo($v); + if (!$isSwitch) { + $this->context->inContinuableLoop = true; + } + $this->context->breakableIsSwitch = $isSwitch; + $this->context->breakableDepth = $breakableDepthTop + 1; + $result = match ($class) { + 'Stmt_For' => $this->parseFor($v), + 'Stmt_Foreach' => $this->parseForeach($v), + 'Stmt_Switch' => $this->parseSwitch($v), + 'Stmt_While' => $this->parseWhile($v), + default => $this->parseDo($v), + }; $this->context->inLoop = $inLoopTop; $this->context->inContinuableLoop = $inContinuableLoopTop; + $this->context->breakableIsSwitch = $breakableIsSwitchTop; + $this->context->breakableDepth = $breakableDepthTop; + // A multi-level break/continue exits the nested construct + // with its countdown flag still set. The propagation check + // must run before any trailing statement of this body. + if ($inLoopTop) { + $flagCheck = $this->genMultiLevelJumpCheck($breakableIsSwitchTop); + if ($flagCheck !== '') { + $result = rtrim($result, "\r\n") . PHP_EOL . $flagCheck; + } + } break; case 'Stmt_If': $result = $this->parseIf($v); diff --git a/src/Context/FunctionContext.php b/src/Context/FunctionContext.php index 9a0dda35..d07df058 100644 --- a/src/Context/FunctionContext.php +++ b/src/Context/FunctionContext.php @@ -84,6 +84,10 @@ class FunctionContext public bool $inLoop = false; /** True while parsing a for/foreach/while/do-while body. */ public bool $inContinuableLoop = false; + /** Number of breakable constructs (loops and switches) enclosing the statement being parsed. */ + public int $breakableDepth = 0; + /** True when the innermost enclosing breakable construct is a switch, not a loop. */ + public bool $breakableIsSwitch = false; public bool $inClosure = false; public ?array $closureReturnTypeCheck = null; public string $closureReturnTypeStr = ''; diff --git a/src/Parser/ForeachTrait.php b/src/Parser/ForeachTrait.php index 54f5478d..5ddb8097 100644 --- a/src/Parser/ForeachTrait.php +++ b/src/Parser/ForeachTrait.php @@ -48,7 +48,7 @@ trait ForeachTrait protected function parseForeachBody(Foreach_ $node): string { - return $this->parseStmts($node->stmts) . $this->genLoopEndFlagCheck(); + return $this->parseStmts($node->stmts); } protected function parseForeachKeyAssignment(Foreach_ $node, string $keyExpr, string $defaultType = Type::VAR): string diff --git a/src/Parser/LoopControlTrait.php b/src/Parser/LoopControlTrait.php index 8cd9f48c..501cd2d0 100644 --- a/src/Parser/LoopControlTrait.php +++ b/src/Parser/LoopControlTrait.php @@ -102,7 +102,6 @@ trait LoopControlTrait $code .= ') {' . PHP_EOL; $code .= $this->parseBlockStmts($stmts); - $code .= $this->genLoopEndFlagCheck(); $code .= $this->getIndent() . '}' . PHP_EOL; return $code; @@ -138,7 +137,6 @@ trait LoopControlTrait $code .= 'while (' . $cond . ') {' . PHP_EOL; } $code .= $this->parseBlockStmts($stmts); - $code .= $this->genLoopEndFlagCheck(); $code .= $this->getIndent() . '}' . PHP_EOL; return $code; @@ -172,7 +170,6 @@ trait LoopControlTrait $code = $this->parseBeforeStmtLines() . PHP_EOL; $code .= 'do {' . PHP_EOL; $code .= $bodyCode; - $code .= $this->genLoopEndFlagCheck(); $code .= $this->getIndent() . '} while (' . $cond . ');' . PHP_EOL; return $code; @@ -189,6 +186,7 @@ trait LoopControlTrait } $num = $v->num; if ($num) { + $this->checkLoopJumpLevel($v, $num, 'break'); if ($num->value > 1) { $this->context->hasMultiLevelBreak = true; return '_brk_flag = ' . ($num->value - 1) . '; break;'; @@ -205,6 +203,7 @@ trait LoopControlTrait } $num = $v->num; if ($num) { + $this->checkLoopJumpLevel($v, $num, 'continue'); if ($num->value > 1) { $this->context->hasMultiLevelContinue = true; return '_cnt_flag = ' . ($num->value - 1) . '; break;'; @@ -214,12 +213,32 @@ trait LoopControlTrait } /** - * Emit flag-propagation checks at the end of a loop body. + * PHP only accepts a positive integer literal that does not exceed the + * number of enclosing loops/switches. The flag lowering relies on this: + * it guarantees the countdown reaches zero at an enclosing construct. + */ + protected function checkLoopJumpLevel(Node\Stmt $v, Node\Expr $num, string $operator): void + { + if (!$num instanceof Node\Scalar\Int_ || $num->value < 1) { + $this->fatalError($v, "'{$operator}' operator accepts only positive integer literals"); + } + if ($num->value > $this->context->breakableDepth) { + $this->fatalError($v, "Cannot '{$operator}' {$num->value} levels"); + } + } + + /** + * Emit flag-propagation checks right after a nested breakable construct. * - * Translates multi-level break / continue into plain break / continue - * by decrementing a counter at each loop boundary until it reaches zero. + * A multi-level break / continue is lowered to a flag assignment plus a + * plain break out of the innermost construct. Each enclosing loop or + * switch places this check immediately after every nested loop / switch + * statement, so the flag keeps breaking outward — before any trailing + * statements of the enclosing body can run — until it reaches zero at + * the targeted level. When the check sits inside a switch, a continue + * that lands on the switch level behaves like break, matching PHP. */ - protected function genLoopEndFlagCheck(): string + protected function genMultiLevelJumpCheck(bool $enclosingIsSwitch): string { $code = ''; $indent = $this->getIndent(); @@ -227,7 +246,11 @@ trait LoopControlTrait $code .= "{$indent}if (_brk_flag > 0) { _brk_flag--; break; }" . PHP_EOL; } if ($this->context->hasMultiLevelContinue) { - $code .= "{$indent}if (_cnt_flag > 0) { _cnt_flag--; if (_cnt_flag == 0) continue; else break; }" . PHP_EOL; + if ($enclosingIsSwitch) { + $code .= "{$indent}if (_cnt_flag > 0) { _cnt_flag--; break; }" . PHP_EOL; + } else { + $code .= "{$indent}if (_cnt_flag > 0) { _cnt_flag--; if (_cnt_flag == 0) continue; else break; }" . PHP_EOL; + } } return $code; } diff --git a/src/Parser/SwitchTrait.php b/src/Parser/SwitchTrait.php index 08d3dedf..794391c1 100644 --- a/src/Parser/SwitchTrait.php +++ b/src/Parser/SwitchTrait.php @@ -2,7 +2,7 @@ /** * This file is part of TypePHP. * - * Lowers switch cases, fallthrough, defaults, and loop-exit flags. + * Lowers switch cases, fallthrough, and defaults. */ namespace TypePhp\Parser; @@ -64,7 +64,6 @@ trait SwitchTrait } $this->indentLevel--; $code .= $this->getIndent() . '}' . PHP_EOL; - $code .= $this->genLoopEndFlagCheck(); $this->indentLevel--; $code .= $this->getIndent() . '} while(0);' . PHP_EOL; @@ -166,7 +165,6 @@ trait SwitchTrait $code .= $this->getIndent() . '}' . PHP_EOL; } } - $code .= $this->genLoopEndFlagCheck(); $this->indentLevel--; $code .= $this->getIndent() . '} while (0);'; diff --git a/tests/compiler/control_flow/break-continue-level-placement.phpt b/tests/compiler/control_flow/break-continue-level-placement.phpt new file mode 100644 index 00000000..eaeac9a7 --- /dev/null +++ b/tests/compiler/control_flow/break-continue-level-placement.phpt @@ -0,0 +1,155 @@ +--TEST-- +Multi-level break/continue must skip trailing statements of enclosing bodies +--FILE-- + +--EXPECT-- +b2 inner 1.1 +break-2: done +c2 inner 1.1 +c2 inner 2.1 +continue-2: done +sw iter 0 +sw after 0 +sw iter 1 +sw case 1 +switch-break-2: done +swc after 0 +swc case 1 +swc after 2 +switch-continue-2: done +b3 deep 0.1 +break-3-through-switch: done +c2s deep 0.1 +c2s after switch 0 +c2s after switch 1 +continue-2-targets-switch: done +c3 deep 0.1 +c3 after switch 1 +continue-3-through-switch: done +n-iter 0 +n-case +native-switch-break-2: done diff --git a/tests/compiler/control_flow/break-continue-level.phpt b/tests/compiler/control_flow/break-continue-level.phpt index db92f7dd..72f91131 100644 --- a/tests/compiler/control_flow/break-continue-level.phpt +++ b/tests/compiler/control_flow/break-continue-level.phpt @@ -37,9 +37,13 @@ while ($i < 3) { } echo "break-2-while: done\n"; -// continue 2 from nested while +// continue 2 from nested while. The counter must advance before the +// inner loop: continue 2 jumps straight to the outer condition, so a +// trailing $i++ would never run and the loop would never terminate +// (PHP itself loops forever on that variant). $i = 0; while ($i < 3) { + $i++; $j = 0; while ($j < 3) { $j++; @@ -47,7 +51,6 @@ while ($i < 3) { continue 2; } } - $i++; } echo "continue-2-while: done\n";