Skip to content

Walk a while loop's exit condition instead of narrowing the body's scope by it - #7063

Merged
ondrejmirtes merged 1 commit into
2.3.xfrom
while-exit-condition-assignments
Nov 14, 2026
Merged

ondrejmirtes merged 1 commit into
2.3.xfrom
while-exit-condition-assignments

Conversation

@ondrejmirtes

@ondrejmirtes ondrejmirtes commented Nov 13, 2026 •

Copy link
Copy Markdown
Member

Closes phpstan/phpstan#15726
Also fixes Problem B of phpstan/phpstan#15722 (inverted correlation after while).

After the body of a while loop 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: in while ($row = fetchRow()) the body's array{mixed} $row was narrowed by false to *NEVER*. Since #5876 keeps subtype-absorbed variables as conditional-expression targets, the conditional expression "if $row[0] is truthy then $printed is true" from the body's if matched that never guard, so $printed came out as true after 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 $i as *NEVER* and is now -1. The while-loop-variables.php expectation for while (($val = fetch()) && $i++ < 10) changes from int<0, 10> to int<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 !== 0 implies $stop is true) has the same root cause: $stop was false and the following elseif ($stop) was reported as always false. With the exit-condition walk it is true with the default configuration. With polluteScopeWithLoopInitialAssignments: false (strict-rules sets it), it was still bool. 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 passes preserveVacuousConditionals as 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 $rows is non-zero then $stop is true" survives, because on the never-iterated path $rows is 0. 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 with true, true, *NEVER*, false, false).
  • Bug15722WithoutLoopPollutionTest: Problem B with loop pollution off (fails without the flag with bool), plus control cases where the answer must stay bool.

Verified with make tests with 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's tests/smoke.php, side-by-side.php and signature-parity.php.

…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
ondrejmirtes force-pushed the while-exit-condition-assignments branch from 33fedbc to ed5ec29 Compare November 13, 2026 23:43
@ondrejmirtes
ondrejmirtes merged commit 2a70999 into 2.3.x Nov 14, 2026
902 of 907 checks passed
@ondrejmirtes
ondrejmirtes deleted the while-exit-condition-assignments branch November 14, 2026 09:06
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.

Variable set under a condition on the while-assigned row is inferred as always true after the loop (regression in 2.2.3)

1 participant