Skip to content

Commit 5575b29

Browse files
SanderMullerclaude
SanderMuller
and
claude
committed
Price closure returns on the scope after a capped nested walk
The depth guard in ClosureTypeResolver answers a nested closure type with its declared return type. A body walk that hit the guard stored those declared types, and the stored result then fixed the outer return type. Such a walk now prices its returns on the scope, where the guard is one level lower. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent a0d8033 commit 5575b29

7 files changed

Lines changed: 321 additions & 105 deletions

File tree

‎src/Analyser/ExprHandler/Helper/ClosureTypeResolver.php‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,14 @@ final class ClosureTypeResolver implements PerFileAnalysisResettable
6565

6666
private static int $resolveClosureTypeDepth = 0;
6767

68+
/**
69+
* Counts the closure types the $resolveClosureTypeDepth guard answered
70+
* without a walk. A body walk that sees it change stored the types of
71+
* nested closures it could not walk, so its return types are priced again
72+
* on the scope once the guard is lower. See readableWalkStorage().
73+
*/
74+
private static int $cappedClosureTypeWalks = 0;
75+
6876
/**
6977
* Per-context resolved closure types, keyed by the closure node. Node
7078
* attributes would persist on the parser cache's retained ASTs after the
@@ -156,6 +164,7 @@ public function getClosureType(
156164
return $this->createClosureTypeFromCache($scope, $expr, $parameters, $isVariadic, $cachedTypes[$cacheKey], $contextFree);
157165
}
158166
if (self::$resolveClosureTypeDepth >= 2) {
167+
self::$cappedClosureTypeWalks++;
159168
return new ClosureType(
160169
$parameters,
161170
$scope->getFunctionType($expr->returnType, false, false),
@@ -173,6 +182,7 @@ public function getClosureType(
173182
$arrowFunctionImpurePoints = [];
174183
$invalidateExpressions = [];
175184
self::$resolveClosureTypeDepth++;
185+
$cappedBefore = self::$cappedClosureTypeWalks;
176186
$walkStorage = new ExpressionResultStorage();
177187
$arrowScope->pushExpressionResultStorage($walkStorage);
178188
try {
@@ -215,12 +225,13 @@ static function (Node $node, Scope $scope) use ($arrowScope, &$arrowFunctionImpu
215225

216226
// the body was processed just above; resolve the return type from its stored
217227
// result rather than reading the still-unprocessed body expression
218-
$returnType = $this->resolveArrowFunctionReturnType($scope, $arrowScope, $expr, false, $walkStorage);
228+
$returnType = $this->resolveArrowFunctionReturnType($scope, $arrowScope, $expr, false, self::readableWalkStorage($walkStorage, $cappedBefore));
219229

220230
return $this->assembleClosureType($scope, $expr, $parameters, $isVariadic, $returnType, $throwPoints, $impurePoints, $invalidateExpressions, [], $cacheKey, $contextFree);
221231
}
222232

223233
self::$resolveClosureTypeDepth++;
234+
$cappedBefore = self::$cappedClosureTypeWalks;
224235

225236
$closureScope = $scope->enterAnonymousFunctionWithoutReflection($expr, $callableParameters, $nativeCallableParameters);
226237
$closureReturnStatements = [];
@@ -290,11 +301,23 @@ static function (Node $node, Scope $scope) use ($arrowScope, &$arrowFunctionImpu
290301
$invalidateExpressions,
291302
$cacheKey,
292303
false,
293-
$walkStorage,
304+
self::readableWalkStorage($walkStorage, $cappedBefore),
294305
$contextFree,
295306
);
296307
}
297308

309+
/**
310+
* The walk's storage, or null when the depth guard answered a nested
311+
* closure type during the walk. The stored types then hold the declared
312+
* return types of the closures the walk could not reach, and reading
313+
* the returns on the scope after the walk prices those closures with
314+
* the guard one level lower.
315+
*/
316+
private static function readableWalkStorage(ExpressionResultStorage $walkStorage, int $cappedBefore): ?ExpressionResultStorage
317+
{
318+
return self::$cappedClosureTypeWalks === $cappedBefore ? $walkStorage : null;
319+
}
320+
298321
/**
299322
* Constructs a closure type from data the engine already gathered while
300323
* walking the body once (see ClosureProcessor::processClosureNode()),

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<?php // lint >= 8.1
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug15406;
6+
7+
use function PHPStan\Testing\assertType;
8+
9+
final class Node
10+
{
11+
/** @param list<self> $children */
12+
public function __construct(public readonly array $children, public readonly int $value) {}
13+
}
14+
15+
/**
16+
* @template T
17+
* @template U
18+
*
19+
* @param list<T> $list
20+
* @param callable(T): iterable<U> $fn
21+
*
22+
* @return list<U>
23+
*/
24+
function collect(array $list, callable $fn): array
25+
{
26+
$result = [];
27+
foreach ($list as $value) {
28+
foreach ($fn($value) as $item) {
29+
$result[] = $item;
30+
}
31+
}
32+
33+
return $result;
34+
}
35+
36+
/**
37+
* @param list<Node> $nodes
38+
*/
39+
function twoLevels(array $nodes): void
40+
{
41+
assertType('list<int>', collect($nodes, static fn (Node $node): array
42+
=> collect($node->children, static fn (Node $child): array => [$child->value]),
43+
));
44+
}
45+
46+
/**
47+
* @param list<Node> $nodes
48+
*/
49+
function threeLevels(array $nodes): void
50+
{
51+
assertType('list<int>', collect($nodes, static fn (Node $node): array
52+
=> collect($node->children, static fn (Node $child): array
53+
=> collect($child->children, static fn (Node $grandChild): array => [$grandChild->value]),
54+
),
55+
));
56+
}
57+
58+
/**
59+
* @param list<Node> $nodes
60+
*/
61+
function threeLevelsGenerators(array $nodes): void
62+
{
63+
assertType('list<int>', collect($nodes, static fn (Node $node): iterable
64+
=> yield from collect($node->children, static fn (Node $child): iterable
65+
=> yield from collect($child->children, static fn (Node $grandChild): iterable => yield $grandChild->value),
66+
),
67+
));
68+
}

‎tests/PHPStan/Analyser/nsrt/bug-15406b.php‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php // lint >= 8.0
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug15406b;
6+
7+
use function PHPStan\Testing\assertType;
8+
9+
/**
10+
* @template T
11+
*/
12+
final class Box
13+
{
14+
/** @param T $value */
15+
public function __construct(public mixed $value)
16+
{
17+
}
18+
19+
/**
20+
* @template U
21+
* @param callable(T): U $callback
22+
* @return Box<U>
23+
*/
24+
public function map(callable $callback): Box
25+
{
26+
return new Box($callback($this->value));
27+
}
28+
}
29+
30+
/**
31+
* @template T
32+
* @param callable(): T $callback
33+
* @return T
34+
*/
35+
function run(callable $callback): mixed
36+
{
37+
return $callback();
38+
}
39+
40+
function threeLevels(): void
41+
{
42+
$box = new Box(1);
43+
assertType('Bug15406b\\Box<Bug15406b\\Box<Bug15406b\\Box<array{k: 1}>>>', $box->map(fn (int $i): Box => (new Box($i))->map(fn (int $j): Box => (new Box($j))->map(fn (int $k): array => ['k' => $k]))));
44+
}
45+
46+
function twoLevels(): void
47+
{
48+
/** @var Box<int> $box */
49+
$box = new Box(1);
50+
assertType('Bug15406b\\Box<Bug15406b\\Box<array{j: int}>>', $box->map(fn (int $i): Box => (new Box($i))->map(fn (int $j): array => ['j' => $j])));
51+
}
52+
53+
run(function (): void {
54+
$box = new Box(1);
55+
assertType('Bug15406b\\Box<Bug15406b\\Box<array{j: 1}>>', $box->map(fn (int $i): Box => (new Box($i))->map(fn (int $j): array => ['j' => $j])));
56+
57+
/** @var Box<int> $box */
58+
$box = new Box(1);
59+
assertType('Bug15406b\\Box<Bug15406b\\Box<array{j: int}>>', $box->map(fn (int $i): Box => (new Box($i))->map(fn (int $j): array => ['j' => $j])));
60+
});

‎tests/PHPStan/Rules/Functions/CallToFunctionParametersRuleTest.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3350,6 +3350,14 @@ public function testBug15432(): void
33503350
]);
33513351
}
33523352

3353+
#[RequiresPhp('>= 8.1.0')]
3354+
public function testBug15406(): void
3355+
{
3356+
$this->checkExplicitMixed = true;
3357+
$this->checkImplicitMixed = true;
3358+
$this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-15406.php'], []);
3359+
}
3360+
33533361
public function testBug15487(): void
33543362
{
33553363
$this->checkExplicitMixed = true;

‎tests/bench/data/bug-15406.php‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug15406Bench;
4+
5+
// closures that call themselves by reference, nested eight levels deep
6+
function run(): int
7+
{
8+
$f1 = function (int $n1) use (&$f1): int {
9+
$f2 = function (int $n2) use (&$f2): int {
10+
$f3 = function (int $n3) use (&$f3): int {
11+
$f4 = function (int $n4) use (&$f4): int {
12+
$f5 = function (int $n5) use (&$f5): int {
13+
$f6 = function (int $n6) use (&$f6): int {
14+
$f7 = function (int $n7) use (&$f7): int {
15+
$f8 = function (int $n8) use (&$f8): int {
16+
return $n8 > 0 ? $f8($n8 - 1) + array_sum(array_map(fn (int $x): int => $f8($x), [1])) : 0;
17+
};
18+
return $n7 > 0 ? $f7($n7 - 1) + array_sum(array_map(fn (int $x): int => $f7($x), [1])) : 0;
19+
};
20+
return $n6 > 0 ? $f6($n6 - 1) + array_sum(array_map(fn (int $x): int => $f6($x), [1])) : 0;
21+
};
22+
return $n5 > 0 ? $f5($n5 - 1) + array_sum(array_map(fn (int $x): int => $f5($x), [1])) : 0;
23+
};
24+
return $n4 > 0 ? $f4($n4 - 1) + array_sum(array_map(fn (int $x): int => $f4($x), [1])) : 0;
25+
};
26+
return $n3 > 0 ? $f3($n3 - 1) + array_sum(array_map(fn (int $x): int => $f3($x), [1])) : 0;
27+
};
28+
return $n2 > 0 ? $f2($n2 - 1) + array_sum(array_map(fn (int $x): int => $f2($x), [1])) : 0;
29+
};
30+
return $n1 > 0 ? $f1($n1 - 1) + array_sum(array_map(fn (int $x): int => $f1($x), [1])) : 0;
31+
};
32+
33+
return $f1(3);
34+
}

‎turbo-ext/src/ClosureTypeResolver.cpp‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66
* constructor keeps the twin's arginfo so Nette autowires it, the per-file
77
* cache lives in the twin's $cachedTypes slot with the twin's shape
88
* (spl_object_id => ['expr' => ..., 'types' => [cacheKey => [...]]]) and is
9-
* updated in place. The private static $resolveClosureTypeDepth guard is a
10-
* C++ counter (the declared static stays for reflection parity).
9+
* updated in place. The private static $resolveClosureTypeDepth guard and
10+
* $cappedClosureTypeWalks are C++ counters (the declared statics stay for
11+
* reflection parity).
1112
*
1213
* getClosureType(), buildClosureTypeForClosure(),
1314
* buildClosureTypeForArrowFunction() and getDeclaredClosureType() are exported
@@ -58,6 +59,9 @@ zend_string *pt_ctr_used_variables = nullptr;
5859
/* the private static $resolveClosureTypeDepth */
5960
int pt_ctr_resolve_closure_type_depth = 0;
6061

62+
/* the private static $cappedClosureTypeWalks */
63+
int pt_ctr_capped_closure_type_walks = 0;
64+
6165
/* {{{ the PHP collaborators (one site each; switch to their direct entries
6266
* once they are ported) */
6367

@@ -355,7 +359,10 @@ class ClosureTypeResolver
355359
ZVAL_DEREF(cached);
356360
return createClosureTypeFromCache(scope, expr, p.parameters.raw(), p.isVariadic, cached, p.contextFree);
357361
}
358-
if (pt_ctr_resolve_closure_type_depth >= 2) return signatureClosureType(scope, expr, p.parameters.raw(), p.isVariadic);
362+
if (pt_ctr_resolve_closure_type_depth >= 2) {
363+
pt_ctr_capped_closure_type_walks++;
364+
return signatureClosureType(scope, expr, p.parameters.raw(), p.isVariadic);
365+
}
359366

360367
if (UNEXPECTED(!countClosureTypeBodyWalk())) return zv::Val();
361368
int isArrowFunction = ptclosure::instanceOf(expr, PT_CLASS_ARROW_FUNCTION);
@@ -1336,6 +1343,12 @@ class ClosureTypeResolver
13361343
zend_hash_index_update(Z_ARRVAL_P(cachedTypes), id, &value);
13371344
}
13381345

1346+
/* Mirrors readableWalkStorage() (NULL for null) */
1347+
static zval *readableWalkStorage(zval *walkStorage, int cappedBefore)
1348+
{
1349+
return pt_ctr_capped_closure_type_walks == cappedBefore ? walkStorage : NULL;
1350+
}
1351+
13391352
/* the arrow-function body walk of getClosureType() */
13401353
zv::Val walkArrowFunction(zval *scope, zval *expr, Parameters &p, zend_string *cacheKey)
13411354
{
@@ -1354,6 +1367,7 @@ class ClosureTypeResolver
13541367
zv::Val arrowFunctionImpurePoints = ptsh::newArrayReference();
13551368
zv::Val invalidateExpressions = ptsh::newArrayReference();
13561369
pt_ctr_resolve_closure_type_depth++;
1370+
int cappedBefore = pt_ctr_capped_closure_type_walks;
13571371
zv::Val walkStorage = pt_expression_result_storage_new();
13581372
if (UNEXPECTED(walkStorage.isUndef())) return zv::Val();
13591373
if (UNEXPECTED(!pt_mutating_scope_push_expression_result_storage(Z_OBJ_P(arrowScope.raw()), walkStorage.raw()))) return zv::Val();
@@ -1390,7 +1404,7 @@ class ClosureTypeResolver
13901404

13911405
// the body was processed just above; resolve the return type from its stored
13921406
// result rather than reading the still-unprocessed body expression
1393-
zv::Val returnType = resolveArrowFunctionReturnType(scope, arrowScope.raw(), expr, false, walkStorage.raw());
1407+
zv::Val returnType = resolveArrowFunctionReturnType(scope, arrowScope.raw(), expr, false, readableWalkStorage(walkStorage.raw(), cappedBefore));
13941408
if (UNEXPECTED(returnType.isUndef())) return zv::Val();
13951409

13961410
zv::Arr usedVariables = zv::Arr::empty();
@@ -1401,6 +1415,7 @@ class ClosureTypeResolver
14011415
zv::Val walkClosure(zval *scope, zval *expr, Parameters &p, zend_string *cacheKey)
14021416
{
14031417
pt_ctr_resolve_closure_type_depth++;
1418+
int cappedBefore = pt_ctr_capped_closure_type_walks;
14041419

14051420
zval null;
14061421
ZVAL_NULL(&null);
@@ -1461,7 +1476,7 @@ class ClosureTypeResolver
14611476
zv::Val impurePoints = mergeLists(Z_REFVAL_P(closureImpurePoints.raw()), resultImpurePoints);
14621477
if (UNEXPECTED(impurePoints.isUndef())) return zv::Val();
14631478

1464-
return buildClosureTypeFromClosureWalk(scope, expr, p.parameters.raw(), p.isVariadic, Z_REFVAL_P(closureReturnStatements.raw()), Z_REFVAL_P(closureYieldStatements.raw()), Z_REFVAL_P(closureExecutionEnds.raw()), throwPointsOwned.raw(), impurePoints.raw(), Z_REFVAL_P(invalidateExpressions.raw()), cacheKey, false, walkStorage.raw(), p.contextFree);
1479+
return buildClosureTypeFromClosureWalk(scope, expr, p.parameters.raw(), p.isVariadic, Z_REFVAL_P(closureReturnStatements.raw()), Z_REFVAL_P(closureYieldStatements.raw()), Z_REFVAL_P(closureExecutionEnds.raw()), throwPointsOwned.raw(), impurePoints.raw(), Z_REFVAL_P(invalidateExpressions.raw()), cacheKey, false, readableWalkStorage(walkStorage.raw(), cappedBefore), p.contextFree);
14651480
}
14661481
};
14671482

0 commit comments

Comments
 (0)