Reuse a by-ref closure's settled convergence pass as its result walk in scope-only walks

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,31 +3204,66 @@ 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());
 			}
 
 			if ($expr->getAttribute(ImmediatelyInvokedClosureVisitor::ATTRIBUTE_NAME) === true) {
 				$closureResultScope = $intermediaryClosureScope;
+				if ($reuseSettledPass) {
+					$settledStatementResult = $intermediaryClosureScopeResult;
+				}
 				break;
 			}
 
 			$closureScope = $scope->enterAnonymousFunction($expr, $callableParameters, $nativeCallableParameters);
 			$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(

tests/PHPStan/Analyser/nsrt/nested-by-ref-closures.php

@@ -0,0 +1,71 @@
+<?php
+
+namespace NestedByRefClosures;
+
+use function PHPStan\Testing\assertType;
+
+class Runner
+{
+
+	/** @param callable(): void $callback */
+	public function run(callable $callback): void
+	{
+	}
+
+	public function nested(): void
+	{
+		$counter = 0;
+		$collected = [];
+		$this->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<string> $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<string>', $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);
+	}
+
+}