Skip HasOffsetValueType propagation in specifyExpressionType when parent type is a large union#5453
Conversation
… parent type is a large union - When narrowing a nested array dim fetch like `$config['menu']['key']`, `specifyExpressionType` recursively propagates `HasOffsetValueType` to parent expressions (`$config['menu']` and `$config`) - Each condition doubles the union members of the parent type because `TypeCombinator::union` preserves distinct `HasOffsetValueType` combinations - After N conditions, the parent type has 2^N union members, causing exponential growth in scope merging and type checking time - Fix: skip adding `HasOffsetValueType` to the parent when the parent's current type is already a union with more than 16 members - This also fixes the previously-reverted #14319 (array|object with many offset checks) since the limit now applies to all types - Added PHPBench regression test reproducing the original issue
src/Analyser/MutatingScope.php
Outdated
| return $scope; | ||
| } | ||
|
|
||
| private function hasOverflowingHasOffsetValueTypes(Type $type): bool |
There was a problem hiding this comment.
I find the method name is misleading since it only count the types from the UnionType ; we could have a big union type without HasOffsetValueType.
We could think this method will count the number of HasOffsetValueType
There was a problem hiding this comment.
Done. I renamed hasOverflowingHasOffsetValueTypes() to isLargeUnionType() and the constant from ARRAY_DIM_FETCH_UNION_HAS_OFFSET_VALUE_TYPE_LIMIT to ARRAY_DIM_FETCH_UNION_MEMBER_LIMIT. The new names accurately reflect that the check is about the union type's member count, not specifically about HasOffsetValueType instances. Tests and static analysis both pass.
The previous name was misleading: it suggested counting HasOffsetValueType members specifically, but it actually checks whether the union type has too many members overall. Rename the method and constant to accurately reflect what they check. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ondrejmirtes
left a comment
ondrejmirtes
left a comment
There was a problem hiding this comment.
Want to block the merge here - I want to try some alternative approaches
|
The review from ondrejmirtes doesn't request any specific code changes. The reviewer is explicitly blocking the merge because they want to explore alternative approaches to solving the exponential union growth problem. There's nothing actionable to implement here - the maintainer wants to investigate different solutions before deciding on the right approach. The PR should remain open but not be merged until ondrejmirtes has had a chance to try their alternative ideas. No commits or pushes are needed since no code changes were requested. |
3 participants
Summary
When analyzing code with multiple
ifconditions checking nested array offsets (like$config['menu']['key']), PHPStan's type tracking causes exponential growth in the parent type's union members. Each condition doubles the number of union members becausespecifyExpressionTyperecursively propagatesHasOffsetValueTypeintersections upward, andTypeCombinator::unionpreserves all distinct combinations. With 8 conditions, the parent type has 2^8 = 256 union members, making analysis take 37+ seconds.Changes
ARRAY_DIM_FETCH_UNION_HAS_OFFSET_VALUE_TYPE_LIMIT = 16constant toMutatingScopehasOverflowingHasOffsetValueTypes()helper that checks if a type is a union with more than 16 membersspecifyExpressionType(), skip addingHasOffsetValueTypeto the parent array expression when the parent's type already exceeds this union member limit (src/Analyser/MutatingScope.php)tests/bench/data/bug-14462.php) reproducing the issue from the bug reportAnalogous cases probed
array|objectwith many offset checks): also fixed by this change, since the limit applies to all types regardless of whether they are definitively arrays. The original fix for #14319 was reverted because it only applied to non-array types; this fix is more general.$config['a']['b']['key']): tested with 8 conditions, completes in ~3s instead of potentially exponential time.specifyExpressionTypeonly recurses forArrayDimFetch, notPropertyFetch).Root cause
The root cause is the recursive
HasOffsetValueTypepropagation inMutatingScope::specifyExpressionType(). When narrowing$config['menu']['key']to a specific type:HasOffsetValueType('key', true)is intersected with$config['menu']'s typeHasOffsetValueType('menu', <updated_type>)with$config's typeTypeCombinator::unioncreates a union of all possibleHasOffsetValueTypecombinationsThe fix stops adding
HasOffsetValueTypeonce the parent type is already a union with more than 16 members, preventing the exponential explosion while still preserving precise type tracking for the first several conditions.Test
tests/bench/data/bug-14462.php- PHPBench regression test with the exact code from the issue report (8 conditional array dim fetch checks on nested array offsets). Before the fix: ~37s. After the fix: ~1.8s.Fixes phpstan/phpstan#14462