Repository navigation
Keep only the current conditionals a replayed statement's recorded walk kept - #7075
Open
ondrejmirtes wants to merge 1 commit into
Open
ondrejmirtes wants to merge 1 commit into
ondrejmirtes wants to merge 1 commit into
Conversation
…lk kept
A statement replayed from a loop's previous pass carried the recorded exit's
whole holder list onto the current scope for each expression whose
conditional holders the statement changed. When the current holders of that
expression differed from the recorded entry's, the recorded ones described
the previous pass's state: `$rowCount++` dropping the `$values` holders
guarded by `$rowCount` replaced the final walk's `$values` holders with the
pass's, such as `$cols` int<1, max> implying `$values` is array{0?: ...}.
The while loop's merge with its not-ran path then kept that holder, and
`$col = 0` in a later for loop narrowed `$values` to array{0?: ...}.
Such an expression now keeps only its current holders that the recorded
walk kept too.
Amp-Thread-ID: https://ampcode.com/threads/T-01a1deb9-7acb-72e2-bab8-0404b371a021
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#15736
When a loop's final walk replays a statement from the previous convergence pass (because it mentions no variable whose state changed),
MutatingScope::withRecordedStatementDelta()copied the recorded exit's whole conditional-holder list for every expression whose holders the statement changed. If the current holders of that expression differed from the recorded entry's, the copied ones described the previous pass's state.In the issue's snippet,
$rowCount++drops the$valuesholders guarded by$rowCount, so the replay replaced the final walk's$valuesholders with the pass's, including "$colsisint<1, max>implies$valuesisarray{0?: ...}". Since 2a70999 the while loop's merge with its not-ran path keeps that holder ($colsis0there), and$col = 0in the laterforloop then narrowed$valuestoarray{0?: ...}.Such an expression now keeps only its current holders that the recorded walk kept too. Dropping holders can only lose precision; when the current holders equal the recorded entry's, the delta is applied exactly as before. The native mirror in
turbo-ext/src/MutatingScope.cppgets the same change.The regression test is a separate
TypeInferenceTestCasewithout bleeding edge: with bleeding edge on,ConstantArrayTypebuilds the loop's shapes differently and the bug does not reproduce.