From 22d56f8a5a365e301ed8c2180b29f433457ee318 Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Tue, 18 Aug 2026 14:31:51 +0000 Subject: [PATCH] Reuse a by-ref closure's settled convergence pass as its result walk in scope-only walks - `NodeScopeResolver::processClosureNode()` walked a `use (&$x)` closure's body twice on every call: once in the by-ref convergence loop (deep context, `NoopNodeCallback`) and once more as the result walk (top-level context, gathering callback). With nested by-ref closures that doubling compounds - `tests/bench/data/bug-11283.php` turned its 10 closure nodes into 513 `processClosureNode()` calls, 212 of them for the innermost closure alone (now 254 and 70). - When the walk is scope-only (the node callback is a `NoopNodeCallback`, possibly behind `GatheringNodeCallback` layers) it emits no rule or collector output, so the convergence pass that settles the by-ref uses is already the result walk from the same entry scope. Such passes now run with the gathering callback and the settled one is reused instead of walking the body again. - Added `NodeScopeResolver::isScopeOnlyWalk()`, which unwraps `GatheringNodeCallback` layers the same way `FiberNodeScopeResolver` does. - Gathering is restarted at the top of every gathering walk, so a non-converging closure that falls back to the result walk does not gather the same returns, yields, execution ends, impure points and invalidate expressions twice. - `tests/bench/data/bug-11283.php` goes from ~2.25s to ~0.99s locally; the analysis output is unchanged for it, for the whole bench corpus and for PHPStan's own source. - Probed the sibling constructs on the same "throwaway pass then result walk" axis: the foreach/while/do-while/for convergence loops and `resolveBackwardGotoScope()` have the same shape, but reuse there is unsafe behind an enclosing gatherer (a loop body, unlike a closure body, shares the enclosing anonymous function reflection, so it would be gathered once per pass) and, once made safe by matching the result walk's statement context, it is a wash on the bench corpus - left unchanged. Arrow functions and closures without by-ref uses already walk their body exactly once. --- src/Analyser/NodeScopeResolver.php | 66 +++++++++++++++-- .../Analyser/nsrt/nested-by-ref-closures.php | 71 +++++++++++++++++++ 2 files changed, 133 insertions(+), 4 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/nested-by-ref-closures.php diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 53ac5a06f71..3e986650e29 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -3037,6 +3037,22 @@ public function callNodeCallback( $nodeCallback($node, $scope); } + /** + * Whether a walk driven by this callback produces no rule or collector + * output - its only product is the resulting scope. Such a walk may be + * replaced by any other walk of the same body from the same entry scope. + * + * @param callable(Node $node, Scope $scope): void $nodeCallback + */ + private static function isScopeOnlyWalk(callable $nodeCallback): bool + { + while ($nodeCallback instanceof GatheringNodeCallback) { + $nodeCallback = $nodeCallback->getInner(); + } + + return $nodeCallback instanceof NoopNodeCallback; + } + /** * @param callable(Node $node, Scope $scope): void $nodeCallback */ @@ -3188,17 +3204,45 @@ public function processClosureNode( $originalStorage = $storage; + // A scope-only walk emits no rule output, so the convergence pass that + // settles the by-ref uses already IS this walk's result pass: let it + // gather and reuse it instead of walking the body once more below. + // Without this a by-ref closure walks its body twice, which multiplies + // with every level of closure nesting. + $reuseSettledPass = self::isScopeOnlyWalk($nodeCallback); + + // only the gathering walk that ends up being the result walk may + // contribute - every gathering walk starts the arrays over so a repeated + // one does not append to them twice + $restartGathering = static function () use (&$executionEnds, &$gatheredReturnStatements, &$gatheredReturnStatementsWithScope, &$gatheredYieldStatements, &$gatheredYieldStatementsWithScope, &$closureImpurePoints, &$invalidateExpressions): void { + $executionEnds = []; + $gatheredReturnStatements = []; + $gatheredReturnStatementsWithScope = []; + $gatheredYieldStatements = []; + $gatheredYieldStatementsWithScope = []; + $closureImpurePoints = []; + $invalidateExpressions = []; + }; + $count = 0; $closureResultScope = null; + $settledStatementResult = null; do { $prevScope = $closureScope; - $storage = $originalStorage->duplicate(); + if ($reuseSettledPass) { + $restartGathering(); + $storage = $originalStorage; + } else { + $storage = $originalStorage->duplicate(); + } // deep context, like the loop handlers' own convergence passes: inner // loops walk single-pass here and only the final walk below (top-level) // runs their full convergence - otherwise every closure-convergence - // pass would re-converge every inner loop from scratch - $intermediaryClosureScopeResult = $this->processStmtNodesInternalWithoutFlushingPendingFibers($expr, $expr->stmts, $closureScope, $storage, new NoopNodeCallback(), StatementContext::createDeep()); + // pass would re-converge every inner loop from scratch. A scope-only + // walk has no final walk to reach that convergence, matching how it + // approximates every other loop it enters deep. + $intermediaryClosureScopeResult = $this->processStmtNodesInternalWithoutFlushingPendingFibers($expr, $expr->stmts, $closureScope, $storage, $reuseSettledPass ? $closureStmtsCallback : new NoopNodeCallback(), StatementContext::createDeep()); $intermediaryClosureScope = $intermediaryClosureScopeResult->getScope(); foreach ($intermediaryClosureScopeResult->getExitPoints() as $exitPoint) { $intermediaryClosureScope = $intermediaryClosureScope->mergeWith($exitPoint->getScope()); @@ -3206,6 +3250,9 @@ public function processClosureNode( if ($expr->getAttribute(ImmediatelyInvokedClosureVisitor::ATTRIBUTE_NAME) === true) { $closureResultScope = $intermediaryClosureScope; + if ($reuseSettledPass) { + $settledStatementResult = $intermediaryClosureScopeResult; + } break; } @@ -3213,6 +3260,10 @@ public function processClosureNode( $closureScope = $closureScope->processClosureScope($intermediaryClosureScope, $prevScope, $byRefUses); if ($closureScope->equals($prevScope)) { + if ($reuseSettledPass) { + // the settled entry scope is the one this pass walked from + $settledStatementResult = $intermediaryClosureScopeResult; + } break; } if ($count >= self::GENERALIZE_AFTER_ITERATION) { @@ -3226,7 +3277,14 @@ public function processClosureNode( } $storage = $originalStorage; - $statementResult = $this->processStmtNodesInternalWithoutFlushingPendingFibers($expr, $expr->stmts, $closureScope, $storage, $closureStmtsCallback, StatementContext::createTopLevel()); + if ($settledStatementResult !== null) { + $statementResult = $settledStatementResult; + } else { + if ($reuseSettledPass) { + $restartGathering(); + } + $statementResult = $this->processStmtNodesInternalWithoutFlushingPendingFibers($expr, $expr->stmts, $closureScope, $storage, $closureStmtsCallback, StatementContext::createTopLevel()); + } $publicStatementResult = $statementResult->toPublic(); $closureReturnStatementsNodeScope = $this->refineClosureNodeScope($closureScope, $scope, $expr, $gatheredReturnStatementsWithScope, $gatheredYieldStatementsWithScope, $executionEnds, $statementResult->getThrowPoints(), array_merge($closureImpurePoints, $statementResult->getImpurePoints()), $invalidateExpressions); $this->callNodeCallback($nodeCallback, new ClosureReturnStatementsNode( diff --git a/tests/PHPStan/Analyser/nsrt/nested-by-ref-closures.php b/tests/PHPStan/Analyser/nsrt/nested-by-ref-closures.php new file mode 100644 index 00000000000..292a534e6a4 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/nested-by-ref-closures.php @@ -0,0 +1,71 @@ +run(function () use (&$counter, &$collected): void { + assertType('int<0, max>', $counter); + assertType("array<'first'|'second'|'third', true>", $collected); + $counter++; + $collected['first'] = true; + $this->run(function () use (&$counter, &$collected): void { + assertType('int<1, max>', $counter); + assertType("non-empty-array<'first'|'second'|'third', true>&hasOffsetValue('first', true)", $collected); + $counter++; + $collected['second'] = true; + $this->run(function () use (&$counter, &$collected): void { + assertType('int<2, max>', $counter); + assertType("non-empty-array<'first'|'second'|'third', true>&hasOffsetValue('first', true)&hasOffsetValue('second', true)", $collected); + $counter++; + $collected['third'] = true; + }); + assertType('int<2, max>', $counter); + assertType("non-empty-array<'first'|'second'|'third', true>&hasOffsetValue('first', true)&hasOffsetValue('second', true)", $collected); + }); + assertType('int<1, max>', $counter); + assertType("non-empty-array<'first'|'second'|'third', true>&hasOffsetValue('first', true)", $collected); + }); + assertType('int<0, max>', $counter); + assertType("array<'first'|'second'|'third', true>", $collected); + } + + /** @param list $items */ + public function nestedInLoop(array $items): void + { + $seen = []; + foreach ($items as $item) { + $this->run(function () use (&$seen, $item): void { + $seen[] = $item; + $this->run(function () use (&$seen): void { + $seen[] = 'inner'; + }); + }); + } + assertType('list', $seen); + } + + public function immediatelyInvoked(): void + { + $value = null; + (function () use (&$value): void { + $this->run(function () use (&$value): void { + $value = new Runner(); + }); + })(); + assertType('NestedByRefClosures\Runner|null', $value); + } + +}