Skip to content

Commit e00bf86

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Do not expand optional-key shapes into every variant before loop widening
MutatingScope::generalizeType() flattened both sides with TypeUtils::flattenTypes(), which turns a constant array with N optional keys into its 2^N concrete variants (four lossy representatives above ten) - only for the union right after to merge them back into the same shape, paying the pairwise reduceArrays() pass for hundreds of variants on every loop iteration. A method building a shape from a dozen conditional writes spent 1.8 s there. The per-key widening reads keys, values and optionality straight from the shape, so only unions are split now. The bug-13637 fixture keeps the loop's int<0, 8> bound on the middle key (the comment had called the previous `int` a residual) and describes the innermost level as the optional-key shape it is. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DfQHLTyyoMfjfa4sXJCGa5
1 parent 7a30820 commit e00bf86

4 files changed

Lines changed: 310 additions & 8 deletions

File tree

‎src/Analyser/MutatingScope.php‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5006,6 +5006,31 @@ private function generalizeVariableTypeHolders(
50065006
return $newVariableTypeHolders;
50075007
}
50085008

5009+
/**
5010+
* TypeUtils::flattenTypes() expands a shape with optional keys into every
5011+
* concrete variant (2^N of them, four lossy representatives above ten),
5012+
* only for the union below to merge them back into the very same shape -
5013+
* quadratic in the number of variants. The per-key widening reads the
5014+
* shape's keys, values and optionality directly, so only unions are split.
5015+
*
5016+
* @return list<Type>
5017+
*/
5018+
private function flattenUnionForGeneralization(Type $type): array
5019+
{
5020+
if (!$type instanceof UnionType) {
5021+
return [$type];
5022+
}
5023+
5024+
$types = [];
5025+
foreach ($type->getTypes() as $innerType) {
5026+
foreach ($this->flattenUnionForGeneralization($innerType) as $flattenedType) {
5027+
$types[] = $flattenedType;
5028+
}
5029+
}
5030+
5031+
return $types;
5032+
}
5033+
50095034
private function generalizeType(Type $a, Type $b, int $depth): Type
50105035
{
50115036
if ($a->equals($b)) {
@@ -5033,8 +5058,8 @@ private function generalizeType(Type $a, Type $b, int $depth): Type
50335058
$otherTypes = [];
50345059

50355060
foreach ([
5036-
'a' => TypeUtils::flattenTypes($a),
5037-
'b' => TypeUtils::flattenTypes($b),
5061+
'a' => $this->flattenUnionForGeneralization($a),
5062+
'b' => $this->flattenUnionForGeneralization($b),
50385063
] as $key => $types) {
50395064
foreach ($types as $type) {
50405065
if ($type instanceof ConstantIntegerType) {

‎tests/PHPStan/Analyser/ScopeTest.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use PhpParser\Node\Expr\MethodCall;
77
use PhpParser\Node\Expr\Variable;
88
use PhpParser\Node\Name\FullyQualified;
9+
use PHPStan\DependencyInjection\BleedingEdgeToggle;
910
use PHPStan\Node\Expr\PossiblyImpureCallExpr;
1011
use PHPStan\Testing\PHPStanTestCase;
1112
use PHPStan\TrinaryLogic;
@@ -27,7 +28,11 @@ class ScopeTest extends PHPStanTestCase
2728

2829
public static function dataGeneralize(): array
2930
{
30-
return [
31+
// A directly constructed shape is a legacy (unsealed) one only while bleeding
32+
// edge is off - the toggle is process-global, and PHPUnit 9 evaluates the
33+
// provider after earlier test classes may have left it on. The widening
34+
// contract below is the legacy one, so the inputs are pinned to it.
35+
return BleedingEdgeToggle::withBleedingEdge(false, static fn (): array => [
3136
[
3237
new ConstantStringType('a'),
3338
new ConstantStringType('a'),
@@ -229,7 +234,7 @@ public static function dataGeneralize(): array
229234
IntegerRangeType::fromInterval(null, 16),
230235
'int<min, 16>',
231236
],
232-
];
237+
]);
233238
}
234239

235240
#[DataProvider('dataGeneralize')]

‎tests/PHPStan/Analyser/nsrt/bug-13637.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ function doesNotWork(): void
1717
}
1818

1919
// The reported regression (innermost values widening to `int<0, max>`) is
20-
// fixed: they stay `int<0, 4>`. The middle key degenerates to `int` rather
21-
// than the ideal `int<0, 8>` — a minor key-precision residual in 3-level
22-
// nesting, not the value-widening bug from the issue.
23-
assertType('non-empty-array<int<0, 4>, non-empty-array<int, non-empty-array<int<1, 9>, array{abc: int<0, 4>, def?: int<0, 4>, ghi?: int<0, 4>}>>>', $final);
20+
// fixed: they stay `int<0, 4>`, and the middle key keeps the loop's
21+
// `int<0, 8>` bound now that widening no longer expands optional-key
22+
// shapes into every variant before merging them back.
23+
assertType('non-empty-array<int<0, 4>, non-empty-array<int<0, 8>, non-empty-array{3?: array{abc: int<0, 4>, def?: int<0, 4>, ghi?: int<0, 4>}, 1?: array{abc: int<0, 4>, def?: int<0, 4>, ghi?: int<0, 4>}, 2?: array{abc: int<0, 4>, def?: int<0, 4>, ghi?: int<0, 4>}, 4?: array{abc: int<0, 4>, def?: int<0, 4>, ghi?: int<0, 4>}, 5?: array{abc: int<0, 4>, def?: int<0, 4>, ghi?: int<0, 4>}, 6?: array{abc: int<0, 4>, def?: int<0, 4>, ghi?: int<0, 4>}, 7?: array{abc: int<0, 4>, def?: int<0, 4>, ghi?: int<0, 4>}, 8?: array{abc: int<0, 4>, def?: int<0, 4>, ghi?: int<0, 4>}, 9?: array{abc: int<0, 4>, def?: int<0, 4>, ghi?: int<0, 4>}}>>', $final);
2424
}
2525

2626
function thisWorks(): void

‎tests/bench/data/loop-widening-optional-keys.php‎

Lines changed: 272 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,272 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace BenchLoopWideningOptionalKeys;
4+
5+
final class Answer
6+
{
7+
8+
/** @var list<string> */
9+
public array $list1 = [];
10+
11+
/** @var list<string> */
12+
public array $list2 = [];
13+
14+
/** @var list<string> */
15+
public array $list3 = [];
16+
17+
/** @var list<string> */
18+
public array $list4 = [];
19+
20+
/** @var list<string> */
21+
public array $list5 = [];
22+
23+
/** @var list<string> */
24+
public array $list6 = [];
25+
26+
/** @var list<string> */
27+
public array $list7 = [];
28+
29+
/** @var list<string> */
30+
public array $list8 = [];
31+
32+
/** @var list<string> */
33+
public array $list9 = [];
34+
35+
/** @var list<string> */
36+
public array $loop1 = [];
37+
38+
/** @var list<string> */
39+
public array $loop2 = [];
40+
41+
/** @var list<string> */
42+
public array $loop3 = [];
43+
44+
}
45+
46+
/**
47+
* Regression test for loop widening expanding a shape with optional keys into every variant.
48+
*
49+
* Nine conditional writes give $values nine optional keys, and the append in each loop makes
50+
* the loop's key optional too. Widening the loop's scope flattened both sides with
51+
* TypeUtils::flattenTypes(), which turns a shape with N optional keys into its 2^N concrete
52+
* variants, only for the union right after to merge the thousand of them back into the same
53+
* shape pair by pair. MutatingScope::generalizeType() now splits only unions.
54+
*
55+
* `bin/phpstan analyse -l 8 --debug` on this file: 4.6 s on 2.3.x, 1.5 s with the fix.
56+
*/
57+
final class Analysis
58+
{
59+
60+
/**
61+
* @return array<string, list<string>>
62+
*/
63+
public function convert1(Answer $answer): array
64+
{
65+
$values = ['fields' => ['answer']];
66+
if ($answer->list1 !== []) {
67+
$values['list1'] = array_map(static fn (string $value): string => strtolower($value), $answer->list1);
68+
}
69+
if ($answer->list2 !== []) {
70+
$values['list2'] = array_map(static fn (string $value): string => strtolower($value), $answer->list2);
71+
}
72+
if ($answer->list3 !== []) {
73+
$values['list3'] = array_map(static fn (string $value): string => strtolower($value), $answer->list3);
74+
}
75+
if ($answer->list4 !== []) {
76+
$values['list4'] = array_map(static fn (string $value): string => strtolower($value), $answer->list4);
77+
}
78+
if ($answer->list5 !== []) {
79+
$values['list5'] = array_map(static fn (string $value): string => strtolower($value), $answer->list5);
80+
}
81+
if ($answer->list6 !== []) {
82+
$values['list6'] = array_map(static fn (string $value): string => strtolower($value), $answer->list6);
83+
}
84+
if ($answer->list7 !== []) {
85+
$values['list7'] = array_map(static fn (string $value): string => strtolower($value), $answer->list7);
86+
}
87+
if ($answer->list8 !== []) {
88+
$values['list8'] = array_map(static fn (string $value): string => strtolower($value), $answer->list8);
89+
}
90+
if ($answer->list9 !== []) {
91+
$values['list9'] = array_map(static fn (string $value): string => strtolower($value), $answer->list9);
92+
}
93+
94+
foreach ($answer->loop1 as $value) {
95+
if ($value !== '') {
96+
$values['items'][] = $value;
97+
}
98+
}
99+
foreach ($answer->loop2 as $value) {
100+
if ($value !== '') {
101+
$values['items'][] = $value;
102+
}
103+
}
104+
foreach ($answer->loop3 as $value) {
105+
if ($value !== '') {
106+
$values['items'][] = $value;
107+
}
108+
}
109+
110+
return $values;
111+
}
112+
113+
/**
114+
* @return array<string, list<string>>
115+
*/
116+
public function convert2(Answer $answer): array
117+
{
118+
$values = ['fields' => ['answer']];
119+
if ($answer->list1 !== []) {
120+
$values['list1'] = array_map(static fn (string $value): string => strtolower($value), $answer->list1);
121+
}
122+
if ($answer->list2 !== []) {
123+
$values['list2'] = array_map(static fn (string $value): string => strtolower($value), $answer->list2);
124+
}
125+
if ($answer->list3 !== []) {
126+
$values['list3'] = array_map(static fn (string $value): string => strtolower($value), $answer->list3);
127+
}
128+
if ($answer->list4 !== []) {
129+
$values['list4'] = array_map(static fn (string $value): string => strtolower($value), $answer->list4);
130+
}
131+
if ($answer->list5 !== []) {
132+
$values['list5'] = array_map(static fn (string $value): string => strtolower($value), $answer->list5);
133+
}
134+
if ($answer->list6 !== []) {
135+
$values['list6'] = array_map(static fn (string $value): string => strtolower($value), $answer->list6);
136+
}
137+
if ($answer->list7 !== []) {
138+
$values['list7'] = array_map(static fn (string $value): string => strtolower($value), $answer->list7);
139+
}
140+
if ($answer->list8 !== []) {
141+
$values['list8'] = array_map(static fn (string $value): string => strtolower($value), $answer->list8);
142+
}
143+
if ($answer->list9 !== []) {
144+
$values['list9'] = array_map(static fn (string $value): string => strtolower($value), $answer->list9);
145+
}
146+
147+
foreach ($answer->loop1 as $value) {
148+
if ($value !== '') {
149+
$values['items'][] = $value;
150+
}
151+
}
152+
foreach ($answer->loop2 as $value) {
153+
if ($value !== '') {
154+
$values['items'][] = $value;
155+
}
156+
}
157+
foreach ($answer->loop3 as $value) {
158+
if ($value !== '') {
159+
$values['items'][] = $value;
160+
}
161+
}
162+
163+
return $values;
164+
}
165+
166+
/**
167+
* @return array<string, list<string>>
168+
*/
169+
public function convert3(Answer $answer): array
170+
{
171+
$values = ['fields' => ['answer']];
172+
if ($answer->list1 !== []) {
173+
$values['list1'] = array_map(static fn (string $value): string => strtolower($value), $answer->list1);
174+
}
175+
if ($answer->list2 !== []) {
176+
$values['list2'] = array_map(static fn (string $value): string => strtolower($value), $answer->list2);
177+
}
178+
if ($answer->list3 !== []) {
179+
$values['list3'] = array_map(static fn (string $value): string => strtolower($value), $answer->list3);
180+
}
181+
if ($answer->list4 !== []) {
182+
$values['list4'] = array_map(static fn (string $value): string => strtolower($value), $answer->list4);
183+
}
184+
if ($answer->list5 !== []) {
185+
$values['list5'] = array_map(static fn (string $value): string => strtolower($value), $answer->list5);
186+
}
187+
if ($answer->list6 !== []) {
188+
$values['list6'] = array_map(static fn (string $value): string => strtolower($value), $answer->list6);
189+
}
190+
if ($answer->list7 !== []) {
191+
$values['list7'] = array_map(static fn (string $value): string => strtolower($value), $answer->list7);
192+
}
193+
if ($answer->list8 !== []) {
194+
$values['list8'] = array_map(static fn (string $value): string => strtolower($value), $answer->list8);
195+
}
196+
if ($answer->list9 !== []) {
197+
$values['list9'] = array_map(static fn (string $value): string => strtolower($value), $answer->list9);
198+
}
199+
200+
foreach ($answer->loop1 as $value) {
201+
if ($value !== '') {
202+
$values['items'][] = $value;
203+
}
204+
}
205+
foreach ($answer->loop2 as $value) {
206+
if ($value !== '') {
207+
$values['items'][] = $value;
208+
}
209+
}
210+
foreach ($answer->loop3 as $value) {
211+
if ($value !== '') {
212+
$values['items'][] = $value;
213+
}
214+
}
215+
216+
return $values;
217+
}
218+
219+
/**
220+
* @return array<string, list<string>>
221+
*/
222+
public function convert4(Answer $answer): array
223+
{
224+
$values = ['fields' => ['answer']];
225+
if ($answer->list1 !== []) {
226+
$values['list1'] = array_map(static fn (string $value): string => strtolower($value), $answer->list1);
227+
}
228+
if ($answer->list2 !== []) {
229+
$values['list2'] = array_map(static fn (string $value): string => strtolower($value), $answer->list2);
230+
}
231+
if ($answer->list3 !== []) {
232+
$values['list3'] = array_map(static fn (string $value): string => strtolower($value), $answer->list3);
233+
}
234+
if ($answer->list4 !== []) {
235+
$values['list4'] = array_map(static fn (string $value): string => strtolower($value), $answer->list4);
236+
}
237+
if ($answer->list5 !== []) {
238+
$values['list5'] = array_map(static fn (string $value): string => strtolower($value), $answer->list5);
239+
}
240+
if ($answer->list6 !== []) {
241+
$values['list6'] = array_map(static fn (string $value): string => strtolower($value), $answer->list6);
242+
}
243+
if ($answer->list7 !== []) {
244+
$values['list7'] = array_map(static fn (string $value): string => strtolower($value), $answer->list7);
245+
}
246+
if ($answer->list8 !== []) {
247+
$values['list8'] = array_map(static fn (string $value): string => strtolower($value), $answer->list8);
248+
}
249+
if ($answer->list9 !== []) {
250+
$values['list9'] = array_map(static fn (string $value): string => strtolower($value), $answer->list9);
251+
}
252+
253+
foreach ($answer->loop1 as $value) {
254+
if ($value !== '') {
255+
$values['items'][] = $value;
256+
}
257+
}
258+
foreach ($answer->loop2 as $value) {
259+
if ($value !== '') {
260+
$values['items'][] = $value;
261+
}
262+
}
263+
foreach ($answer->loop3 as $value) {
264+
if ($value !== '') {
265+
$values['items'][] = $value;
266+
}
267+
}
268+
269+
return $values;
270+
}
271+
272+
}

0 commit comments

Comments
 (0)