Skip to content

Commit d189012

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Root differing scope entries in every variable they spell
getDifferingVariableRoots() rooted a differing expression only in its leading variable, so the two-pass template argument walk replayed a statement assigning $key although $collection->get($key) differed: when the observation pass never tracked the narrowing, the recorded delta had nothing to invalidate and the stale narrowing survived the reassignment. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSGnoYacTfJv3taAhZkj5o
1 parent ab8d6ba commit d189012

5 files changed

Lines changed: 776 additions & 691 deletions

File tree

‎src/Analyser/MutatingScope.php‎

Lines changed: 31 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@
136136
use function ltrim;
137137
use function md5;
138138
use function preg_match;
139+
use function preg_match_all;
139140
use function spl_object_id;
140141
use function sprintf;
141142
use function str_starts_with;
@@ -1542,10 +1543,11 @@ public function withoutMemoizedTypes(): self
15421543
}
15431544

15441545
/**
1545-
* The variables rooting the tracked expressions whose state differs between
1546-
* this scope and $other - a statement mentioning none of them walks the same
1547-
* on both - or null when a differing entry has no variable root (a static
1548-
* property, a class constant fetch).
1546+
* The variables spelled by the tracked expressions whose state differs
1547+
* between this scope and $other - a statement mentioning none of them walks
1548+
* the same on both - or null when a differing entry has no variable root (a
1549+
* static property, a class constant fetch). Every variable of an entry counts,
1550+
* not just its root: assigning $b invalidates $a->get($b).
15491551
*
15501552
* @return list<string>|null
15511553
*/
@@ -1565,21 +1567,25 @@ public function getDifferingVariableRoots(self $other): ?array
15651567
if ($theirHolder !== null && ($theirHolder === $holder || $holder->equals($theirHolder))) {
15661568
continue;
15671569
}
1568-
$root = self::getVariableRootOfExpressionKey($key);
1569-
if ($root === null) {
1570+
$variables = self::getVariablesOfExpressionKey($key);
1571+
if ($variables === null) {
15701572
return null;
15711573
}
1572-
$roots[$root] = true;
1574+
foreach ($variables as $variable) {
1575+
$roots[$variable] = true;
1576+
}
15731577
}
15741578
foreach ($theirs as $key => $holder) {
15751579
if (isset($ours[$key]) || $holder->getExpr() instanceof PossiblyImpureCallExpr) {
15761580
continue;
15771581
}
1578-
$root = self::getVariableRootOfExpressionKey($key);
1579-
if ($root === null) {
1582+
$variables = self::getVariablesOfExpressionKey($key);
1583+
if ($variables === null) {
15801584
return null;
15811585
}
1582-
$roots[$root] = true;
1586+
foreach ($variables as $variable) {
1587+
$roots[$variable] = true;
1588+
}
15831589
}
15841590
}
15851591
$conditionalTables = [
@@ -1591,28 +1597,38 @@ public function getDifferingVariableRoots(self $other): ?array
15911597
if (isset($theirs[$key]) && $theirs[$key] === $holders) {
15921598
continue;
15931599
}
1594-
$root = self::getVariableRootOfExpressionKey($key);
1595-
if ($root === null) {
1600+
$variables = self::getVariablesOfExpressionKey($key);
1601+
if ($variables === null) {
15961602
foreach ($holders as $holder) {
15971603
if (!$holder->getTypeHolder()->getExpr() instanceof PossiblyImpureCallExpr) {
15981604
return null;
15991605
}
16001606
}
16011607
continue;
16021608
}
1603-
$roots[$root] = true;
1609+
foreach ($variables as $variable) {
1610+
$roots[$variable] = true;
1611+
}
16041612
}
16051613
}
16061614

16071615
return array_keys($roots);
16081616
}
16091617

1610-
private static function getVariableRootOfExpressionKey(string $key): ?string
1618+
/**
1619+
* The variables an expression key spells, or null when the key is not
1620+
* rooted in a variable.
1621+
*
1622+
* @return list<string>|null
1623+
*/
1624+
private static function getVariablesOfExpressionKey(string $key): ?array
16111625
{
1612-
if (preg_match('/^\$([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)/', $key, $matches) !== 1) {
1626+
if (preg_match('/^\$[a-zA-Z_\x80-\xff]/', $key) !== 1) {
16131627
return null;
16141628
}
16151629

1630+
preg_match_all('/\$([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)/', $key, $matches);
1631+
16161632
return $matches[1];
16171633
}
16181634

‎tests/PHPStan/Analyser/nsrt/template-argument-replay-invalidation.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace TemplateArgumentReplayInvalidation;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
/** @template T */
8+
class Collection
9+
{
10+
11+
/** @param T|null $value */
12+
public function __construct($value = null)
13+
{
14+
}
15+
16+
/** @return T|null */
17+
public function get(int $key)
18+
{
19+
return null;
20+
}
21+
22+
}
23+
24+
/** @param Collection<string> $collection */
25+
function takeStrings(Collection $collection): void
26+
{
27+
}
28+
29+
function reassignedArgument(): void
30+
{
31+
$key = rand();
32+
$collection = new Collection(null);
33+
if ($collection->get($key) === null) {
34+
return;
35+
}
36+
assertType('string', $collection->get($key));
37+
if (is_int($collection->get($key))) {
38+
$key = 1;
39+
}
40+
$key = 5;
41+
assertType('string|null', $collection->get($key));
42+
takeStrings($collection);
43+
}

‎turbo-ext/src/MutatingScope.cpp‎

Lines changed: 38 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3085,7 +3085,7 @@ class MutatingScope
30853085
return thisDuplicateWith(args);
30863086
}
30873087

3088-
/* The variables rooting the tracked expressions whose state differs
3088+
/* The variables spelled by the tracked expressions whose state differs
30893089
* between this scope and $other; null when a differing entry has no
30903090
* variable root. $other is an instance of this class (a subclass
30913091
* included) — its slots are read directly. */
@@ -3139,10 +3139,9 @@ class MutatingScope
31393139
for (auto entry : zv::TableRef(table.ours)) {
31403140
zval *theirHolders = findByEntryKey(table.theirs, entry);
31413141
if (theirHolders != NULL && Z_TYPE_P(theirHolders) != IS_NULL && zend_is_identical(theirHolders, entry.value().raw())) continue;
3142-
zend_string *key = entryKeyString(entry, "getVariableRootOfExpressionKey", "key");
3142+
zend_string *key = entryKeyString(entry, "getVariablesOfExpressionKey", "key");
31433143
if (UNEXPECTED(key == NULL)) return zv::Val();
3144-
zv::Str root = getVariableRootOfExpressionKey(key);
3145-
if (root.isNull()) {
3144+
if (!addVariablesOfExpressionKey(roots, key)) {
31463145
zv::Ref holders = entry.value().deref();
31473146
if (UNEXPECTED(!holders.isArray())) {
31483147
zend_error(E_WARNING, "foreach() argument must be of type array|object, %s given", zend_zval_value_name(holders.raw()));
@@ -3163,7 +3162,6 @@ class MutatingScope
31633162
}
31643163
continue;
31653164
}
3166-
roots.set(root.get(), zv::Val::boolean(true));
31673165
}
31683166
}
31693167

@@ -3175,39 +3173,52 @@ class MutatingScope
31753173
return zv::Val(std::move(keys));
31763174
}
31773175

3178-
/* $root = self::getVariableRootOfExpressionKey($key); `noRoot` when
3179-
* it is null (the caller returns null), else $roots[$root] = true;
3180-
* false = pending exception */
3176+
/* $variables = self::getVariablesOfExpressionKey($key); `noRoot` when
3177+
* it is null (the caller returns null), else $roots[$variable] = true
3178+
* for each; false = pending exception */
31813179
[[nodiscard]] static bool addVariableRoot(zv::Arr &roots, const zv::ArrayEntry &entry, bool &noRoot)
31823180
{
3183-
zend_string *key = entryKeyString(entry, "getVariableRootOfExpressionKey", "key");
3181+
zend_string *key = entryKeyString(entry, "getVariablesOfExpressionKey", "key");
31843182
if (UNEXPECTED(key == NULL)) return false;
3185-
zv::Str root = getVariableRootOfExpressionKey(key);
3186-
noRoot = root.isNull();
3187-
if (!noRoot) {
3188-
roots.set(root.get(), zv::Val::boolean(true));
3189-
}
3183+
noRoot = !addVariablesOfExpressionKey(roots, key);
31903184
return true;
31913185
}
31923186

3193-
/* private static: preg_match('/^\$([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)/'); NULL for no match */
3194-
static zv::Str getVariableRootOfExpressionKey(zend_string *key)
3187+
static bool isVariableNameStart(unsigned char c)
3188+
{
3189+
return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_' || c >= 0x80;
3190+
}
3191+
3192+
/* private static getVariablesOfExpressionKey(): false when the key is not
3193+
* rooted in a variable (preg_match('/^\$[a-zA-Z_\x80-\xff]/') fails), else
3194+
* $roots[$variable] = true for each preg_match_all(
3195+
* '/\$([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*)/') match */
3196+
static bool addVariablesOfExpressionKey(zv::Arr &roots, zend_string *key)
31953197
{
31963198
const unsigned char *s = (const unsigned char *) ZSTR_VAL(key);
31973199
size_t n = ZSTR_LEN(key);
3198-
if (n < 2 || s[0] != '$') return zv::Str();
3199-
unsigned char c = s[1];
3200-
if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_' || c >= 0x80)) return zv::Str();
3201-
size_t end = 2;
3202-
while (end < n) {
3203-
c = s[end];
3204-
if ((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '_' || c >= 0x80) {
3205-
end++;
3206-
} else {
3207-
break;
3200+
if (n < 2 || s[0] != '$' || !isVariableNameStart(s[1])) return false;
3201+
size_t i = 0;
3202+
while (i + 1 < n) {
3203+
if (s[i] != '$' || !isVariableNameStart(s[i + 1])) {
3204+
i++;
3205+
continue;
32083206
}
3207+
size_t end = i + 2;
3208+
while (end < n) {
3209+
unsigned char c = s[end];
3210+
if (isVariableNameStart(c) || (c >= '0' && c <= '9')) {
3211+
end++;
3212+
} else {
3213+
break;
3214+
}
3215+
}
3216+
zend_string *name = zend_string_init((const char *) s + i + 1, end - i - 1, 0);
3217+
roots.set(name, zv::Val::boolean(true));
3218+
zend_string_release(name);
3219+
i = end;
32093220
}
3210-
return zv::Str::adopt(zend_string_init((const char *) s + 1, end - 1, 0));
3221+
return true;
32113222
}
32123223

32133224
/* This scope after a statement whose recorded walk stands. */

0 commit comments

Comments
 (0)