fix(parser): propagate multi-level break/continue before trailing statements of enclosing bodies - #17
Merged
matyhtf merged 1 commit intoAug 28, 2026
Conversation
…tements
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.
Member
|
Thank you for the detailed report, careful analysis, and comprehensive fix. The propagation change correctly addresses the trailing-statement leak and the native-switch level handling, and the positive coverage is very helpful. We have merged the PR. We will add the remaining negative compiler-diagnostic tests for invalid break/continue levels in a follow-up commit shortly. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
break N/continue Nare lowered to a countdown flag plus a plainbreak, but the propagation check was emitted only at the end of each enclosing loop body — so statements between the inner construct and the end of the body still executed:On the native (int-typed) switch path the check also sat inside the
do { switch ... } while(0)wrapper, decrementing the flag a second time for the switch level the C++breakhad already exited — sobreak 2from a native switch inside a loop never exited the loop at all.Fix
parseStmts) and drop the now-dead end-of-body emissions, including the double-decrementing one in the native-switch wrapper.FunctionContext::$breakableIsSwitch), a continue landing on that level lowers tobreak, matching PHP's continue-targets-switch semantics.parseBreak/parseContinuevalidate the level against the number of enclosing breakable constructs (newFunctionContext::$breakableDepth), reproducing PHP's compile-time error (Cannot 'break' 2 levels). The countdown scheme relies on this to always reach zero at an enclosing construct.Tests
break-continue-level-placement.phpt: 8 scenarios with observable side effects (break/continue 2 and 3 across loops, both switch lowerings, continue targeting a switch). Expected output generated by running the same code under PHP 8.4.continue-2-whilescenario inbreak-continue-level.phptencoded the old behavior: its trailing$i++only ran because of the misplaced check, and standard PHP loops forever on that code (verified). The counter now advances before the inner loop.control_flow+loop+switchsuites: 38/40 pass; the 2foreach-iterator-*failures are deprecation-notice noise that fails identically on master. No new PHPStan errors. A compiled 5-scenario repro diffs clean against PHP 8.4 output.