Simulate loop analysis for goto-label pairs#5701
Conversation
Treat goto as a terminating statement with exit points, and implement loop-like stabilization for backward goto-label pairs at both the same statement-list level and across nested structures like try-catch.
| $backwardGotoLabels = []; | ||
| $labelIndices = []; | ||
| foreach ($stmts as $idx => $s) { | ||
| if (!($s instanceof Node\Stmt\Label)) { | ||
| continue; | ||
| } | ||
|
|
||
| $labelIndices[$s->name->toString()] = $idx; | ||
| } | ||
| $nodeFinder = new NodeFinder(); | ||
| if ($labelIndices !== []) { | ||
| foreach ($labelIndices as $labelName => $labelIdx) { | ||
| $stmtsAfterLabel = array_slice($stmts, $labelIdx + 1); | ||
| /** @var Goto_[] $gotos */ | ||
| $gotos = $nodeFinder->findInstanceOf($stmtsAfterLabel, Goto_::class); | ||
| foreach ($gotos as $goto) { | ||
| if ($goto->name->toString() === $labelName) { | ||
| $backwardGotoLabels[$labelName] = true; | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| $nestedBackwardGotoLabels = []; | ||
| foreach ($stmts as $idx => $s) { | ||
| if ($s instanceof Node\Stmt\Label) { | ||
| continue; | ||
| } | ||
| /** @var Node\Stmt\Label[] $nestedLabels */ | ||
| $nestedLabels = $nodeFinder->findInstanceOf([$s], Node\Stmt\Label::class); | ||
| if ($nestedLabels === []) { | ||
| continue; | ||
| } | ||
| /** @var Goto_[] $nestedGotos */ | ||
| $nestedGotos = $nodeFinder->findInstanceOf([$s], Goto_::class); | ||
| if ($nestedGotos === []) { | ||
| continue; | ||
| } | ||
| $nestedLabelNames = []; | ||
| foreach ($nestedLabels as $nestedLabel) { | ||
| $nestedLabelNames[$nestedLabel->name->toString()] = true; | ||
| } | ||
| foreach ($nestedGotos as $nestedGoto) { | ||
| $gotoName = $nestedGoto->name->toString(); | ||
| if (!isset($nestedLabelNames[$gotoName])) { | ||
| continue; | ||
| } | ||
|
|
||
| $nestedBackwardGotoLabels[$idx][$gotoName] = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Would be really nice if we were able to move this code to a richParserNodeVisitor and if it ran very efficiently when traversing the AST. I don't want to do it here because the same code might be analysed multiple times and this would waste a lot of resources. i'm talking about code between lines 413-464.
Would be nice if the attributes on AST Node would be set just once on a specific node, like Label or Goto_ so that it's not spammed on all nodes for example.
There was a problem hiding this comment.
Done. The review feedback has been addressed with a single commit that:
-
Created
src/Parser/GotoLabelVisitor.php— arichParserNodeVisitorthat runs once per file parse (not per analysis pass) and sets attributes on specific AST nodes:hasBackwardGotoonLabelnodes that have a backward goto targeting themnestedBackwardGotoLabelson compound statement nodes (likeTryCatch) that contain both a label and a goto to it
-
Updated
src/Analyser/NodeScopeResolver.php— removed the 53-line scanning block (the$backwardGotoLabels/$nestedBackwardGotoLabelscomputation withNodeFinder) and replaced the two consumption sites withgetAttribute()calls reading from the pre-computed visitor attributes.
The attributes are only set on the specific relevant nodes (Label and compound statements), not spammed on all nodes, as the reviewer requested.
The backward goto and nested backward goto detection code was running inside processStmtNodesInternalWithoutFlushingPendingFibers, which meant it executed on every analysis pass including stabilization loops. Move it to GotoLabelVisitor so it runs once per parse and stores results as attributes on Label and compound-statement nodes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Done. Applied |
2 participants
Summary
This PR treats
gotoas a terminating statement (likebreak/continue/return) and implements loop-like scope stabilization for backward goto-label pairs (where agotojumps back to an earlier label, creating a loop).Changes
gotoas a terminating statement: Agotostatement now produces an exit point carrying its scope, and marks execution as terminated. This fixes forward-goto cases where code after agotowas not recognized as unreachable, and where labels after terminating code were not recognized as reachable viagoto.Same-level backward goto stabilization: When a label at the current statement-list level has a
gototargeting it from later in the same list, the statements between the label and the goto are processed in a stabilization loop (usingNoopNodeCallbackandenterDeep(), with generalization afterGENERALIZE_AFTER_ITERATIONand a cap atLOOP_SCOPE_ITERATIONS), exactly matching the existing loop analysis pattern forwhile/for/foreach.Nested backward goto stabilization: When a compound statement (e.g.,
try-catch) contains both a label and agototargeting it across sub-blocks (label intrybody,gotoincatchbody), the containing statement is processed through the same stabilization loop before the real analysis pass.Label reachability: Labels now un-terminate execution when reached via a forward
gotoexit point. The unreachable-statement detection (getNextUnreachableStatements) stops at Label nodes to avoid false positives.Regression tests
bug-4674.php(NSRT) — forward goto type narrowingbug-7711.php(NSRT) — backward goto null-check retry loopbug-7734.php(NSRT) — forward goto resource narrowingbug-9904.php(NSRT) — forward goto skips foreachgoto-label-stabilization.php(NSRT) — comprehensive stabilization tests (forward narrowing, backward loops, retry patterns, multiple gotos to same label)bug-11731.php(UnreachableStatementRule) — unreachable code after gotobug-12167.php(NumberComparisonOperatorsConstantConditionRule) — try-catch retry pattern with nested goto-labelbug-12852.php(BooleanNotConstantConditionRule) — unconditional assignment before condition in goto loop (correct always-true report, matching while-loop semantics)bug-14638.php(MissingReturnRule) — Symfony cache goto retry pattern with missing return false positiveCloses phpstan/phpstan#14641
Closes phpstan/phpstan#920
Closes phpstan/phpstan#14638
Closes phpstan/phpstan#4674
Closes phpstan/phpstan#7711
Closes phpstan/phpstan#7734
Closes phpstan/phpstan#9904
Closes phpstan/phpstan#11731
Closes phpstan/phpstan#12167