Skip to content

fix(parser): propagate multi-level break/continue before trailing statements of enclosing bodies - #16

Closed
AlessioGiacobbe wants to merge 1 commit into
swoole:masterfrom
AlessioGiacobbe:fix/multilevel-break-placement
Closed

fix(parser): propagate multi-level break/continue before trailing statements of enclosing bodies#16
AlessioGiacobbe wants to merge 1 commit into
swoole:masterfrom
AlessioGiacobbe:fix/multilevel-break-placement

Conversation

@AlessioGiacobbe

@AlessioGiacobbe AlessioGiacobbe commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Problem

break N / continue N are lowered to a countdown flag plus a plain break, 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:

foreach ([1, 2, 3] as $x) {
    foreach ([1, 2, 3] as $y) {
        break 2;
    }
    echo "leaked\n";   // printed by the compiled binary, not by PHP
}

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++ break had already exited — so break 2 from a native switch inside a loop never exited the loop at all.

Fix

  • Emit the propagation check immediately after every nested loop/switch statement (from parseStmts) and drop the now-dead end-of-body emissions, including the double-decrementing one in the native-switch wrapper.
  • When the check sits directly inside a switch (new FunctionContext::$breakableIsSwitch), a continue landing on that level lowers to break, matching PHP's continue-targets-switch semantics.
  • parseBreak/parseContinue validate the level against the number of enclosing breakable constructs (new FunctionContext::$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

  • New 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.
  • The continue-2-while scenario in break-continue-level.phpt encoded 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 + switch suites: 38/40 pass; the 2 foreach-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.

…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.
@AlessioGiacobbe
AlessioGiacobbe force-pushed the fix/multilevel-break-placement branch from 5041fe9 to fea85e5 Compare August 28, 2026 10:05
@AlessioGiacobbe
AlessioGiacobbe deleted the fix/multilevel-break-placement branch August 28, 2026 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant