Skip to content

Commit 7dfe139

Browse files
phpstan-botondrejmirtes
phpstan-bot
authored and
ondrejmirtes
committed
Do not infer by-ref use effects at invocations for a closure assigned to its own by-ref variable
- ClosureSignatureInference::getByRefUseMarkers() returns no markers for a closure that is the right side of an assignment to one of its by-ref uses (`$f = function () use (&$f)`). The body can reach the closure itself through that variable in ways the invocation tracking does not follow, so those by-ref uses keep the creation-time fixpoint. Before this, a body that only reached itself indirectly (a nested closure or arrow function capturing `$f`, an alias, `call_user_func($f)`, `$f->__invoke()`, or just reading `$f`) was entered with `$f` as it was before the assignment: `null`. - getAssignedClosures() takes a `$nested` flag (cached under its own attribute) to also look inside nested function-likes. A closure created in another closure's body is observed through the enclosing function's frame. - The turbo-ext C++ mirror (ClosureSignatureInference.cpp and its generated header) is updated the same way. - Considered first: making the existing "body invokes its own by-ref variable" guard follow captures into nested closures and arrow functions. That fixes the reported nested case but misses aliasing, call_user_func, __invoke and plain reads. It also makes mutual recursion through a captured variable less precise, so I didn't use it.
1 parent 4ef9ab7 commit 7dfe139

6 files changed

Lines changed: 205 additions & 19 deletions

File tree

‎src/Analyser/Generics/ClosureSignatureInference.php‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ final class ClosureSignatureInference
8787

8888
private const ASSIGNED_CLOSURES_ATTRIBUTE = 'closureSignatureAssignedClosures';
8989

90+
private const NESTED_ASSIGNED_CLOSURES_ATTRIBUTE = 'closureSignatureNestedAssignedClosures';
91+
9092
private const REBOUND_VARIABLES_ATTRIBUTE = 'closureSignatureReboundVariables';
9193

9294
public function __construct(
@@ -118,7 +120,7 @@ public static function isByRefMarker(UnresolvedTemplateArgumentType $marker): bo
118120
* by variable name - while observing, and in the second pass for a site the
119121
* observation resolved. Empty for a closure whose invocations cannot be
120122
* followed: a generator (invoking it does not run the body), a closure
121-
* invoking its own by-ref variable.
123+
* invoking its own by-ref variable or assigned to it.
122124
*
123125
* @return array<string, Type>
124126
*/
@@ -143,12 +145,19 @@ public function getByRefUseMarkers(MutatingScope $scope, Closure|ArrowFunction $
143145
return [];
144146
}
145147
$reboundNames = self::getReboundVariableNames($body, $frame->getClosureSignatureStmts());
148+
$assignedClosures = self::getAssignedClosures($body, $frame->getClosureSignatureStmts(), true);
146149
foreach ($names as $name) {
147150
// unset or bound to another reference, the variable no longer is the
148151
// one the closure writes to - its effects stay the creation-time fixpoint
149152
if (isset($reboundNames[$name])) {
150153
return [];
151154
}
155+
// `$f = function () use (&$f)`: the body reaches the closure itself
156+
// through the variable - aliased, captured, passed on - so its
157+
// invocations cannot all be followed
158+
if (in_array($expr, $assignedClosures[$name] ?? [], true)) {
159+
return [];
160+
}
152161
}
153162

154163
$markers = [];
@@ -1120,13 +1129,17 @@ public function findAssignedClosures(MutatingScope $scope, string $name): array
11201129
}
11211130

11221131
/**
1132+
* With $nested, also the closures assigned in the function-likes the body
1133+
* contains - to the variables of those.
1134+
*
11231135
* @param Node\Stmt[] $stmts
11241136
* @return array<string, list<Closure|ArrowFunction>>
11251137
*/
1126-
private static function getAssignedClosures(Node $body, array $stmts): array
1138+
private static function getAssignedClosures(Node $body, array $stmts, bool $nested = false): array
11271139
{
1140+
$attribute = $nested ? self::NESTED_ASSIGNED_CLOSURES_ATTRIBUTE : self::ASSIGNED_CLOSURES_ATTRIBUTE;
11281141
/** @var array<string, list<Closure|ArrowFunction>>|null $cached */
1129-
$cached = $body->getAttribute(self::ASSIGNED_CLOSURES_ATTRIBUTE);
1142+
$cached = $body->getAttribute($attribute);
11301143
if ($cached !== null) {
11311144
return $cached;
11321145
}
@@ -1135,7 +1148,7 @@ private static function getAssignedClosures(Node $body, array $stmts): array
11351148
$stack = $stmts;
11361149
while (count($stack) > 0) {
11371150
$node = array_pop($stack);
1138-
if ($node instanceof Node\FunctionLike || $node instanceof Node\Stmt\ClassLike) {
1151+
if (!$nested && ($node instanceof Node\FunctionLike || $node instanceof Node\Stmt\ClassLike)) {
11391152
continue;
11401153
}
11411154
if (
@@ -1160,7 +1173,7 @@ private static function getAssignedClosures(Node $body, array $stmts): array
11601173
}
11611174
}
11621175
}
1163-
$body->setAttribute(self::ASSIGNED_CLOSURES_ATTRIBUTE, $assigned);
1176+
$body->setAttribute($attribute, $assigned);
11641177

11651178
return $assigned;
11661179
}

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

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
<?php // lint >= 7.4
2+
3+
declare(strict_types=1);
4+
5+
namespace Bug15442;
6+
7+
use function PHPStan\Testing\assertType;
8+
9+
class GridLayout
10+
{
11+
protected function buildTemplate(): void
12+
{
13+
$fx = static function () use (&$fx) {
14+
(static function () use ($fx) {
15+
assertType('static-Closure(): void', $fx);
16+
$fx();
17+
18+
dummy(); // @phpstan-ignore-line
19+
})();
20+
};
21+
22+
dummy($fx); // @phpstan-ignore-line
23+
}
24+
25+
public function nestedArrowFunction(): void
26+
{
27+
$fx = static function () use (&$fx) {
28+
(static fn () => $fx())();
29+
assertType('static-Closure(): void', $fx);
30+
};
31+
}
32+
33+
public function nestedByRefCapture(): void
34+
{
35+
$fx = static function () use (&$fx) {
36+
(static function () use (&$fx) {
37+
assertType('static-Closure(): void', $fx);
38+
$fx();
39+
})();
40+
};
41+
}
42+
43+
public function callUserFunc(): void
44+
{
45+
$fx = static function () use (&$fx) {
46+
assertType('static-Closure(): void', $fx);
47+
call_user_func($fx);
48+
};
49+
}
50+
51+
public function invokeMethod(): void
52+
{
53+
$fx = static function () use (&$fx) {
54+
assertType('static-Closure(): void', $fx);
55+
$fx->__invoke();
56+
};
57+
}
58+
59+
public function alias(): void
60+
{
61+
$fx = static function () use (&$fx) {
62+
$alias = $fx;
63+
assertType('static-Closure(): void', $alias);
64+
$alias();
65+
};
66+
}
67+
68+
/** @param list<int> $a */
69+
public function passedAsCallback(array $a): void
70+
{
71+
$fx = static function (array $a) use (&$fx): void {
72+
assertType('static-Closure(array): void', $fx);
73+
array_map($fx, $a);
74+
};
75+
$fx($a);
76+
}
77+
78+
public function readOnly(): void
79+
{
80+
$fx = static function () use (&$fx) {
81+
assertType('static-Closure(): void', $fx);
82+
};
83+
$fx();
84+
}
85+
86+
public function insideClosure(): void
87+
{
88+
$outer = function (): void {
89+
$fx = static function () use (&$fx) {
90+
(static function () use ($fx) {
91+
assertType('static-Closure(): void', $fx);
92+
$fx();
93+
})();
94+
};
95+
$fx();
96+
};
97+
$outer();
98+
}
99+
100+
public function otherByRefUseStillFollowed(): void
101+
{
102+
$x = 1;
103+
$c = function () use (&$x): void {
104+
assertType('1', $x);
105+
(function () use ($x): void {
106+
assertType('1', $x);
107+
})();
108+
$x = 'a';
109+
};
110+
$c();
111+
assertType("'a'", $x);
112+
}
113+
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,12 @@ public function testBug15285(): void
461461
$this->analyse([__DIR__ . '/data/bug-15285.php'], []);
462462
}
463463

464+
#[RequiresPhp('>= 7.4.0')]
465+
public function testBug15442(): void
466+
{
467+
$this->analyse([__DIR__ . '/data/bug-15442.php'], []);
468+
}
469+
464470
#[RequiresPhp('>= 8.0.0')]
465471
public function testBug11935WithCheckExplicitMixed(): void
466472
{

‎tests/PHPStan/Rules/Functions/data/bug-15442.php‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Bug15442Rule;
6+
7+
class GridLayout
8+
{
9+
protected function buildTemplate(): void
10+
{
11+
$fx = static function () use (&$fx) {
12+
(static function () use ($fx) {
13+
$fx();
14+
15+
dummy();
16+
})();
17+
};
18+
19+
dummy($fx);
20+
}
21+
22+
protected function arrowFunction(): void
23+
{
24+
$fx = static function () use (&$fx) {
25+
(static fn () => $fx())();
26+
};
27+
28+
dummy($fx);
29+
}
30+
}

‎turbo-ext/src/ClosureSignatureInference.cpp‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ zend_string *pt_csi_return_template_name = nullptr;
3030
zend_string *pt_csi_closed_body_attribute = nullptr;
3131
zend_string *pt_csi_returns_context_typed_attribute = nullptr;
3232
zend_string *pt_csi_assigned_closures_attribute = nullptr;
33+
zend_string *pt_csi_nested_assigned_closures_attribute = nullptr;
3334
/* the by-ref follow-up's template names and attribute names */
3435
zend_string *pt_csi_invocation_template_name = nullptr;
3536
zend_string *pt_csi_by_ref_uses_attribute = nullptr;
@@ -230,9 +231,10 @@ void containsMarkerBody(zval *contains, zval *state1, uint32_t argc, zval *argv,
230231
}
231232

232233
/* Mirrors the private static getAssignedClosures(); UNDEF = pending exception */
233-
zv::Val getAssignedClosures(zval *body, zval *stmts)
234+
zv::Val getAssignedClosures(zval *body, zval *stmts, bool nested = false)
234235
{
235-
zv::Val cached = pt_engine_node_get_attribute(Z_OBJ_P(body), ZSTR_VAL(pt_csi_assigned_closures_attribute), ZSTR_LEN(pt_csi_assigned_closures_attribute));
236+
zend_string *attribute = nested ? pt_csi_nested_assigned_closures_attribute : pt_csi_assigned_closures_attribute;
237+
zv::Val cached = pt_engine_node_get_attribute(Z_OBJ_P(body), ZSTR_VAL(attribute), ZSTR_LEN(attribute));
236238
if (UNEXPECTED(cached.isUndef())) return zv::Val();
237239
if (!cached.isNull()) return cached;
238240

@@ -246,15 +248,17 @@ zv::Val getAssignedClosures(zval *body, zval *stmts)
246248
for (;;) {
247249
zv::Val node = popNode(stack);
248250
if (node.isUndef()) break;
249-
bool skip;
250-
if (UNEXPECTED(!isA(node.raw(), PT_CLASS_FUNCTION_LIKE, skip))) return zv::Val();
251-
if (!skip && UNEXPECTED(!isA(node.raw(), PT_CLASS_CLASS_LIKE_STMT, skip))) return zv::Val();
252-
if (skip) continue;
251+
if (!nested) {
252+
bool skip;
253+
if (UNEXPECTED(!isA(node.raw(), PT_CLASS_FUNCTION_LIKE, skip))) return zv::Val();
254+
if (!skip && UNEXPECTED(!isA(node.raw(), PT_CLASS_CLASS_LIKE_STMT, skip))) return zv::Val();
255+
if (skip) continue;
256+
}
253257
if (UNEXPECTED(!collectAssignedClosure(node.raw(), assigned))) return zv::Val();
254258
if (UNEXPECTED(!pushSubNodes(node.raw(), stack))) return zv::Val();
255259
}
256260
zv::Val result(std::move(assigned));
257-
if (UNEXPECTED(!pt_engine_node_set_attribute(Z_OBJ_P(body), ZSTR_VAL(pt_csi_assigned_closures_attribute), ZSTR_LEN(pt_csi_assigned_closures_attribute), result.raw()))) return zv::Val();
261+
if (UNEXPECTED(!pt_engine_node_set_attribute(Z_OBJ_P(body), ZSTR_VAL(attribute), ZSTR_LEN(attribute), result.raw()))) return zv::Val();
258262
return result;
259263
}
260264

@@ -1007,13 +1011,28 @@ class ClosureSignatureInference
10071011
if (UNEXPECTED(stmts.isUndef())) return zv::Val();
10081012
zv::Val reboundNames = getReboundVariableNames(body.raw(), stmts.raw());
10091013
if (UNEXPECTED(reboundNames.isUndef())) return zv::Val();
1014+
zv::Val assignedClosures = getAssignedClosures(body.raw(), stmts.raw(), true);
1015+
if (UNEXPECTED(assignedClosures.isUndef())) return zv::Val();
10101016
for (auto entry : zv::ArrRef(names.raw())) {
10111017
zval *name = entry.value().deref().raw();
1018+
if (Z_TYPE_P(name) != IS_STRING) continue;
10121019
// unset or bound to another reference, the variable no longer is the
10131020
// one the closure writes to - its effects stay the creation-time fixpoint
1014-
if (Z_TYPE_P(name) == IS_STRING && zend_symtable_find(Z_ARRVAL_P(reboundNames.raw()), Z_STR_P(name)) != NULL) {
1021+
if (zend_symtable_find(Z_ARRVAL_P(reboundNames.raw()), Z_STR_P(name)) != NULL) {
10151022
return zv::Val(zv::Arr::empty());
10161023
}
1024+
// `$f = function () use (&$f)`: the body reaches the closure itself
1025+
// through the variable - aliased, captured, passed on - so its
1026+
// invocations cannot all be followed
1027+
zval *closures = zend_symtable_find(Z_ARRVAL_P(assignedClosures.raw()), Z_STR_P(name));
1028+
if (closures != NULL && Z_TYPE_P(closures) == IS_ARRAY) {
1029+
for (auto closure : zv::ArrRef(closures)) {
1030+
zval *candidate = closure.value().deref().raw();
1031+
if (Z_TYPE_P(candidate) == IS_OBJECT && Z_OBJ_P(candidate) == Z_OBJ_P(expr)) {
1032+
return zv::Val(zv::Arr::empty());
1033+
}
1034+
}
1035+
}
10171036
}
10181037

10191038
zv::Arr markers = zv::Arr::empty();
@@ -1917,6 +1936,7 @@ PT_MINIT_REGISTRATION(pt_register_closure_signature_inference)
19171936
pt_csi_closed_body_attribute = zend_string_init_interned(PT_LC("closureSignatureClosedBody"), 1);
19181937
pt_csi_returns_context_typed_attribute = zend_string_init_interned(PT_LC("closureSignatureReturnsContextTyped"), 1);
19191938
pt_csi_assigned_closures_attribute = zend_string_init_interned(PT_LC("closureSignatureAssignedClosures"), 1);
1939+
pt_csi_nested_assigned_closures_attribute = zend_string_init_interned(PT_LC("closureSignatureNestedAssignedClosures"), 1);
19201940
pt_csi_invocation_template_name = zend_string_init_interned(PT_LC("@invokes"), 1);
19211941
pt_csi_by_ref_uses_attribute = zend_string_init_interned(PT_LC("closureSignatureByRefUses"), 1);
19221942
pt_csi_arrow_function_outer_variables_attribute = zend_string_init_interned(PT_LC("closureSignatureArrowFunctionOuterVariables"), 1);
@@ -1933,6 +1953,7 @@ PT_MINIT_REGISTRATION(pt_register_closure_signature_inference)
19331953
cls.privateClassConstantString("ARROW_FUNCTION_OUTER_VARIABLES_ATTRIBUTE", "closureSignatureArrowFunctionOuterVariables");
19341954
cls.privateClassConstantString("CLOSED_BODY_ATTRIBUTE", "closureSignatureClosedBody");
19351955
cls.privateClassConstantString("ASSIGNED_CLOSURES_ATTRIBUTE", "closureSignatureAssignedClosures");
1956+
cls.privateClassConstantString("NESTED_ASSIGNED_CLOSURES_ATTRIBUTE", "closureSignatureNestedAssignedClosures");
19361957
cls.privateClassConstantString("REBOUND_VARIABLES_ATTRIBUTE", "closureSignatureReboundVariables");
19371958

19381959
/* the real parameter types: the DI container autowires the service by

‎turbo-ext/src/generated/ClosureSignatureInference.h‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,11 @@ inline constexpr char strings[] =
104104
"scanClosedBody\0" /* 1340 */
105105
"infersInvocationReturnType\0" /* 1355 */
106106
"findAssignedClosures\0" /* 1382 */
107-
"getAssignedClosures\0" /* 1403 */
108-
"collectTargetNames\0" /* 1423 */
109-
"isContextTyped"; /* 1442 */
107+
"nested\0" /* 1403 */
108+
"false\0" /* 1410 */
109+
"getAssignedClosures\0" /* 1416 */
110+
"collectTargetNames\0" /* 1436 */
111+
"isContextTyped"; /* 1455 */
110112
inline constexpr reg::PackedArg args[] = {
111113
reg::packed(0, MAY_BE_BOOL), /* __construct $enabled */
112114
reg::packed(20, 0, 27), /* isClosureSignatureMarker $marker */
@@ -212,6 +214,7 @@ inline constexpr reg::PackedArg args[] = {
212214
reg::packed(79, MAY_BE_ARRAY), /* findAssignedClosures return */
213215
reg::packed(868, 0, 873), /* getAssignedClosures $body */
214216
reg::packed(836, MAY_BE_ARRAY), /* getAssignedClosures $stmts */
217+
reg::packed(1403, MAY_BE_BOOL, reg::NoString, false, false, 1410), /* getAssignedClosures $nested */
215218
reg::packed(79, MAY_BE_ARRAY), /* getAssignedClosures return */
216219
reg::packed(937, 0, 649), /* collectTargetNames $target */
217220
reg::packed(842, MAY_BE_ARRAY, reg::NoString, true, false), /* collectTargetNames $names */
@@ -262,9 +265,9 @@ inline constexpr sigtab::Sig isClosedBody = { { 1327 /* isClosedBody */, 2, 90,
262265
inline constexpr sigtab::Sig scanClosedBody = { { 1340 /* scanClosedBody */, 2, 93, 2, 95, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
263266
inline constexpr sigtab::Sig infersInvocationReturnType = { { 1355 /* infersInvocationReturnType */, 2, 96, 2, 98, ZEND_ACC_PUBLIC } };
264267
inline constexpr sigtab::Sig findAssignedClosures = { { 1382 /* findAssignedClosures */, 2, 99, 2, 101, ZEND_ACC_PUBLIC } };
265-
inline constexpr sigtab::Sig getAssignedClosures = { { 1403 /* getAssignedClosures */, 2, 102, 2, 104, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
266-
inline constexpr sigtab::Sig collectTargetNames = { { 1423 /* collectTargetNames */, 2, 105, 2, 107, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
267-
inline constexpr sigtab::Sig isContextTyped = { { 1442 /* isContextTyped */, 1, 108, 1, 109, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
268+
inline constexpr sigtab::Sig getAssignedClosures = { { 1416 /* getAssignedClosures */, 2, 102, 3, 105, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
269+
inline constexpr sigtab::Sig collectTargetNames = { { 1436 /* collectTargetNames */, 2, 106, 2, 108, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
270+
inline constexpr sigtab::Sig isContextTyped = { { 1455 /* isContextTyped */, 1, 109, 1, 110, ZEND_ACC_PRIVATE | ZEND_ACC_STATIC } };
268271
} // namespace sig
269272

270273
} // namespace ptdecl::ClosureSignatureInference

0 commit comments

Comments
 (0)