From 4e116689b3ca67ac566358d24b745d237be78e2c Mon Sep 17 00:00:00 2001 From: "ondrej@mirtes.cz" Date: Sun, 15 Nov 2026 10:46:55 +0000 Subject: [PATCH] Keep only the current conditionals a replayed statement's recorded walk kept A statement replayed from a loop's previous pass carried the recorded exit's whole holder list onto the current scope for each expression whose conditional holders the statement changed. When the current holders of that expression differed from the recorded entry's, the recorded ones described the previous pass's state: `$rowCount++` dropping the `$values` holders guarded by `$rowCount` replaced the final walk's `$values` holders with the pass's, such as `$cols` int<1, max> implying `$values` is array{0?: ...}. The while loop's merge with its not-ran path then kept that holder, and `$col = 0` in a later for loop narrowed `$values` to array{0?: ...}. Such an expression now keeps only its current holders that the recorded walk kept too. Amp-Thread-ID: https://ampcode.com/threads/T-01a1deb9-7acb-72e2-bab8-0404b371a021 --- src/Analyser/MutatingScope.php | 23 +++++++---- tests/PHPStan/Analyser/Bug15736Test.php | 33 +++++++++++++++ tests/PHPStan/Analyser/data/bug-15736.php | 50 +++++++++++++++++++++++ turbo-ext/src/MutatingScope.cpp | 50 +++++++++++++++++++---- 4 files changed, 140 insertions(+), 16 deletions(-) create mode 100644 tests/PHPStan/Analyser/Bug15736Test.php create mode 100644 tests/PHPStan/Analyser/data/bug-15736.php diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index f459f053a2c..bf19f368357 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -106,6 +106,7 @@ use function abs; use function array_diff_key; use function array_filter; +use function array_intersect_key; use function array_key_exists; use function array_keys; use function array_last; @@ -1515,17 +1516,25 @@ private static function getVariablesOfExpressionKey(string $key): ?array public function withRecordedStatementDelta(self $recordedEntry, self $recordedExit): self { $conditionalExpressions = $this->conditionalExpressions; - foreach ($recordedExit->conditionalExpressions as $key => $holders) { - if (isset($recordedEntry->conditionalExpressions[$key]) && $recordedEntry->conditionalExpressions[$key] === $holders) { + foreach (array_keys($recordedExit->conditionalExpressions + $recordedEntry->conditionalExpressions) as $key) { + $entryHolders = $recordedEntry->conditionalExpressions[$key] ?? []; + $exitHolders = $recordedExit->conditionalExpressions[$key] ?? []; + if ($entryHolders === $exitHolders) { continue; } - $conditionalExpressions[$key] = $holders; - } - foreach (array_keys($recordedEntry->conditionalExpressions) as $key) { - if (isset($recordedExit->conditionalExpressions[$key])) { + $currentHolders = $conditionalExpressions[$key] ?? []; + if ($currentHolders !== $entryHolders) { + // the statement changed the holders of an expression whose holders + // differ here (e.g. it assigned the guard of one): the recorded + // exit's holders describe the recorded state, so keep only this + // scope's holders that the recorded walk kept + $exitHolders = array_intersect_key($currentHolders, $entryHolders, $exitHolders); + } + if ($exitHolders === []) { + unset($conditionalExpressions[$key]); continue; } - unset($conditionalExpressions[$key]); + $conditionalExpressions[$key] = $exitHolders; } return $this->duplicateWith( diff --git a/tests/PHPStan/Analyser/Bug15736Test.php b/tests/PHPStan/Analyser/Bug15736Test.php new file mode 100644 index 00000000000..c2887ac2f6e --- /dev/null +++ b/tests/PHPStan/Analyser/Bug15736Test.php @@ -0,0 +1,33 @@ +assertFileAsserts($assertType, $file, ...$args); + } + +} diff --git a/tests/PHPStan/Analyser/data/bug-15736.php b/tests/PHPStan/Analyser/data/bug-15736.php new file mode 100644 index 00000000000..46b85849182 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-15736.php @@ -0,0 +1,50 @@ + 1] : false; +} + +function query(): object +{ + return new stdClass(); +} + +function doFoo(object $rows): void +{ + $values = []; + $header = 0; + $cols = 0; + $rowCount = 0; + $items = query(); + while ($row = fetchRow($rows)) { + $header++; + if ($header === 1) { + while ($item = fetchRow($items)) { + $cols++; + } + } + + $col = 0; + $items = query(); + while ($item = fetchRow($items)) { + $values[$col][$rowCount] = $item['value']; + $col++; + } + $rowCount++; + } + + assertType('array, non-empty-array, int>>', $values); + for ($col = 0; $col < $cols; $col++) { + assertType('array, non-empty-array, int>>', $values); + for ($r = 0; $r < $rowCount; $r++) { + echo $values[$col][$r]; + } + } +} diff --git a/turbo-ext/src/MutatingScope.cpp b/turbo-ext/src/MutatingScope.cpp index c819fa79eb3..3f1097828e0 100644 --- a/turbo-ext/src/MutatingScope.cpp +++ b/turbo-ext/src/MutatingScope.cpp @@ -2996,6 +2996,42 @@ class MutatingScope return true; } + /* withRecordedStatementDelta()'s step for one conditional expression + * key; a missing or null holder list (NULL here) is the twin's `?? []` */ + static void applyRecordedConditionalDelta(zv::Arr &conditionalExpressions, const zv::ArrayEntry &key, zval *entryHolders, zval *exitHolders) + { + zval empty; + ZVAL_EMPTY_ARRAY(&empty); + if (entryHolders == NULL || Z_TYPE_P(entryHolders) == IS_NULL) entryHolders = ∅ + if (exitHolders == NULL || Z_TYPE_P(exitHolders) == IS_NULL) exitHolders = ∅ + if (zend_is_identical(entryHolders, exitHolders)) return; + + zval *currentHolders = findByEntryKey(conditionalExpressions.table(), key); + if (currentHolders == NULL || Z_TYPE_P(currentHolders) == IS_NULL) currentHolders = ∅ + zval kept; + if (!zend_is_identical(currentHolders, entryHolders)) { + /* array_intersect_key($currentHolders, $entryHolders, $exitHolders) */ + zv::Arr intersected = zv::Arr::create(0); + for (auto holder : zv::TableRef(Z_ARRVAL_P(currentHolders))) { + if (findByEntryKey(Z_ARRVAL_P(entryHolders), holder) == NULL || findByEntryKey(Z_ARRVAL_P(exitHolders), holder) == NULL) continue; + zval copy; + ZVAL_COPY(©, holder.value().raw()); + updateByEntryKey(intersected.table(), holder, ©); + } + kept = intersected.take(); + } else { + ZVAL_COPY(&kept, exitHolders); + } + + conditionalExpressions.separate(); + if (zend_hash_num_elements(Z_ARRVAL(kept)) == 0) { + zval_ptr_dtor(&kept); + deleteByEntryKey(conditionalExpressions.table(), key); + return; + } + updateByEntryKey(conditionalExpressions.table(), key, &kept); + } + /* This scope after a statement whose recorded walk stands. */ zv::Val withRecordedStatementDelta(zend_object *recordedEntryObject, zend_object *recordedExitObject) { @@ -3009,18 +3045,14 @@ class MutatingScope zv::Arr conditionalExpressions = zv::Arr::copyOfTable(Z_ARRVAL_P(slot(PT_MS_PROP_CONDITIONAL_EXPRESSIONS).raw())); HashTable *entryConditionals = Z_ARRVAL_P(recordedEntry.slot(PT_MS_PROP_CONDITIONAL_EXPRESSIONS).raw()); HashTable *exitConditionals = Z_ARRVAL_P(recordedExit.slot(PT_MS_PROP_CONDITIONAL_EXPRESSIONS).raw()); + /* the twin's `$recordedExit + $recordedEntry` key walk: the exit's + * keys, then the keys only the entry has */ for (auto entry : zv::TableRef(exitConditionals)) { - zval *entryHolders = findByEntryKey(entryConditionals, entry); - if (entryHolders != NULL && Z_TYPE_P(entryHolders) != IS_NULL && zend_is_identical(entryHolders, entry.value().raw())) continue; - conditionalExpressions.separate(); - zval copy; - ZVAL_COPY(©, entry.value().raw()); - updateByEntryKey(conditionalExpressions.table(), entry, ©); + applyRecordedConditionalDelta(conditionalExpressions, entry, findByEntryKey(entryConditionals, entry), entry.value().raw()); } for (auto entry : zv::TableRef(entryConditionals)) { - if (issetByEntryKey(exitConditionals, entry)) continue; - conditionalExpressions.separate(); - deleteByEntryKey(conditionalExpressions.table(), entry); + if (findByEntryKey(exitConditionals, entry) != NULL) continue; + applyRecordedConditionalDelta(conditionalExpressions, entry, entry.value().raw(), NULL); } static const struct { uint32_t slot; const char *name; } holderTables[] = {