Skip to content

Commit 003d601

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Build a loop's back edge from its continue statements when the body end is unreachable
filterOutLoopExitPoints() marks a loop body containing break or continue as not always terminating, and the loop handlers then treated the end of the body as reachable. When the body ends in return or throw, that dead scope was merged into the next iteration and into the scope after the loop, so a variable assigned on the only path that loops back came out as possibly undefined. InternalStatementResult now keeps whether the end of the statements is reachable, and getLoopBackEdgeScope() returns the scope the next iteration starts from: the reachable end merged with the continue statements, or null when no iteration follows. While, do-while, for and foreach loops, including unrolled constant-array iterations, build their convergence passes and post-loop scopes from it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014LVEGd9G9w8j64EZQ7rysC
1 parent 26ba3b0 commit 003d601

9 files changed

Lines changed: 393 additions & 65 deletions

File tree

‎src/Analyser/InternalStatementResult.php‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
final class InternalStatementResult
1010
{
1111

12+
private bool $endReachable;
13+
1214
/**
1315
* @param InternalStatementExitPoint[] $exitPoints
1416
* @param InternalThrowPoint[] $throwPoints
@@ -24,8 +26,10 @@ public function __construct(
2426
private array $impurePoints,
2527
private array $endStatements = [],
2628
private ?VariableFlow $variableFlow = null,
29+
?bool $endReachable = null,
2730
)
2831
{
32+
$this->endReachable = $endReachable ?? !$isAlwaysTerminating;
2933
foreach ($exitPoints as $exitPoint) {
3034
$this->scope = $this->scope->addTemplateArgumentConstraints($exitPoint->getScope()->getTemplateArgumentConstraints());
3135
}
@@ -39,6 +43,30 @@ public function getVariableFlow(): ?VariableFlow
3943
return $this->variableFlow;
4044
}
4145

46+
/**
47+
* Whether execution can reach the end of the statements. Unlike isAlwaysTerminating(),
48+
* filterOutLoopExitPoints() does not reset it: a loop body left only through
49+
* break or continue still cannot reach its end.
50+
*/
51+
public function isEndReachable(): bool
52+
{
53+
return $this->endReachable;
54+
}
55+
56+
/**
57+
* The scope the next iteration of a loop starts from: the end of the body when
58+
* it is reachable, merged with the body's continue statements. Null when the body
59+
* never reaches the next iteration.
60+
*/
61+
public function getLoopBackEdgeScope(): ?MutatingScope
62+
{
63+
$backEdge = $this->endReachable ? $this->scope : null;
64+
foreach ($this->getExitPointsByType(Stmt\Continue_::class) as $continueExitPoint) {
65+
$backEdge = $backEdge === null ? $continueExitPoint->getScope() : $backEdge->mergeWith($continueExitPoint->getScope());
66+
}
67+
return $backEdge;
68+
}
69+
4270
public function toPublic(): StatementResult
4371
{
4472
return new StatementResult(
@@ -81,14 +109,14 @@ public function filterOutLoopExitPoints(): self
81109

82110
$num = $statement->num;
83111
if (!$num instanceof Int_) {
84-
return new self($this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints, $this->impurePoints, variableFlow: $this->variableFlow);
112+
return new self($this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints, $this->impurePoints, variableFlow: $this->variableFlow, endReachable: false);
85113
}
86114

87115
if ($num->value !== 1) {
88116
continue;
89117
}
90118

91-
return new self($this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints, $this->impurePoints, variableFlow: $this->variableFlow);
119+
return new self($this->scope, $this->hasYield, false, $this->exitPoints, $this->throwPoints, $this->impurePoints, variableFlow: $this->variableFlow, endReachable: false);
92120
}
93121

94122
return $this;

‎src/Analyser/StmtHandler/DoWhileHandler.php‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44

55
use PhpParser\Node\Stmt;
66
use PhpParser\Node\Stmt\Break_;
7-
use PhpParser\Node\Stmt\Continue_;
87
use PhpParser\Node\Stmt\Do_;
98
use PHPStan\Analyser\ExpressionContext;
109
use PHPStan\Analyser\ExpressionResultStorage;
@@ -72,12 +71,8 @@ public function processStmt(
7271
$scope->pushExpressionResultStorage($storage);
7372
try {
7473
$bodyScopeResult = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $bodyRecording, $context->enterDeep()->withoutTemplateArgumentResolution())->filterOutLoopExitPoints();
75-
$alwaysTerminating = $bodyScopeResult->isAlwaysTerminating();
76-
$bodyScope = $bodyScopeResult->getScope();
77-
foreach ($bodyScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
78-
$bodyScope = $bodyScope->mergeWith($continueExitPoint->getScope());
79-
}
80-
$finalScope = $alwaysTerminating ? $finalScope : $bodyScope->mergeWith($finalScope);
74+
$backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope();
75+
$finalScope = $backEdgeScope === null ? $finalScope : $backEdgeScope->mergeWith($finalScope);
8176
foreach ($bodyScopeResult->getExitPointsByType(Break_::class) as $breakExitPoint) {
8277
$finalScope = $breakExitPoint->getScope()->mergeWith($finalScope);
8378
}
@@ -88,10 +83,16 @@ public function processStmt(
8883
$replayPassStorage = $storage;
8984
$replayPassResult = $bodyScopeResult;
9085
}
91-
$bodyScope = $nodeScopeResolver->processExprNode($stmt, $stmt->cond, $bodyScope, $storage, new NoopNodeCallback(), ExpressionContext::createDeep(resolveTemplateArguments: false))->getTruthyScope();
86+
if ($backEdgeScope !== null) {
87+
$bodyScope = $nodeScopeResolver->processExprNode($stmt, $stmt->cond, $backEdgeScope, $storage, new NoopNodeCallback(), ExpressionContext::createDeep(resolveTemplateArguments: false))->getTruthyScope();
88+
}
9289
} finally {
9390
$scope->popExpressionResultStorage();
9491
}
92+
if ($backEdgeScope === null) {
93+
$bodyScope = $prevScope;
94+
break;
95+
}
9596
if ($bodyScope->equals($prevScope)) {
9697
break;
9798
}
@@ -120,10 +121,9 @@ public function processStmt(
120121
} else {
121122
$bodyScopeResult = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context)->filterOutLoopExitPoints();
122123
}
123-
$bodyScope = $bodyScopeResult->getScope();
124-
foreach ($bodyScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
125-
$bodyScope = $bodyScope->mergeWith($continueExitPoint->getScope());
126-
}
124+
$backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope();
125+
$backEdgeDead = $backEdgeScope === null;
126+
$bodyScope = $backEdgeScope ?? $bodyScopeResult->getScope();
127127

128128
// the condition is processed once on the post-body scope; its result
129129
// answers both the always-iterates check below and the falsey post-loop
@@ -138,16 +138,16 @@ public function processStmt(
138138
$alwaysIterates = $condBooleanType->isTrue()->yes();
139139
}
140140

141-
if ($alwaysIterates) {
141+
if ($alwaysIterates || $backEdgeDead) {
142142
$alwaysTerminating = count($bodyScopeResult->getExitPointsByType(Break_::class)) === 0;
143143
} else {
144144
$alwaysTerminating = $bodyScopeResult->isAlwaysTerminating();
145145
}
146-
$finalScope = $alwaysTerminating ? $finalScope : $bodyScope->mergeWith($finalScope);
146+
$finalScope = $alwaysTerminating || $backEdgeDead ? $finalScope : $bodyScope->mergeWith($finalScope);
147147
if ($finalScope === null) {
148148
$finalScope = $scope;
149149
}
150-
if (!$alwaysTerminating) {
150+
if (!$alwaysTerminating && !$backEdgeDead) {
151151
$hasYield = $condResult->hasYield();
152152
$throwPoints = $condResult->getThrowPoints();
153153
$impurePoints = $condResult->getImpurePoints();
@@ -162,7 +162,7 @@ public function processStmt(
162162

163163
$breakExitPoints = $bodyScopeResult->getExitPointsByType(Break_::class);
164164
if (count($breakExitPoints) > 0) {
165-
$breakScope = $alwaysIterates ? null : $finalScope;
165+
$breakScope = $alwaysIterates || $backEdgeDead ? null : $finalScope;
166166
foreach ($breakExitPoints as $breakExitPoint) {
167167
$breakScope = $breakScope === null ? $breakExitPoint->getScope() : $breakScope->mergeWith($breakExitPoint->getScope());
168168
}

‎src/Analyser/StmtHandler/ForHandler.php‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
use PhpParser\Node\Name;
1313
use PhpParser\Node\Stmt;
1414
use PhpParser\Node\Stmt\Break_;
15-
use PhpParser\Node\Stmt\Continue_;
1615
use PhpParser\Node\Stmt\For_;
1716
use PHPStan\Analyser\ExpressionContext;
1817
use PHPStan\Analyser\ExpressionResultStorage;
@@ -200,10 +199,12 @@ public function processStmt(
200199
$bodyScope = $nodeScopeResolver->processExprNode($stmt, $lastCondExpr, $bodyScope, $storage, new NoopNodeCallback(), ExpressionContext::createDeep(resolveTemplateArguments: false))->getTruthyScope();
201200
}
202201
$bodyScopeResult = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, new NoopNodeCallback(), $context->enterDeep()->withoutTemplateArgumentResolution())->filterOutLoopExitPoints();
203-
$bodyScope = $bodyScopeResult->getScope();
204-
foreach ($bodyScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
205-
$bodyScope = $bodyScope->mergeWith($continueExitPoint->getScope());
202+
$backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope();
203+
if ($backEdgeScope === null) {
204+
$bodyScope = $prevScope;
205+
break;
206206
}
207+
$bodyScope = $backEdgeScope;
207208

208209
foreach ($stmt->loop as $loopExpr) {
209210
$exprResult = $nodeScopeResolver->processExprNode($stmt, $loopExpr, $bodyScope, $storage, new NoopNodeCallback(), ExpressionContext::createTopLevel(resolveTemplateArguments: false));
@@ -244,10 +245,9 @@ public function processStmt(
244245
}
245246

246247
$finalScopeResult = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context)->filterOutLoopExitPoints();
247-
$finalScope = $finalScopeResult->getScope();
248-
foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
249-
$finalScope = $continueExitPoint->getScope()->mergeWith($finalScope);
250-
}
248+
$backEdgeScope = $finalScopeResult->getLoopBackEdgeScope();
249+
$backEdgeDead = $backEdgeScope === null;
250+
$finalScope = $backEdgeScope ?? $finalScopeResult->getScope();
251251

252252
$loopScope = $finalScope;
253253
foreach ($stmt->loop as $loopExpr) {
@@ -271,14 +271,14 @@ public function processStmt(
271271

272272
$breakExitPoints = $finalScopeResult->getExitPointsByType(Break_::class);
273273
if (count($breakExitPoints) > 0) {
274-
$breakScope = $alwaysIterates->yes() ? null : $finalScope;
274+
$breakScope = $alwaysIterates->yes() || $backEdgeDead ? null : $finalScope;
275275
foreach ($breakExitPoints as $breakExitPoint) {
276276
$breakScope = $breakScope === null ? $breakExitPoint->getScope() : $breakScope->mergeWith($breakExitPoint->getScope());
277277
}
278278
$finalScope = $breakScope;
279279
}
280280

281-
if ($isIterableAtLeastOnce->no() || $finalScopeResult->isAlwaysTerminating()) {
281+
if ($isIterableAtLeastOnce->no() || $finalScopeResult->isAlwaysTerminating() || ($backEdgeDead && count($breakExitPoints) === 0)) {
282282
if ($nodeScopeResolver->shouldPolluteScopeWithLoopInitialAssignments()) {
283283
$finalScope = $initScope;
284284
} else {

‎src/Analyser/StmtHandler/ForeachHandler.php‎

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -232,10 +232,12 @@ static function () use ($condResult, $emptyArrayType): Type {
232232
try {
233233
$bodyScope = $this->enterForeach($nodeScopeResolver, $bodyScope, $storage, $originalScope, $stmt, $foreachIterateeType, $foreachNativeIterateeType, $nodeCallback);
234234
$bodyScopeResult = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $bodyRecording, $context->enterDeep()->withoutTemplateArgumentResolution())->filterOutLoopExitPoints();
235-
$bodyScope = $bodyScopeResult->getScope();
236-
foreach ($bodyScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
237-
$bodyScope = $bodyScope->mergeWith($continueExitPoint->getScope());
235+
$backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope();
236+
if ($backEdgeScope === null) {
237+
$bodyScope = $prevScope;
238+
break;
238239
}
240+
$bodyScope = $backEdgeScope;
239241
} finally {
240242
$scope->popExpressionResultStorage();
241243
}
@@ -278,7 +280,7 @@ static function () use ($condResult, $emptyArrayType): Type {
278280
$finalPassContext = $unrolledTotalKeys !== null ? $context->enterUnrolledForeach($unrolledTotalKeys) : $context;
279281
$finalScopeResult = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $finalPassContext)->filterOutLoopExitPoints();
280282
}
281-
$finalScope = $finalScopeResult->getScope();
283+
$finalScope = $finalScopeResult->isEndReachable() ? $finalScopeResult->getScope() : null;
282284
$scopesWithIterableValueType = [];
283285

284286
$keyVarExpr = null;
@@ -299,7 +301,7 @@ static function () use ($condResult, $emptyArrayType): Type {
299301
$trackingExpr = $originalKeyVarExpr ?? $originalValueExpr;
300302

301303
$continueExitPointHasUnoriginalKeyType = false;
302-
if ($trackingExpr !== null) {
304+
if ($trackingExpr !== null && $finalScope !== null) {
303305
if ($finalScope->hasExpressionType($trackingExpr)->yes()) {
304306
$scopesWithIterableValueType[] = $finalScope;
305307
} else {
@@ -309,7 +311,7 @@ static function () use ($condResult, $emptyArrayType): Type {
309311

310312
foreach ($finalScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
311313
$continueScope = $continueExitPoint->getScope();
312-
$finalScope = $continueScope->mergeWith($finalScope);
314+
$finalScope = $finalScope === null ? $continueScope : $continueScope->mergeWith($finalScope);
313315
if ($trackingExpr === null || !$continueScope->hasExpressionType($trackingExpr)->yes()) {
314316
$continueExitPointHasUnoriginalKeyType = true;
315317
continue;
@@ -318,8 +320,9 @@ static function () use ($condResult, $emptyArrayType): Type {
318320
}
319321
$breakExitPoints = $finalScopeResult->getExitPointsByType(Break_::class);
320322
foreach ($breakExitPoints as $breakExitPoint) {
321-
$finalScope = $breakExitPoint->getScope()->mergeWith($finalScope);
323+
$finalScope = $finalScope === null ? $breakExitPoint->getScope() : $breakExitPoint->getScope()->mergeWith($finalScope);
322324
}
325+
$finalScope ??= $finalScopeResult->getScope();
323326

324327
if ($unrolledEndScope !== null) {
325328
$finalScope = $unrolledEndScope;
@@ -628,7 +631,7 @@ private function enterForeach(NodeScopeResolver $nodeScopeResolver, MutatingScop
628631
}
629632

630633
/**
631-
* @return array{bodyScope: MutatingScope, endScope: MutatingScope, totalKeys: int}|null
634+
* @return array{bodyScope: MutatingScope, endScope: MutatingScope|null, totalKeys: int}|null
632635
*/
633636
private function tryProcessUnrolledConstantArrayForeach(
634637
NodeScopeResolver $nodeScopeResolver,
@@ -755,14 +758,22 @@ private function tryProcessUnrolledConstantArrayForeach(
755758
$bodyContext,
756759
)->filterOutLoopExitPoints();
757760

758-
$iterEndScope = $bodyResult->getScope();
759-
foreach ($bodyResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
760-
$iterEndScope = $iterEndScope->mergeWith($continueExitPoint->getScope());
761-
}
761+
$iterEndScope = $bodyResult->getLoopBackEdgeScope();
762762
foreach ($bodyResult->getExitPointsByType(Break_::class) as $breakExitPoint) {
763763
$allBreakScopes[] = $breakExitPoint->getScope();
764764
}
765765

766+
if ($iterEndScope === null) {
767+
if ($isOptional) {
768+
// the key may be missing, the next iteration then starts from the previous one
769+
continue;
770+
}
771+
772+
// no later iteration runs, the loop is left only through its break statements
773+
$chainScope = null;
774+
break;
775+
}
776+
766777
if ($isOptional) {
767778
$chainScope = $iterEndScope->mergeWith($chainScope);
768779
} else {
@@ -774,11 +785,15 @@ private function tryProcessUnrolledConstantArrayForeach(
774785
for ($i = 1, $c = count($entryScopes); $i < $c; $i++) {
775786
$arrayBodyScope = $arrayBodyScope->mergeWith($entryScopes[$i]);
776787
}
777-
if (count($entryScopes) === 1) {
788+
if (count($entryScopes) === 1 && $chainScope !== null) {
778789
$arrayBodyScope = $arrayBodyScope->mergeWith($chainScope);
779790
}
780791

781792
$allBodyScopes[] = $arrayBodyScope;
793+
if ($chainScope === null) {
794+
continue;
795+
}
796+
782797
$allChainScopes[] = $chainScope;
783798
}
784799

@@ -791,21 +806,22 @@ private function tryProcessUnrolledConstantArrayForeach(
791806
$bodyScope = $bodyScope->mergeWith($allBodyScopes[$i]);
792807
}
793808

794-
$endScope = $allChainScopes[0];
795-
for ($i = 1, $c = count($allChainScopes); $i < $c; $i++) {
796-
$endScope = $endScope->mergeWith($allChainScopes[$i]);
809+
$chainEndScope = null;
810+
foreach ($allChainScopes as $chainScope) {
811+
$chainEndScope = $chainEndScope === null ? $chainScope : $chainEndScope->mergeWith($chainScope);
797812
}
798813

814+
$endScope = $chainEndScope;
799815
foreach ($allBreakScopes as $breakScope) {
800-
$endScope = $endScope->mergeWith($breakScope);
816+
$endScope = $endScope === null ? $breakScope : $endScope->mergeWith($breakScope);
801817
}
802818

803819
// Unsealed shapes describe zero-or-more additional entries beyond the
804820
// explicit keys. Run the scope-generalizing loop on top of the
805821
// unrolled explicit iterations so body-scope variables (e.g. counters)
806822
// account for the extra iterations while keeping the lower bound
807823
// established by the non-optional explicit keys.
808-
if ($hasUnsealed) {
824+
if ($hasUnsealed && $chainEndScope !== null && $endScope !== null) {
809825
$loopScope = $endScope;
810826
$count = 0;
811827
do {
@@ -814,13 +830,15 @@ private function tryProcessUnrolledConstantArrayForeach(
814830
$iterBodyScope = $loopScope->mergeWith($endScope);
815831
$iterBodyScope = $this->enterForeach($nodeScopeResolver, $iterBodyScope, $iterStorage, $originalScope, $stmt, $iterateeType, $nativeIterateeType, new NoopNodeCallback());
816832
$iterBodyScopeResult = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $iterBodyScope, $iterStorage, new NoopNodeCallback(), $context->enterDeep()->withoutTemplateArgumentResolution())->filterOutLoopExitPoints();
817-
$loopScope = $iterBodyScopeResult->getScope();
818-
foreach ($iterBodyScopeResult->getExitPointsByType(Continue_::class) as $continueExitPoint) {
819-
$loopScope = $loopScope->mergeWith($continueExitPoint->getScope());
820-
}
833+
$backEdgeScope = $iterBodyScopeResult->getLoopBackEdgeScope();
821834
foreach ($iterBodyScopeResult->getExitPointsByType(Break_::class) as $breakExitPoint) {
822835
$endScope = $endScope->mergeWith($breakExitPoint->getScope());
823836
}
837+
if ($backEdgeScope === null) {
838+
$loopScope = $prevLoopScope;
839+
break;
840+
}
841+
$loopScope = $backEdgeScope;
824842
$bodyScope = $bodyScope->mergeWith($loopScope);
825843
if ($loopScope->equals($prevLoopScope)) {
826844
break;

0 commit comments

Comments
 (0)