Repository navigation
Fix two regressions from NarrowedSubjectType narrowing - #6586
Merged
Merged
Conversation
ConditionalType::traverse() maps the normalized branches and builds a new ConditionalType from them, which normalized them again. When the subject is narrowed in a branch of an enclosing conditional type on the same template type, every nested conditional type gets a new subject, is rebuilt, and each rebuilt level normalizes - traverses - the rest of the chain again. A chain of n nested conditions took about 4^n: 15 levels exhausted any memory limit. The mapped branches are images of normalized branches: the references to the subject in them were already narrowed, and follow whatever the subject maps to. Normalizing them again against the mapped subject would only narrow unrelated types that happen to equal it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uob4ZH23SzXKt3uM5tDUrQ
In `($key is Model ? A : ($key is Arrayable ? B : C))` the inner condition asks about the part of the argument that is not a Model. That narrowing used to happen only because both conditions were resolved against the same argument type object, which the enclosing ConditionalType matched by identity - gone since branches are narrowed through NarrowedSubjectType. ConditionalTypeForParameter::resolveInType() now resolves the parameter as `subject & target` inside the if branch and `subject ~ target` inside the else branch, recognising the nested conditions by the parameter name. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uob4ZH23SzXKt3uM5tDUrQ
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
1 participant
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#15310
Closes phpstan/phpstan#15296
Two regressions from #6480, one commit each.
Keep the normalized branches of a traversed ConditionalType normalized (#15310)
ConditionalType::traverse()built a newConditionalTypefrom the mapped normalized branches, and the new object normalized them again. When an enclosing conditional type on the same template type narrows the subject, every nested level gets a new subject and is rebuilt. Each rebuilt level then traverses the rest of the chain again. Analysis time grew about 4× per nesting level: 9 levels took 5 s, 11 took 68 s, and Symfony's 15-levelBuiltinTypeexhausted memory. Before #6480, identity (===) matching never matched the nested subjects, so nothing was rebuilt.The mapped branches are images of normalized branches, and the narrowed references follow the subject's mapping on their own. So the new object now keeps them as its normalized branches. 15 levels: 0.74 s.
Test:
AnalyserIntegrationTest::testBug15310with the reporter's standalone snippet. It runs out of 1 GB on 2.2.x.Narrow the parameter for nested conditions on the same parameter (#15296)
($key is Model ? A : ($key is Arrayable ? B : C)): the inner condition used to see?User ~ Model. That only worked because the enclosingConditionalTypematched the inner subject by identity, since both levels were built from the same argument type object. #6480 removed identity matching on purpose (#15268).ConditionalTypeForParameter::resolveInType()now resolves the parameter assubject & targetinside the if branch andsubject ~ targetinside the else branch. It recognises the nested conditions by parameter name, at any depth. This covers everything that goes throughresolveInType():@return,@param-out,@phpstan-assert,@throws,@phpstan-self-out.Test:
nsrt/bug-15296.phpwith the reporter's snippet, plusis not, if-branch and 3-level variants. 4 assertions fail on 2.2.x.Both tests fail without their fix. The full suite,
make phpstanandmake cspass.2.3.x needs the C++ counterparts in
ConditionalType.cppandConditionalTypeForParameter.cppafter the merge-up.🤖 Generated with Claude Code
https://claude.ai/code/session_01Uob4ZH23SzXKt3uM5tDUrQ