Skip to content

Do not generalize an array union that is no larger than its members - #6751

Open
HawkForge wants to merge 2 commits into
phpstan:2.3.xfrom
HawkForge:fix/oversized-array-coalesce-offset
Open

HawkForge wants to merge 2 commits into
phpstan:2.3.xfrom
HawkForge:fix/oversized-array-coalesce-offset

Conversation

@HawkForge

Copy link
Copy Markdown
Contributor

For phpstan/phpstan#15459. Thanks @bmdevel for the report and the generator script.

When the branches of $r['o0'] ?? null merge back, the declared shape and its narrowed variant (o0 set and not null) are unioned. reduceArrays() merges them back into the declared shape, but processArrayTypes() then passes the result to optimizeConstantArrays(). That function counts the constant array values in the result, nested shapes included. In the report this is 197 + 20 × 3 = 257, which is above ConstantArrayTypeBuilder::ARRAY_COUNT_LIMIT, so the shape is generalized to non-empty-array<..., ...>&oversized-array. Every later read of a key on it then returns the union of all value types.

2.2 never got to optimizeConstantArrays() here. Its key numbering in processArrayTypes() overflowed at the 63rd distinct key, and the arrays then went through reduceArrays($arrayTypes, false) and returned early. 2.3 removed that numbering, so large shapes now reach the limit check.

With this change, a union that reduces to a single array is kept as it is when that array holds no more constant array values than the largest of the arrays it was made of. The limit is there to stop unions from growing, and a union like this one does not grow. Unions that do grow past the limit, and unions that stay as several arrays, are still optimized as before. The extra counting happens only when the single reduced array is above the limit. The same change is ported to turbo-ext/src/TypeCombinator.cpp, with the regenerated declarations header and the version bump in a separate commit.

tests/PHPStan/Analyser/nsrt/bug-15459.php is the issue's reproducer, with dumpType() replaced by assertType(). Without the fix it fails with (array{currency: string, price: float}&oversized-array)|string|null. It passes with the fix, both with and without the Turbo extension loaded. The full test suite, smoke.php, signature-parity.php and side-by-side.php pass.

Closes phpstan/phpstan#15459

When the scope branches of a check like `$r['o0'] ?? null` merge back,
the shape and its narrowed variant are unioned. reduceArrays() merges
them into the original shape, but optimizeConstantArrays() then counts
the constant array values in the result (nested shapes included) and,
above ConstantArrayTypeBuilder::ARRAY_COUNT_LIMIT, turns it into an
oversized general array. A declared shape with more than 256 values in
total therefore lost its keys after the first `??` on it.

2.2 did not reach optimizeConstantArrays() here: from the 63rd distinct
key on, its key numbering overflowed and the arrays went through
reduceArrays() without the optimization. 2.3 removed that numbering.

The union is now kept as it is when it reduces to a single array that
holds no more constant array values than the largest of the arrays it
was made of - nothing grows, so there is nothing for the limit to
bound.

Closes phpstan/phpstan#15459
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.

Reading an optional key of a large array shape with ?? returns oversized-array with other value types (regression in 2.3.0)

1 participant