From 366cc05029b0605b3e85962639ac2a0394dc9f43 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 28 Jul 2026 20:22:31 +0200 Subject: [PATCH 1/3] Fill the foreach key-loop native types with the native flavour The loop array rewrite filled $keyLoopNativeTypes with the same getType() reads as the PHPDoc list, so a key-type-changing loop mapped the native array's key type to the PHPDoc key union. Latent in practice - the native mapping is guarded behind keyTypeChanged and a mappable native array - but the flavour was plainly wrong. Also drops an unused variable left behind by the for-loop condition conversion. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b --- src/Analyser/NodeScopeResolver.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 789b4844c6..0bef339beb 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -1629,7 +1629,7 @@ public function processStmtNode( } } $keyLoopTypes[] = $scopeWithIterableValueType->getType($keyVarExpr); - $keyLoopNativeTypes[] = $scopeWithIterableValueType->getType($keyVarExpr); + $keyLoopNativeTypes[] = $scopeWithIterableValueType->getNativeType($keyVarExpr); } else { // No key variable: the narrowed value var is the array element type directly. $dimFetchType = $scopeWithIterableValueType->getVariableType($stmt->valueVar->name); @@ -1944,7 +1944,6 @@ public function processStmtNode( foreach ($stmt->cond as $condExpr) { $condResult = $this->processExprNode($stmt, $condExpr, $bodyScope, $storage, new NoopNodeCallback(), ExpressionContext::createDeep()); $initScope = $condResult->getScope(); - $condResultScope = $condResult->getScope(); // only the last condition expression is relevant whether the loop continues // see https://www.php.net/manual/en/control-structures.for.php From 86312b777bd019bdc62a1e38ea2128c973bf67fb Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 28 Jul 2026 20:22:32 +0200 Subject: [PATCH 2/3] Create merge conditionals from the differing holders only createConditionalExpressions() scanned every holder map in full - twice per merge - although each of its loops only ever selects entries whose our/their/merged holders differ. mergeVariableHolders() knows exactly which keys those are and now reports them; the conditional creation iterates just that diff - typically a handful of keys instead of hundreds of holders. The ScopeOps turbo twin mirrors both signature changes. createConditionalExpressions was the largest self-time item in the single-pass branch's SPX profile; make phpstan user CPU measured -2..5% here. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b --- src/Analyser/MutatingScope.php | 5 +- src/Analyser/ScopeOps.php | 33 ++++++-- src/Turbo/TurboExtensionEnabler.php | 2 +- turbo-ext/src/ScopeOps.cpp | 112 ++++++++++++++++++++-------- turbo-ext/tests/smoke.php | 16 ++++ 5 files changed, 131 insertions(+), 37 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index fec12d1d88..e24dc605a8 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -3530,7 +3530,8 @@ public function mergeWith(?self $otherScope, bool $preserveVacuousConditionals = $ourExpressionTypes = $this->expressionTypes; $theirExpressionTypes = $otherScope->expressionTypes; - $mergedExpressionTypes = ScopeOps::mergeVariableHolders($ourExpressionTypes, $theirExpressionTypes); + $differingExpressionKeys = []; + $mergedExpressionTypes = ScopeOps::mergeVariableHolders($ourExpressionTypes, $theirExpressionTypes, $differingExpressionKeys); $conditionalExpressions = ScopeOps::intersectConditionalExpressions($this->conditionalExpressions, $otherScope->conditionalExpressions); if ($preserveVacuousConditionals) { $conditionalExpressions = $this->preserveVacuousConditionalExpressions( @@ -3549,12 +3550,14 @@ public function mergeWith(?self $otherScope, bool $preserveVacuousConditionals = $ourExpressionTypes, $theirExpressionTypes, $mergedExpressionTypes, + $differingExpressionKeys, ); $conditionalExpressions = ScopeOps::createConditionalExpressions( $conditionalExpressions, $theirExpressionTypes, $ourExpressionTypes, $mergedExpressionTypes, + $differingExpressionKeys, ); [$mergedExpressionTypes, $mergedNativeTypes] = ScopeOps::finishMerge( diff --git a/src/Analyser/ScopeOps.php b/src/Analyser/ScopeOps.php index b2015c6840..e5fb684ead 100644 --- a/src/Analyser/ScopeOps.php +++ b/src/Analyser/ScopeOps.php @@ -26,6 +26,7 @@ use function array_filter; use function array_key_exists; use function array_key_first; +use function array_keys; use function array_slice; use function count; use function get_class; @@ -195,9 +196,10 @@ public static function scopeWith( * * @param array $ourVariableTypeHolders * @param array $theirVariableTypeHolders + * @param array $differingKeys * @return array */ - public static function mergeVariableHolders(array $ourVariableTypeHolders, array $theirVariableTypeHolders): array + public static function mergeVariableHolders(array $ourVariableTypeHolders, array $theirVariableTypeHolders, array &$differingKeys = []): array { $intersectedVariableTypeHolders = []; $globalVariableCallback = static fn (Node $node) => $node instanceof Variable && is_string($node->name) && in_array($node->name, Scope::SUPERGLOBAL_VARIABLES, true); @@ -209,8 +211,10 @@ public static function mergeVariableHolders(array $ourVariableTypeHolders, array continue; } + $differingKeys[$exprString] = true; $intersectedVariableTypeHolders[$exprString] = $variableTypeHolder->and($theirVariableTypeHolders[$exprString]); } else { + $differingKeys[$exprString] = true; $expr = $variableTypeHolder->getExpr(); $containsSuperGlobal = $expr->getAttribute(self::CONTAINS_SUPER_GLOBAL_ATTRIBUTE_NAME); @@ -231,6 +235,7 @@ public static function mergeVariableHolders(array $ourVariableTypeHolders, array continue; } + $differingKeys[$exprString] = true; $expr = $variableTypeHolder->getExpr(); $containsSuperGlobal = $expr->getAttribute(self::CONTAINS_SUPER_GLOBAL_ATTRIBUTE_NAME); @@ -358,6 +363,7 @@ public static function intersectConditionalExpressions(array $ourConditionalExpr * @param array $ourExpressionTypes * @param array $theirExpressionTypes * @param array $mergedExpressionTypes + * @param array $differingKeys * @return array */ public static function createConditionalExpressions( @@ -365,6 +371,7 @@ public static function createConditionalExpressions( array $ourExpressionTypes, array $theirExpressionTypes, array $mergedExpressionTypes, + array $differingKeys, ): array { $newVariableTypes = $ourExpressionTypes; @@ -375,7 +382,11 @@ public static function createConditionalExpressions( // branch — but it remains a valid conditional *target*, so only exclude // it from guard selection instead of dropping it entirely. $guardsToExclude = []; - foreach ($theirExpressionTypes as $exprString => $holder) { + foreach (array_keys($differingKeys) as $exprString) { + if (!array_key_exists($exprString, $theirExpressionTypes)) { + continue; + } + $holder = $theirExpressionTypes[$exprString]; if (!array_key_exists($exprString, $mergedExpressionTypes)) { continue; } @@ -396,7 +407,11 @@ public static function createConditionalExpressions( } $typeGuards = []; - foreach ($newVariableTypes as $exprString => $holder) { + foreach (array_keys($differingKeys) as $exprString) { + if (!array_key_exists($exprString, $newVariableTypes)) { + continue; + } + $holder = $newVariableTypes[$exprString]; if ($holder->getExpr() instanceof VirtualNode) { continue; } @@ -436,7 +451,11 @@ public static function createConditionalExpressions( $guardIsSuperTypeOfTheirExprCache = []; $theirExprIsSuperTypeOfGuardCache = []; - foreach ($newVariableTypes as $exprString => $holder) { + foreach (array_keys($differingKeys) as $exprString) { + if (!array_key_exists($exprString, $newVariableTypes)) { + continue; + } + $holder = $newVariableTypes[$exprString]; if ($holder->getExpr() instanceof VirtualNode) { continue; } @@ -497,7 +516,11 @@ public static function createConditionalExpressions( } } - foreach ($mergedExpressionTypes as $exprString => $mergedExprTypeHolder) { + foreach (array_keys($differingKeys) as $exprString) { + if (!array_key_exists($exprString, $mergedExpressionTypes)) { + continue; + } + $mergedExprTypeHolder = $mergedExpressionTypes[$exprString]; if (array_key_exists($exprString, $ourExpressionTypes)) { continue; } diff --git a/src/Turbo/TurboExtensionEnabler.php b/src/Turbo/TurboExtensionEnabler.php index 691c31e90c..a41ec14395 100644 --- a/src/Turbo/TurboExtensionEnabler.php +++ b/src/Turbo/TurboExtensionEnabler.php @@ -20,7 +20,7 @@ final class TurboExtensionEnabler * version is the short SHA of the last commit touching turbo-ext/src/, * enforced by the phar.yml turbo-version job. */ - public const EXPECTED_EXTENSION_VERSION = 'e9ce4dc'; + public const EXPECTED_EXTENSION_VERSION = 'b7ed7b5'; private static bool $typeCombinatorCacheEnabled = false; diff --git a/turbo-ext/src/ScopeOps.cpp b/turbo-ext/src/ScopeOps.cpp index db9d9d7265..307f228b36 100644 --- a/turbo-ext/src/ScopeOps.cpp +++ b/turbo-ext/src/ScopeOps.cpp @@ -311,11 +311,15 @@ class ScopeOps return zv::Val::adopt(out); } - /* Mirrors ScopeOps::mergeVariableHolders(). */ - static zv::Val mergeVariableHolders(zv::TableRef ours, zv::TableRef theirs) + /* + * Mirrors ScopeOps::mergeVariableHolders(). differing (nullable) receives + * a true marker for every key that is not one shared holder on both sides + * — the twin's &$differingKeys out-parameter. + */ + static zv::Val mergeVariableHolders(zv::TableRef ours, zv::TableRef theirs, HashTable *differing) { zv::Arr merged = zv::Arr::create(ours.size()); - if (UNEXPECTED(!mergeVariableHoldersInto(merged, ours, theirs))) { + if (UNEXPECTED(!mergeVariableHoldersInto(merged, ours, theirs, differing))) { return zv::Val(); } return zv::Val(std::move(merged)); @@ -394,7 +398,7 @@ class ScopeOps /* mergedNative += filter(mergeVariableHolders(oursRemaining, theirsRemaining)) */ { - zv::Val remainingMerged = mergeVariableHolders(zv::TableRef(oursNativeRemaining.table()), zv::TableRef(theirsNativeRemaining.table())); + zv::Val remainingMerged = mergeVariableHolders(zv::TableRef(oursNativeRemaining.table()), zv::TableRef(theirsNativeRemaining.table()), NULL); if (UNEXPECTED(remainingMerged.isUndef())) { return zv::Val(); } @@ -461,7 +465,7 @@ class ScopeOps * per-guard caches; the input array is only duplicated once the first * conditional is actually appended. */ - static zv::Val createConditionalExpressions(zv::TableRef conditional, zv::TableRef ours, zv::TableRef theirs, zv::TableRef merged) + static zv::Val createConditionalExpressions(zv::TableRef conditional, zv::TableRef ours, zv::TableRef theirs, zv::TableRef merged, zv::TableRef differingKeys) { zend_class_entry *virtualNodeCe = pt_class(PT_CLASS_VIRTUAL_NODE); if (UNEXPECTED(virtualNodeCe == NULL)) { @@ -472,16 +476,22 @@ class ScopeOps zv::ScratchTable typeGuards(8); /* guardsToExclude: subtype-absorbed their-branch variables are poor - * guards but stay valid conditional targets */ - for (auto entry : theirs) { - zend_string *key = entry.stringKeyOrNull(); - zend_ulong idx = entry.indexKey(); + * guards but stay valid conditional targets. Only the merge's differing + * keys can qualify — iterate those (in their insertion order, like the + * twin) instead of the whole holder maps. */ + for (auto diffEntry : differingKeys) { + zend_string *key = diffEntry.stringKeyOrNull(); + zend_ulong idx = diffEntry.indexKey(); + zval *theirSlot = pt_ht_find(theirs.table(), key, idx); + if (theirSlot == NULL) { + continue; + } zval *mergedSlot = pt_ht_find(merged.table(), key, idx); if (mergedSlot == NULL) { continue; } - zv::Ref holder = entry.value().deref(); + zv::Ref holder = zv::Ref(theirSlot).deref(); if (UNEXPECTED(!pt_check_holder(holder.raw()))) { return zv::Val(); } @@ -522,11 +532,15 @@ class ScopeOps } /* typeGuards */ - for (auto entry : ours) { - zend_string *key = entry.stringKeyOrNull(); - zend_ulong idx = entry.indexKey(); - zv::Ref holder = entry.value().deref(); + for (auto diffEntry : differingKeys) { + zend_string *key = diffEntry.stringKeyOrNull(); + zend_ulong idx = diffEntry.indexKey(); + zval *ourSlot = pt_ht_find(ours.table(), key, idx); + if (ourSlot == NULL) { + continue; + } + zv::Ref holder = zv::Ref(ourSlot).deref(); if (UNEXPECTED(!pt_check_holder(holder.raw()))) { return zv::Val(); } @@ -585,10 +599,15 @@ class ScopeOps zv::Arr result; /* stays UNDEF until the first append duplicates the input */ /* main loop: pair non-merged expressions with guards */ - for (auto entry : ours) { - zend_string *key = entry.stringKeyOrNull(); - zend_ulong idx = entry.indexKey(); - zv::Ref holder = entry.value().deref(); + for (auto diffEntry : differingKeys) { + zend_string *key = diffEntry.stringKeyOrNull(); + zend_ulong idx = diffEntry.indexKey(); + + zval *ourSlot = pt_ht_find(ours.table(), key, idx); + if (ourSlot == NULL) { + continue; + } + zv::Ref holder = zv::Ref(ourSlot).deref(); if (instanceof_function(holderExpr(holder)->ce, virtualNodeCe)) { continue; @@ -711,14 +730,18 @@ class ScopeOps } /* their-only expressions: record certainty-No conditionals per guard */ - for (auto entry : merged) { - zend_string *key = entry.stringKeyOrNull(); - zend_ulong idx = entry.indexKey(); + for (auto diffEntry : differingKeys) { + zend_string *key = diffEntry.stringKeyOrNull(); + zend_ulong idx = diffEntry.indexKey(); + zval *mergedSlot = pt_ht_find(merged.table(), key, idx); + if (mergedSlot == NULL) { + continue; + } if (pt_ht_exists(ours.table(), key, idx)) { continue; } - zv::Ref mergedHolder = entry.value().deref(); + zv::Ref mergedHolder = zv::Ref(mergedSlot).deref(); if (UNEXPECTED(!pt_check_holder(mergedHolder.raw()))) { return zv::Val(); } @@ -1381,8 +1404,19 @@ class ScopeOps return zv::Val::adopt(created); } + /* $differing[$key] = true (marker insert, overwrites) */ + static void markDiffering(HashTable *differing, zend_string *skey, zend_ulong idx) + { + if (differing == NULL) { + return; + } + zval trueZv; + ZVAL_TRUE(&trueZv); + pt_ht_update(differing, skey, idx, &trueZv); + } + /* The two loops of mergeVariableHolders(), filling a caller-owned table. */ - static bool mergeVariableHoldersInto(zv::Arr &merged, zv::TableRef ours, zv::TableRef theirs) + static bool mergeVariableHoldersInto(zv::Arr &merged, zv::TableRef ours, zv::TableRef theirs, HashTable *differing) { for (auto entry : ours) { zend_string *key = entry.stringKeyOrNull(); @@ -1402,6 +1436,7 @@ class ScopeOps if (holder.asObject() == theirHolder.asObject()) { tableAddNewCopy(merged.table(), key, idx, holder); } else { + markDiffering(differing, key, idx); zval andHolder; if (UNEXPECTED(!pt_holder_and(holder.raw(), theirHolder.raw(), &andHolder))) { return false; @@ -1409,6 +1444,7 @@ class ScopeOps tableAddNew(merged.table(), key, idx, zv::Val::adopt(andHolder)); } } else { + markDiffering(differing, key, idx); bool containsSuperGlobal = pt_expr_contains_superglobal(holderExpr(holder)); if (UNEXPECTED(EG(exception))) { return false; @@ -1427,6 +1463,7 @@ class ScopeOps if (pt_ht_exists(merged.table(), key, idx)) { continue; } + markDiffering(differing, key, idx); zv::Ref holder = entry.value().deref(); if (UNEXPECTED(!pt_check_holder(holder.raw()))) { return false; @@ -2060,13 +2097,27 @@ void pt_register_scope_ops() { reg::Class cls("PHPStanTurbo\\ScopeOps"); - cls.method("mergeVariableHolders", reg::PublicStatic, 2, { reg::arrayArg("ourVariableTypeHolders"), reg::arrayArg("theirVariableTypeHolders") }, [](INTERNAL_FUNCTION_PARAMETERS) { + cls.method("mergeVariableHolders", reg::PublicStatic, 2, { reg::arrayArg("ourVariableTypeHolders"), reg::arrayArg("theirVariableTypeHolders"), reg::any("differingKeys", true) }, [](INTERNAL_FUNCTION_PARAMETERS) { HashTable *ours, *theirs; - ZEND_PARSE_PARAMETERS_START(2, 2) + zval *differing_zv = NULL; + ZEND_PARSE_PARAMETERS_START(2, 3) Z_PARAM_ARRAY_HT(ours) Z_PARAM_ARRAY_HT(theirs) + Z_PARAM_OPTIONAL + Z_PARAM_ZVAL(differing_zv) ZEND_PARSE_PARAMETERS_END(); - zv::Val result = ScopeOps::mergeVariableHolders(zv::TableRef(ours), zv::TableRef(theirs)); + HashTable *differing = NULL; + if (differing_zv != NULL && Z_ISREF_P(differing_zv)) { + /* the twin declares `array &$differingKeys = []`; vivify like PHP + * would and write through the reference */ + zval *inner = Z_REFVAL_P(differing_zv); + if (Z_TYPE_P(inner) != IS_ARRAY) { + convert_to_array(inner); + } + SEPARATE_ARRAY(inner); + differing = Z_ARRVAL_P(inner); + } + zv::Val result = ScopeOps::mergeVariableHolders(zv::TableRef(ours), zv::TableRef(theirs), differing); if (UNEXPECTED(result.isUndef())) { RETURN_THROWS(); } @@ -2184,15 +2235,16 @@ void pt_register_scope_ops() result.intoReturnValue(return_value); }); - cls.method("createConditionalExpressions", reg::PublicStatic, 4, { reg::arrayArg("conditionalExpressions"), reg::arrayArg("ourExpressionTypes"), reg::arrayArg("theirExpressionTypes"), reg::arrayArg("mergedExpressionTypes") }, [](INTERNAL_FUNCTION_PARAMETERS) { - HashTable *conditional, *ours, *theirs, *merged; - ZEND_PARSE_PARAMETERS_START(4, 4) + cls.method("createConditionalExpressions", reg::PublicStatic, 5, { reg::arrayArg("conditionalExpressions"), reg::arrayArg("ourExpressionTypes"), reg::arrayArg("theirExpressionTypes"), reg::arrayArg("mergedExpressionTypes"), reg::arrayArg("differingKeys") }, [](INTERNAL_FUNCTION_PARAMETERS) { + HashTable *conditional, *ours, *theirs, *merged, *differing_keys; + ZEND_PARSE_PARAMETERS_START(5, 5) Z_PARAM_ARRAY_HT(conditional) Z_PARAM_ARRAY_HT(ours) Z_PARAM_ARRAY_HT(theirs) Z_PARAM_ARRAY_HT(merged) + Z_PARAM_ARRAY_HT(differing_keys) ZEND_PARSE_PARAMETERS_END(); - zv::Val result = ScopeOps::createConditionalExpressions(zv::TableRef(conditional), zv::TableRef(ours), zv::TableRef(theirs), zv::TableRef(merged)); + zv::Val result = ScopeOps::createConditionalExpressions(zv::TableRef(conditional), zv::TableRef(ours), zv::TableRef(theirs), zv::TableRef(merged), zv::TableRef(differing_keys)); if (UNEXPECTED(result.isUndef())) { RETURN_THROWS(); } diff --git a/turbo-ext/tests/smoke.php b/turbo-ext/tests/smoke.php index e46759fb23..e668d30344 100644 --- a/turbo-ext/tests/smoke.php +++ b/turbo-ext/tests/smoke.php @@ -337,6 +337,22 @@ function check(bool $cond, string $msg): void check($storage->pendingFibers === [], "ERS $label: fiber array entries can be unset"); } +// ---- ScopeOps::mergeVariableHolders differingKeys ---- +$sharedP = $pH($expr1, $int, $pYes); +$sharedN = $nH($expr1, $int, $nYes); +$mergePOurs = ['$shared' => $sharedP, '$a' => $pH($expr1, $int, $pYes), '$b' => $pH($expr2, $string, $pYes)]; +$mergePTheirs = ['$shared' => $sharedP, '$b' => $pH($expr2, $string, $pMaybe), '$c' => $pH($expr2, $int, $pYes)]; +$mergeNOurs = ['$shared' => $sharedN, '$a' => $nH($expr1, $int, $nYes), '$b' => $nH($expr2, $string, $nYes)]; +$mergeNTheirs = ['$shared' => $sharedN, '$b' => $nH($expr2, $string, $nMaybe), '$c' => $nH($expr2, $int, $nYes)]; +$pDiffering = []; +$pMerged = \PHPStan\Analyser\ScopeOps::mergeVariableHolders($mergePOurs, $mergePTheirs, $pDiffering); +$nDiffering = []; +$nMerged = \PHPStanTurbo\ScopeOps::mergeVariableHolders($mergeNOurs, $mergeNTheirs, $nDiffering); +check($pDiffering === $nDiffering, 'ScopeOps mergeVariableHolders differingKeys parity: ' . json_encode($pDiffering) . ' vs ' . json_encode($nDiffering)); +check(array_keys($pMerged) === array_keys($nMerged), 'ScopeOps mergeVariableHolders merged keys parity'); +check(array_keys(\PHPStanTurbo\ScopeOps::mergeVariableHolders($mergeNOurs, $mergeNTheirs)) === array_keys($nMerged), 'ScopeOps mergeVariableHolders without differingKeys'); + + // ---- NodeScanner ---- $covered[\PHPStan\Node\NodeScanner::class] = true; $smokeParserFactory = new \PhpParser\ParserFactory(); From 3f94c3991e720922c463dadd091cdb9c8e6b2c4e Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 28 Jul 2026 20:22:32 +0200 Subject: [PATCH 3/3] Bump expected turbo version --- src/Turbo/TurboExtensionEnabler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Turbo/TurboExtensionEnabler.php b/src/Turbo/TurboExtensionEnabler.php index a41ec14395..adeb565965 100644 --- a/src/Turbo/TurboExtensionEnabler.php +++ b/src/Turbo/TurboExtensionEnabler.php @@ -20,7 +20,7 @@ final class TurboExtensionEnabler * version is the short SHA of the last commit touching turbo-ext/src/, * enforced by the phar.yml turbo-version job. */ - public const EXPECTED_EXTENSION_VERSION = 'b7ed7b5'; + public const EXPECTED_EXTENSION_VERSION = '86312b7'; private static bool $typeCombinatorCacheEnabled = false;