Skip to content

Commit e288e03

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Feed engine gatherers from NodeScopeResolver frames instead of wrapping node callbacks
GatheringNodeCallback existed so engine-feeding gatherers (return statements, execution ends, impure points, invalidate expressions) could ride the node-callback channel while only the rule-facing remainder got the storage-backed scope - callNodeCallback() unwrapped the chain per emission, and VirtualAssignNodeCallback had to rebuild chains so a filter could not starve a gatherer. With everything synchronous the split is unnecessary: gatherers live as frames on NodeScopeResolver, fed the raw walk scope directly by callNodeCallback() and by replayRecording() for replayed emissions (previously a replayed gatherer saw the storage-backed scope - the raw scope is state-identical and consistent with live emissions). The nine construction sites push a frame around their body walk and pass the rule-facing callback through untouched. Fresh walks extensions start mid-analysis (processNodes()/processStmtNodes()) suspend the frames, so a rule-started re-walk cannot feed the interrupted walk's gatherers - matching the old behavior where such walks carried no wrapper. The ShallowNodeCallback marker went with it: nothing has consulted it since the fiber era. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnwgpaeUXRkgSDyg95tfK8
1 parent 5d39326 commit e288e03

12 files changed

Lines changed: 191 additions & 143 deletions

‎src/Analyser/ExprHandler/FuncCallHandler.php‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
use PHPStan\Analyser\ExprHandler\Helper\EarlyTerminatingCallHelper;
2323
use PHPStan\Analyser\ExprHandler\Helper\OutputBufferHelper;
2424
use PHPStan\Analyser\ExprHandler\Helper\VoidToNullTypeTransformer;
25-
use PHPStan\Analyser\GatheringNodeCallback;
2625
use PHPStan\Analyser\ImpurePoint;
2726
use PHPStan\Analyser\InternalThrowPoint;
2827
use PHPStan\Analyser\MutatingScope;
@@ -230,7 +229,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
230229
$arrayWalkArrayArg = null;
231230
$arrayWalkOriginalArrayType = null;
232231
$arrayWalkOriginalArrayNativeType = null;
233-
$nodeCallbackForArgs = $nodeCallback;
232+
$argsGatherer = null;
234233
if (
235234
$functionReflection !== null
236235
&& $functionReflection->getName() === 'array_walk'
@@ -254,7 +253,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
254253
$arrayWalkOriginalArrayType = $scope->getType($arrayWalkArrayArg);
255254
$arrayWalkOriginalArrayNativeType = $scope->getNativeType($arrayWalkArrayArg);
256255

257-
$nodeCallbackForArgs = new GatheringNodeCallback(static function (Node $node, Scope $scope) use ($callbackArg, $firstParamName, &$arrayWalkValueTypes): void {
256+
$argsGatherer = static function (Node $node, Scope $scope) use ($callbackArg, $firstParamName, &$arrayWalkValueTypes): void {
258257
if (!($node instanceof ClosureReturnStatementsNode) || $node->getClosureExpr() !== $callbackArg) {
259258
return;
260259
}
@@ -286,12 +285,21 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
286285
TypeCombinator::union(...$types),
287286
TypeCombinator::union(...$nativeTypes),
288287
];
289-
}, $nodeCallback);
288+
};
290289
}
291290
}
292291

293292
$scopeBeforeArgs = $scope;
294-
$argsResult = $nodeScopeResolver->processArgs($stmt, $functionReflection, null, $variants, $namedArgumentsVariants, $normalizedExpr, $scope, $storage, $nodeCallbackForArgs, $context);
293+
if ($argsGatherer !== null) {
294+
$nodeScopeResolver->pushNodeGatherer($argsGatherer);
295+
}
296+
try {
297+
$argsResult = $nodeScopeResolver->processArgs($stmt, $functionReflection, null, $variants, $namedArgumentsVariants, $normalizedExpr, $scope, $storage, $nodeCallback, $context);
298+
} finally {
299+
if ($argsGatherer !== null) {
300+
$nodeScopeResolver->popNodeGatherer();
301+
}
302+
}
295303
$resolvedParametersAcceptor = $argsResult->getResolvedParametersAcceptor();
296304
$scope = $argsResult->getScope();
297305
$hasYield = $argsResult->hasYield();

‎src/Analyser/ExprHandler/NewHandler.php‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use PHPStan\Analyser\ExpressionResultFactory;
1515
use PHPStan\Analyser\ExpressionResultStorage;
1616
use PHPStan\Analyser\ExprHandler;
17-
use PHPStan\Analyser\GatheringNodeCallback;
1817
use PHPStan\Analyser\ImpurePoint;
1918
use PHPStan\Analyser\InternalThrowPoint;
2019
use PHPStan\Analyser\MutatingScope;
@@ -130,7 +129,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
130129

131130
if ($constructorReflection->getDeclaringClass()->getName() === $classReflection->getName()) {
132131
$constructorResult = null;
133-
$nodeScopeResolver->processStmtNode($expr->class, $scope, $storage, new GatheringNodeCallback(static function (Node $node, Scope $scope) use ($classReflection, &$constructorResult): void {
132+
$nodeScopeResolver->pushNodeGatherer(static function (Node $node, Scope $scope) use ($classReflection, &$constructorResult): void {
134133
if (!$node instanceof MethodReturnStatementsNode) {
135134
return;
136135
}
@@ -148,7 +147,13 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex
148147
return;
149148
}
150149
$constructorResult = $node;
151-
}, $nodeCallback), StatementContext::createTopLevel());
150+
});
151+
try {
152+
$nodeScopeResolver->processStmtNode($expr->class, $scope, $storage, $nodeCallback, StatementContext::createTopLevel());
153+
} finally {
154+
$nodeScopeResolver->popNodeGatherer();
155+
}
156+
152157
if ($constructorResult !== null) {
153158
$throwPoints = array_map(static fn (ThrowPoint $point): InternalThrowPoint => InternalThrowPoint::createFromPublic($point, $scope), $constructorResult->getStatementResult()->getThrowPoints());
154159
$impurePoints = $constructorResult->getImpurePoints();

‎src/Analyser/GatheringNodeCallback.php‎

Lines changed: 0 additions & 50 deletions
This file was deleted.

‎src/Analyser/NodeScopeResolver.php‎

Lines changed: 131 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@
102102
use function array_last;
103103
use function array_map;
104104
use function array_merge;
105+
use function array_pop;
105106
use function array_slice;
106107
use function array_values;
107108
use function count;
@@ -124,6 +125,20 @@ class NodeScopeResolver
124125

125126
private ?ExpressionResultStorageStack $expressionResultStorageStack = null;
126127

128+
/**
129+
* Engine-feeding gatherer frames (return statements, execution ends,
130+
* impure points, ...), innermost last. callNodeCallback() feeds every
131+
* frame the raw walk scope at the emission position - gatherers are
132+
* engine code and never ask about types, and their arrays are read as
133+
* soon as the enclosing body walk returns. Frames replace the former
134+
* GatheringNodeCallback wrapper chain: gatherers no longer ride the
135+
* node-callback channel, so callback filters (VirtualAssignNodeCallback)
136+
* cannot accidentally starve them.
137+
*
138+
* @var list<callable(Node, Scope): void>
139+
*/
140+
private array $nodeGatherers = [];
141+
127142
/**
128143
* @param ExtensionsCollection<FunctionParameterOutTypeExtension> $functionParameterOutTypeExtensions
129144
* @param ExtensionsCollection<MethodParameterOutTypeExtension> $methodParameterOutTypeExtensions
@@ -217,8 +232,6 @@ public function processNodes(
217232
$scope = $scope->toWalkScope();
218233

219234
$expressionResultStorage = new ExpressionResultStorage();
220-
$alreadyTerminated = false;
221-
$exitPoints = [];
222235

223236
$stmts = [];
224237
$stmtToNodeIndex = [];
@@ -231,6 +244,34 @@ public function processNodes(
231244
$stmts[] = $node;
232245
}
233246

247+
// a fresh walk an extension starts mid-analysis must not feed the
248+
// interrupted walk's gatherer frames (see processStmtNodes())
249+
$gatherers = $this->nodeGatherers;
250+
$this->nodeGatherers = [];
251+
try {
252+
$this->processNodesStatements($nodes, $stmts, $stmtToNodeIndex, $scope, $expressionResultStorage, $nodeCallback);
253+
} finally {
254+
$this->nodeGatherers = $gatherers;
255+
}
256+
}
257+
258+
/**
259+
* @param Node[] $nodes
260+
* @param Node\Stmt[] $stmts
261+
* @param array<int, int> $stmtToNodeIndex
262+
* @param callable(Node $node, Scope $scope): void $nodeCallback
263+
*/
264+
private function processNodesStatements(
265+
array $nodes,
266+
array $stmts,
267+
array $stmtToNodeIndex,
268+
MutatingScope $scope,
269+
ExpressionResultStorage $expressionResultStorage,
270+
callable $nodeCallback,
271+
): void
272+
{
273+
$alreadyTerminated = false;
274+
$exitPoints = [];
234275
$dummyParent = new Node\Stmt\Nop();
235276
foreach ($stmts as $si => $node) {
236277
if ($alreadyTerminated && !($node instanceof Node\Stmt\Function_ || $node instanceof Node\Stmt\ClassLike || $node instanceof Node\Stmt\Label)) {
@@ -446,14 +487,23 @@ public function processStmtNodes(
446487
// rule-facing ask paths
447488
$scope = $scope->toWalkScope();
448489
$storage = new ExpressionResultStorage();
449-
return $this->processStmtNodesInternal(
450-
$parentNode,
451-
$stmts,
452-
$scope,
453-
$storage,
454-
$nodeCallback,
455-
$context,
456-
)->toPublic();
490+
// a fresh walk an extension starts mid-analysis must not feed the
491+
// interrupted walk's gatherer frames - they describe the body walk
492+
// that was interrupted, not the nested one
493+
$gatherers = $this->nodeGatherers;
494+
$this->nodeGatherers = [];
495+
try {
496+
return $this->processStmtNodesInternal(
497+
$parentNode,
498+
$stmts,
499+
$scope,
500+
$storage,
501+
$nodeCallback,
502+
$context,
503+
)->toPublic();
504+
} finally {
505+
$this->nodeGatherers = $gatherers;
506+
}
457507
}
458508

459509
/**
@@ -988,12 +1038,41 @@ private function hasContextSensitiveConstruct(Node $node): bool
9881038
*
9891039
* @param callable(Node $node, Scope $scope): void $nodeCallback
9901040
*/
1041+
/**
1042+
* Opens an engine-feeding gatherer frame for the duration of a body walk.
1043+
* The caller closes it in a finally block via popNodeGatherer().
1044+
*
1045+
* @param callable(Node, Scope): void $gatherer
1046+
*/
1047+
public function pushNodeGatherer(callable $gatherer): void
1048+
{
1049+
$this->nodeGatherers[] = $gatherer;
1050+
}
1051+
1052+
public function popNodeGatherer(): void
1053+
{
1054+
array_pop($this->nodeGatherers);
1055+
}
1056+
1057+
/**
1058+
* @param callable(Node $node, Scope $scope): void $nodeCallback
1059+
*/
9911060
public function replayRecording(RecordingNodeCallback $recording, callable $nodeCallback, ExpressionResultStorage $storage): void
9921061
{
9931062
$stack = $this->getExpressionResultStorageStack();
9941063
$stack->push($storage);
9951064
try {
996-
$recording->replayThrough($nodeCallback);
1065+
foreach ($recording->getPairs() as [$node, $scope]) {
1066+
if (!$scope instanceof MutatingScope) {
1067+
throw new ShouldNotHappenException();
1068+
}
1069+
// gatherer frames observe replayed emissions exactly like live
1070+
// ones - with the raw walk scope
1071+
foreach ($this->nodeGatherers as $gatherer) {
1072+
$gatherer($node, $scope);
1073+
}
1074+
$nodeCallback($node, $scope->toNodeCallbackScope());
1075+
}
9971076
} finally {
9981077
$stack->pop();
9991078
}
@@ -1026,15 +1105,14 @@ public function callNodeCallback(
10261105
ExpressionResultStorage $storage,
10271106
): void
10281107
{
1029-
// Engine-feeding gatherers must observe the node at the emission
1108+
// Engine-feeding gatherer frames observe the node at the emission
10301109
// position - their arrays are read as soon as the enclosing body walk
10311110
// returns. Gatherers are engine code and never ask about types -
10321111
// handing them the raw scope skips a NodeCallbackScope construction per
10331112
// emission; the scopes they capture (return statements, impure points)
10341113
// answer later asks through the storage hub like any MutatingScope.
1035-
while ($nodeCallback instanceof GatheringNodeCallback) {
1036-
($nodeCallback->getGatherer())($node, $scope);
1037-
$nodeCallback = $nodeCallback->getInner();
1114+
foreach ($this->nodeGatherers as $gatherer) {
1115+
$gatherer($node, $scope);
10381116
}
10391117

10401118
if ($nodeCallback instanceof NoopNodeCallback) {
@@ -1155,7 +1233,7 @@ public function processClosureNode(
11551233
$gatheredYieldStatementsWithScope = [];
11561234
$closureImpurePoints = [];
11571235
$invalidateExpressions = [];
1158-
$closureStmtsCallback = new GatheringNodeCallback(static function (Node $node, Scope $scope) use (&$executionEnds, &$gatheredReturnStatements, &$gatheredReturnStatementsWithScope, &$gatheredYieldStatements, &$gatheredYieldStatementsWithScope, &$closureScope, &$closureImpurePoints, &$invalidateExpressions): void {
1236+
$closureStmtsGatherer = static function (Node $node, Scope $scope) use (&$executionEnds, &$gatheredReturnStatements, &$gatheredReturnStatementsWithScope, &$gatheredYieldStatements, &$gatheredYieldStatementsWithScope, &$closureScope, &$closureImpurePoints, &$invalidateExpressions): void {
11591237
if ($scope->getAnonymousFunctionReflection() !== $closureScope->getAnonymousFunctionReflection()) {
11601238
return;
11611239
}
@@ -1188,10 +1266,15 @@ public function processClosureNode(
11881266

11891267
$gatheredReturnStatements[] = new ReturnStatement($scope, $node);
11901268
$gatheredReturnStatementsWithScope[] = [$node, $scope];
1191-
}, $nodeCallback);
1269+
};
11921270

11931271
if (count($byRefUses) === 0) {
1194-
$statementResult = $this->processStmtNodesInternal($expr, $expr->stmts, $closureScope, $storage, $closureStmtsCallback, StatementContext::createTopLevel());
1272+
$this->pushNodeGatherer($closureStmtsGatherer);
1273+
try {
1274+
$statementResult = $this->processStmtNodesInternal($expr, $expr->stmts, $closureScope, $storage, $nodeCallback, StatementContext::createTopLevel());
1275+
} finally {
1276+
$this->popNodeGatherer();
1277+
}
11951278
$publicStatementResult = $statementResult->toPublic();
11961279
$closureReturnStatementsNodeScope = $this->refineClosureNodeScope($closureScope, $scope, $expr, $gatheredReturnStatementsWithScope, $gatheredYieldStatementsWithScope, $executionEnds, $statementResult->getThrowPoints(), array_merge($closureImpurePoints, $statementResult->getImpurePoints()), $invalidateExpressions);
11971280
$this->callNodeCallback($nodeCallback, new ClosureReturnStatementsNode(
@@ -1269,23 +1352,28 @@ public function processClosureNode(
12691352
}
12701353

12711354
$storage = $originalStorage;
1272-
if (
1273-
$replayBodyRecording !== null && $replayPassStorage !== null
1274-
&& $replayPassResult !== null && $replayEntryScope !== null
1275-
&& $closureScope->equals($replayEntryScope)
1276-
) {
1277-
// the final walk would repeat the recorded fixpoint pass exactly
1278-
// (same entry scope, deterministic walk) - adopt the pass's result
1279-
// and replay its emissions through the gathering callback instead.
1280-
// The pass's own entry scope takes over: the recorded pairs carry
1281-
// its anonymous-function reflection, which the gathering filter
1282-
// compares by identity (the state is equals-identical anyway).
1283-
$closureScope = $replayEntryScope;
1284-
$originalStorage->mergeResults($replayPassStorage);
1285-
$this->replayRecording($replayBodyRecording, $closureStmtsCallback, $originalStorage);
1286-
$statementResult = $replayPassResult;
1287-
} else {
1288-
$statementResult = $this->processStmtNodesInternal($expr, $expr->stmts, $closureScope, $storage, $closureStmtsCallback, StatementContext::createTopLevel());
1355+
$this->pushNodeGatherer($closureStmtsGatherer);
1356+
try {
1357+
if (
1358+
$replayBodyRecording !== null && $replayPassStorage !== null
1359+
&& $replayPassResult !== null && $replayEntryScope !== null
1360+
&& $closureScope->equals($replayEntryScope)
1361+
) {
1362+
// the final walk would repeat the recorded fixpoint pass exactly
1363+
// (same entry scope, deterministic walk) - adopt the pass's result
1364+
// and replay its emissions through the real callback instead.
1365+
// The pass's own entry scope takes over: the recorded pairs carry
1366+
// its anonymous-function reflection, which the gatherer's filter
1367+
// compares by identity (the state is equals-identical anyway).
1368+
$closureScope = $replayEntryScope;
1369+
$originalStorage->mergeResults($replayPassStorage);
1370+
$this->replayRecording($replayBodyRecording, $nodeCallback, $originalStorage);
1371+
$statementResult = $replayPassResult;
1372+
} else {
1373+
$statementResult = $this->processStmtNodesInternal($expr, $expr->stmts, $closureScope, $storage, $nodeCallback, StatementContext::createTopLevel());
1374+
}
1375+
} finally {
1376+
$this->popNodeGatherer();
12891377
}
12901378
$publicStatementResult = $statementResult->toPublic();
12911379
$closureReturnStatementsNodeScope = $this->refineClosureNodeScope($closureScope, $scope, $expr, $gatheredReturnStatementsWithScope, $gatheredYieldStatementsWithScope, $executionEnds, $statementResult->getThrowPoints(), array_merge($closureImpurePoints, $statementResult->getImpurePoints()), $invalidateExpressions);
@@ -1410,7 +1498,7 @@ public function processArrowFunctionNode(
14101498
// feeds ClosureTypeResolver::buildClosureTypeForArrowFunction().
14111499
$arrowFunctionImpurePoints = [];
14121500
$invalidateExpressions = [];
1413-
$arrowFunctionStmtsCallback = new GatheringNodeCallback(static function (Node $node, Scope $innerScope) use ($arrowFunctionScope, &$arrowFunctionImpurePoints, &$invalidateExpressions): void {
1501+
$arrowFunctionStmtsGatherer = static function (Node $node, Scope $innerScope) use ($arrowFunctionScope, &$arrowFunctionImpurePoints, &$invalidateExpressions): void {
14141502
if ($innerScope->getAnonymousFunctionReflection() !== $arrowFunctionScope->getAnonymousFunctionReflection()) {
14151503
return;
14161504
}
@@ -1432,9 +1520,14 @@ public function processArrowFunctionNode(
14321520
true,
14331521
);
14341522
$invalidateExpressions[] = new InvalidateExprNode($node->getPropertyFetch());
1435-
}, $nodeCallback);
1523+
};
14361524

1437-
$exprResult = $this->processExprNode($stmt, $expr->expr, $arrowFunctionScope, $storage, $arrowFunctionStmtsCallback, ExpressionContext::createTopLevel());
1525+
$this->pushNodeGatherer($arrowFunctionStmtsGatherer);
1526+
try {
1527+
$exprResult = $this->processExprNode($stmt, $expr->expr, $arrowFunctionScope, $storage, $nodeCallback, ExpressionContext::createTopLevel());
1528+
} finally {
1529+
$this->popNodeGatherer();
1530+
}
14381531

14391532
$closureTypeThrowPoints = array_map(static fn (InternalThrowPoint $throwPoint) => $throwPoint->toPublic(), $exprResult->getThrowPoints());
14401533
$closureTypeImpurePoints = array_merge($arrowFunctionImpurePoints, $exprResult->getImpurePoints());

0 commit comments

Comments
 (0)