Repository navigation
Walk a while loop's exit condition instead of narrowing the body's scope by it - #7063
Merged
Merged
Conversation
…ope by it
After the body ran, the loop exits through the condition evaluated again.
The post-loop scope was the body's back-edge scope narrowed by the
condition's falsey types, without the condition itself running there, so
an assignment in it narrowed the body's stale value instead of replacing
it: in `while ($row = fetchRow())` the body's array{mixed} $row became
never. Matching conditional expressions on a never guard then fixed
variables set under a condition on $row to their our-branch type, such as
$printed to true. The condition is now walked on the back-edge scope and
the loop exits through its falsey scope, which also counts side effects
such as `$i++` in the condition.
When the loop may not run at all, the post-loop scope merges the exit
scope with the scope where the first condition was falsey. These are
alternative paths like an if's branches, so the merge now keeps a
conditional expression that only one path records when the other path
makes it vacuously true, as IfHandler does. Without it, the correlation
`$rows !== 0` implies `$stop` after
`while (!($stop = $s->stop()) && ($rows = $s->next()))` was lost when
polluteScopeWithLoopInitialAssignments is off (as in strict-rules).
Amp-Thread-ID: https://ampcode.com/threads/T-01a1d722-328d-734b-b55e-1eb10133f833
ondrejmirtes
force-pushed
the
while-exit-condition-assignments
branch
from
November 13, 2026 23:43
33fedbc to
ed5ec29
Compare
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.
Closes phpstan/phpstan#15726
Also fixes Problem B of phpstan/phpstan#15722 (inverted correlation after
while).After the body of a
whileloop ran, the post-loop scope was the body's back-edge scope narrowed by the condition's falsey types, without the condition itself running there. An assignment in the condition therefore narrowed the body's stale value instead of replacing it: inwhile ($row = fetchRow())the body'sarray{mixed}$rowwas narrowed byfalseto*NEVER*. Since #5876 keeps subtype-absorbed variables as conditional-expression targets, the conditional expression "if$row[0]is truthy then$printedistrue" from the body'sifmatched that never guard, so$printedcame out astrueafter the loop.WhileHandler now walks the condition on the back-edge scope and takes its falsey scope, the same way DoWhileHandler exits. The native WhileHandler.cpp mirror makes the same change.
This also fixes the side effects of a condition being dropped on exit.
$i = 10; while ($i-- > 0) {}left$ias*NEVER*and is now-1. Thewhile-loop-variables.phpexpectation forwhile (($val = fetch()) && $i++ < 10)changes fromint<0, 10>toint<0, 11>, because the loop exits after$i++has run with$i = 10.#15722 Problem B (
while (!($stop = $s->stop()) && ($rows = $s->next())) {}, then$rows !== 0implies$stopistrue) has the same root cause:$stopwasfalseand the followingelseif ($stop)was reported as always false. With the exit-condition walk it istruewith the default configuration. WithpolluteScopeWithLoopInitialAssignments: false(strict-rules sets it), it was stillbool. When the loop may not run at all, the post-loop scope merges the exit scope with the scope where the first condition was falsey. These are alternative paths, like an if's branches, so that merge now passespreserveVacuousConditionalsas IfHandler does. A conditional expression that only one path records is kept when the other path makes it vacuously true (its guard type is impossible there) or already satisfies it. Here "if$rowsis non-zero then$stopistrue" survives, because on the never-iterated path$rowsis0. Both rescue rules are sound for any either/or merge, and an over-approximated never-iterated scope (merged with the pre-condition scope when loop pollution is off) only makes them harder to satisfy. The native mirror passes the same flag.Tests:
nsrt/bug-15726.php: the issue's snippet, the$i--exit, and #15722 Problem B (fails before this PR withtrue,true,*NEVER*,false,false).Bug15722WithoutLoopPollutionTest: Problem B with loop pollution off (fails without the flag withbool), plus control cases where the answer must staybool.Verified with
make testswith and without the turbo extension loaded,make phpstan(which runs with strict-rules, so loop pollution is off; output identical with and without the extension),make cs, and the extension'stests/smoke.php,side-by-side.phpandsignature-parity.php.