diff --git a/src/Analyser/StmtHandler/DoWhileHandler.php b/src/Analyser/StmtHandler/DoWhileHandler.php index f05e8967912..b1933d6998b 100644 --- a/src/Analyser/StmtHandler/DoWhileHandler.php +++ b/src/Analyser/StmtHandler/DoWhileHandler.php @@ -95,6 +95,11 @@ public function processStmt( $previousPass = $passRecord; $previousPassStorage = $storage; $backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope(); + if ($backEdgeScope === null && !$bodyIsReplayable) { + // a nested loop walks only its first iteration in a pass - its unconverged + // types can make the back edge look dead, so a walk converging it decides + $backEdgeScope = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage->duplicate(), new NoopNodeCallback(), $context->withoutTemplateArgumentResolution())->filterOutLoopExitPoints()->getLoopBackEdgeScope(); + } $finalScope = $backEdgeScope === null ? $finalScope : $backEdgeScope->mergeWith($finalScope); foreach ($bodyScopeResult->getExitPointsByType(Break_::class) as $breakExitPoint) { $finalScope = $breakExitPoint->getScope()->mergeWith($finalScope); diff --git a/src/Analyser/StmtHandler/ForHandler.php b/src/Analyser/StmtHandler/ForHandler.php index 7a95e2c5710..3879f51adb3 100644 --- a/src/Analyser/StmtHandler/ForHandler.php +++ b/src/Analyser/StmtHandler/ForHandler.php @@ -202,6 +202,7 @@ public function processStmt( // entry state differs from the previous pass's (StatementsHandler::processLoopPassStmtNodes()) $previousPass = null; $previousPassStorage = null; + $bodyIsReplayable = $nodeScopeResolver->isReplayableConvergenceBody($stmt, $stmt->stmts); do { $prevScope = $bodyScope; $storage = $originalStorage->duplicate(); @@ -231,6 +232,11 @@ public function processStmt( $previousPassStorage = $storage; $passFlows[] = $bodyScopeResult->getVariableFlow(); $backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope(); + if ($backEdgeScope === null && !$bodyIsReplayable) { + // a nested loop walks only its first iteration in a pass - its unconverged + // types can make the back edge look dead, so a walk converging it decides + $backEdgeScope = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage->duplicate(), new NoopNodeCallback(), $context->withoutTemplateArgumentResolution())->filterOutLoopExitPoints()->getLoopBackEdgeScope(); + } if ($backEdgeScope === null) { $bodyScope = $prevScope; break; diff --git a/src/Analyser/StmtHandler/ForeachHandler.php b/src/Analyser/StmtHandler/ForeachHandler.php index 2ba5323bf7b..260541c2afb 100644 --- a/src/Analyser/StmtHandler/ForeachHandler.php +++ b/src/Analyser/StmtHandler/ForeachHandler.php @@ -254,6 +254,11 @@ static function () use ($condResult, $emptyArrayType): Type { $previousPass = $passRecord; $previousPassStorage = $storage; $backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope(); + if ($backEdgeScope === null && !$bodyIsReplayable) { + // a nested loop walks only its first iteration in a pass - its unconverged + // types can make the back edge look dead, so a walk converging it decides + $backEdgeScope = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage->duplicate(), new NoopNodeCallback(), $context->withoutTemplateArgumentResolution())->filterOutLoopExitPoints()->getLoopBackEdgeScope(); + } if ($backEdgeScope === null) { $bodyScope = $prevScope; break; @@ -868,6 +873,11 @@ private function tryProcessUnrolledConstantArrayForeach( $iterBodyScope = $this->enterForeach($nodeScopeResolver, $iterBodyScope, $iterStorage, $originalScope, $stmt, $iterateeType, $nativeIterateeType, new NoopNodeCallback()); $iterBodyScopeResult = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $iterBodyScope, $iterStorage, new NoopNodeCallback(), $context->enterDeep()->withoutTemplateArgumentResolution())->filterOutLoopExitPoints(); $backEdgeScope = $iterBodyScopeResult->getLoopBackEdgeScope(); + if ($backEdgeScope === null && !$nodeScopeResolver->isReplayableConvergenceBody($stmt, $stmt->stmts)) { + // a nested loop walks only its first iteration in a pass - its unconverged + // types can make the back edge look dead, so a walk converging it decides + $backEdgeScope = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $iterBodyScope, $iterStorage->duplicate(), new NoopNodeCallback(), $context->withoutTemplateArgumentResolution())->filterOutLoopExitPoints()->getLoopBackEdgeScope(); + } foreach ($iterBodyScopeResult->getExitPointsByType(Break_::class) as $breakExitPoint) { $endScope = $endScope->mergeWith($breakExitPoint->getScope()); } diff --git a/src/Analyser/StmtHandler/WhileHandler.php b/src/Analyser/StmtHandler/WhileHandler.php index aa198450901..7f19d7c95f5 100644 --- a/src/Analyser/StmtHandler/WhileHandler.php +++ b/src/Analyser/StmtHandler/WhileHandler.php @@ -123,6 +123,11 @@ public function processStmt( $previousPass = $passRecord; $previousPassStorage = $storage; $backEdgeScope = $bodyScopeResult->getLoopBackEdgeScope(); + if ($backEdgeScope === null && !$bodyIsReplayable) { + // a nested loop walks only its first iteration in a pass - its unconverged + // types can make the back edge look dead, so a walk converging it decides + $backEdgeScope = $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage->duplicate(), new NoopNodeCallback(), $context->withoutTemplateArgumentResolution())->filterOutLoopExitPoints()->getLoopBackEdgeScope(); + } } finally { $scope->popExpressionResultStorage(); } diff --git a/tests/PHPStan/Analyser/nsrt/bug-15420.php b/tests/PHPStan/Analyser/nsrt/bug-15420.php new file mode 100644 index 00000000000..3a02536cf74 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15420.php @@ -0,0 +1,180 @@ +', $i); + if ($later !== []) { + $cards = $later; + } else { + break; + } + } + } +} + +/** @param string[] $cards */ +function whileLoop(array $cards): void +{ + $i = 0; + while ($i < 10) { + $seen = []; + $later = []; + foreach ($cards as $card) { + if (array_key_exists($card, $seen)) { + $later[] = $card; + } + $seen[$card] = true; + } + if ($later === []) { + break; + } + assertType('int<0, 9>', $i); + $i++; + } +} + +/** @param string[] $cards */ +function doWhileLoop(array $cards): void +{ + $i = 0; + do { + $seen = []; + $later = []; + foreach ($cards as $card) { + if (array_key_exists($card, $seen)) { + $later[] = $card; + } + $seen[$card] = true; + } + if ($later === []) { + break; + } + assertType('int<0, max>', $i); + $i++; + } while ($i < 10); +} + +/** + * @param string[] $cards + * @param int[] $rounds + */ +function foreachLoop(array $cards, array $rounds): void +{ + $i = 0; + foreach ($rounds as $round) { + $seen = []; + $later = []; + foreach ($cards as $card) { + if (array_key_exists($card, $seen)) { + $later[] = $card; + } + $seen[$card] = true; + } + if ($later === []) { + break; + } + assertType('int<0, max>', $i); + $i++; + } +} + +/** + * @param string[] $cards + * @param array{a: int, b: int, ...} $rounds + */ +function unsealedConstantArrayForeachLoop(array $cards, array $rounds): void +{ + $i = 0; + foreach ($rounds as $round) { + $seen = []; + $later = []; + foreach ($cards as $card) { + if (array_key_exists($card, $seen)) { + $later[] = $card; + } + $seen[$card] = true; + } + if ($later === []) { + break; + } + assertType('int<0, max>', $i); + $i++; + } +} + +/** @param string[] $cards */ +function innerWhile(array $cards): void +{ + for ($i = 0; $i < 10; $i++) { + $seen = false; + $later = false; + $j = 0; + while ($j < count($cards)) { + if ($seen) { + $later = true; + } + $seen = true; + $j++; + } + if (!$later) { + break; + } + assertType('int<0, 9>', $i); + } +} + +/** @param string[] $cards */ +function innerFor(array $cards): void +{ + for ($i = 0; $i < 10; $i++) { + $seen = false; + $later = false; + for ($j = 0; $j < count($cards); $j++) { + if ($seen) { + $later = true; + } + $seen = true; + } + if (!$later) { + break; + } + assertType('int<0, 9>', $i); + } +} + +/** @param string[] $cards */ +function innerDoWhile(array $cards): void +{ + for ($i = 0; $i < 10; $i++) { + $seen = false; + $later = false; + $j = 0; + do { + if ($seen) { + $later = true; + } + $seen = true; + $j++; + } while ($j < count($cards)); + if (!$later) { + break; + } + assertType('int<0, 9>', $i); + } +} diff --git a/tests/PHPStan/Rules/Comparison/NumberComparisonOperatorsConstantConditionRuleTest.php b/tests/PHPStan/Rules/Comparison/NumberComparisonOperatorsConstantConditionRuleTest.php index 4e1074b480a..1e46c065b95 100644 --- a/tests/PHPStan/Rules/Comparison/NumberComparisonOperatorsConstantConditionRuleTest.php +++ b/tests/PHPStan/Rules/Comparison/NumberComparisonOperatorsConstantConditionRuleTest.php @@ -326,6 +326,11 @@ public function testBug15243(): void $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-15243.php'], []); } + public function testBug15420(): void + { + $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-15420.php'], []); + } + public function testShiftLeftUnboundedRange(): void { $this->analyse([__DIR__ . '/../../Analyser/nsrt/shift-left-unbounded-range.php'], []); diff --git a/turbo-ext/src/DoWhileHandler.cpp b/turbo-ext/src/DoWhileHandler.cpp index 8bd3377df21..931908c68a3 100644 --- a/turbo-ext/src/DoWhileHandler.cpp +++ b/turbo-ext/src/DoWhileHandler.cpp @@ -130,6 +130,10 @@ class DoWhileHandler if (UNEXPECTED(bodyScopeResult.isUndef())) return; zv::Val backEdgeScope = pt_internal_statement_result_loop_back_edge_scope(bodyScopeResult.raw()); if (UNEXPECTED(backEdgeScope.isUndef())) return; + if (backEdgeScope.isNull() && !bodyIsReplayable) { + backEdgeScope = ptlh::convergedPassBackEdgeScope(nodeScopeResolver, stmt, stmtsHold.raw(), bodyScope.raw(), storage.raw(), context); + if (UNEXPECTED(backEdgeScope.isUndef())) return; + } if (!backEdgeScope.isNull()) { if (UNEXPECTED(!ptlh::otherMergeWith(backEdgeScope.raw(), finalScope))) return; } diff --git a/turbo-ext/src/ForHandler.cpp b/turbo-ext/src/ForHandler.cpp index f187def3240..02b0cec7eab 100644 --- a/turbo-ext/src/ForHandler.cpp +++ b/turbo-ext/src/ForHandler.cpp @@ -219,6 +219,12 @@ class ForHandler // entry state differs from the previous pass's (StatementsHandler::processLoopPassStmtNodes()) zv::Val previousPass = zv::Val::null(); zv::Val previousPassStorage = zv::Val::null(); + bool bodyIsReplayable; + { + zval *bodyStmts = ptsh::readNodeProperty(pt_fh_stmts_site, stmt, PT_LC("stmts")); + if (UNEXPECTED(bodyStmts == NULL)) return zv::Val(); + if (UNEXPECTED(!pt_node_scope_resolver_is_replayable_convergence_body(nodeScopeResolver, stmt, bodyStmts, bodyIsReplayable))) return zv::Val(); + } do { zv::Val prevScope = zv::Val::copyOf(bodyScope.ref()); zv::Val storage = pt_expression_result_storage_duplicate(originalStorage); @@ -279,6 +285,10 @@ class ForHandler } zv::Val backEdgeScope = pt_internal_statement_result_loop_back_edge_scope(bodyScopeResult.raw()); if (UNEXPECTED(backEdgeScope.isUndef())) return; + if (backEdgeScope.isNull() && !bodyIsReplayable) { + backEdgeScope = ptlh::convergedPassBackEdgeScope(nodeScopeResolver, stmt, stmtsHold.raw(), bodyScope.raw(), storage.raw(), context); + if (UNEXPECTED(backEdgeScope.isUndef())) return; + } if (backEdgeScope.isNull()) { bodyScope = std::move(prevScope); brokeOut = true; diff --git a/turbo-ext/src/ForeachHandler.cpp b/turbo-ext/src/ForeachHandler.cpp index 93d6fd6990c..e7e92bffb71 100644 --- a/turbo-ext/src/ForeachHandler.cpp +++ b/turbo-ext/src/ForeachHandler.cpp @@ -543,6 +543,10 @@ class ForeachHandler if (UNEXPECTED(bodyScopeResult.isUndef())) return; zv::Val backEdgeScope = pt_internal_statement_result_loop_back_edge_scope(bodyScopeResult.raw()); if (UNEXPECTED(backEdgeScope.isUndef())) return; + if (backEdgeScope.isNull() && !bodyIsReplayable) { + backEdgeScope = ptlh::convergedPassBackEdgeScope(nodeScopeResolver, stmt, parts.stmts.raw(), bodyScope.raw(), storage.raw(), context); + if (UNEXPECTED(backEdgeScope.isUndef())) return; + } if (backEdgeScope.isNull()) { bodyScope = std::move(prevScope); brokeOut = true; @@ -1724,6 +1728,14 @@ class ForeachHandler if (UNEXPECTED(iterBodyScopeResult.isUndef())) return false; zv::Val backEdgeScope = pt_internal_statement_result_loop_back_edge_scope(iterBodyScopeResult.raw()); if (UNEXPECTED(backEdgeScope.isUndef())) return false; + if (backEdgeScope.isNull()) { + bool bodyIsReplayable; + if (UNEXPECTED(!pt_node_scope_resolver_is_replayable_convergence_body(nodeScopeResolver, stmt, parts.stmts.raw(), bodyIsReplayable))) return false; + if (!bodyIsReplayable) { + backEdgeScope = ptlh::convergedPassBackEdgeScope(nodeScopeResolver, stmt, parts.stmts.raw(), iterBodyScope.raw(), iterStorage.raw(), context); + if (UNEXPECTED(backEdgeScope.isUndef())) return false; + } + } { zv::Val breaks = ptlh::breakExitPoints(iterBodyScopeResult.raw()); if (UNEXPECTED(breaks.isUndef())) return false; diff --git a/turbo-ext/src/LoopHandlerCalls.h b/turbo-ext/src/LoopHandlerCalls.h index ba2067e2346..f7fb5c8d3c5 100644 --- a/turbo-ext/src/LoopHandlerCalls.h +++ b/turbo-ext/src/LoopHandlerCalls.h @@ -79,6 +79,27 @@ inline zv::Val newPassNodeCallback(bool replayable) return replayable ? newRecordingNodeCallback() : newNoopNodeCallback(); } +/* a nested loop walks only its first iteration in a pass - its unconverged + * types can make the back edge look dead, so a walk converging it decides: + * $nodeScopeResolver->processStmtNodesInternal($stmt, $stmt->stmts, $entryScope, + * $storage->duplicate(), new NoopNodeCallback(), $context->withoutTemplateArgumentResolution()) + * ->filterOutLoopExitPoints()->getLoopBackEdgeScope() + * (UNDEF = pending exception) */ +inline zv::Val convergedPassBackEdgeScope(zval *nodeScopeResolver, zval *stmt, zval *stmts, zval *entryScope, zval *storage, zval *context) +{ + zv::Val storageCopy = pt_expression_result_storage_duplicate(storage); + if (UNEXPECTED(storageCopy.isUndef())) return zv::Val(); + zv::Val noop = newNoopNodeCallback(); + if (UNEXPECTED(noop.isUndef())) return zv::Val(); + zv::Val walkContext = pt_statement_context_without_template_argument_resolution(context); + if (UNEXPECTED(walkContext.isUndef())) return zv::Val(); + zv::Val walked = pt_node_scope_resolver_process_stmt_nodes_internal(nodeScopeResolver, stmt, stmts, entryScope, storageCopy.raw(), noop.raw(), walkContext.raw()); + if (UNEXPECTED(walked.isUndef())) return zv::Val(); + zv::Val result = pt_internal_statement_result_filter_out_loop_exit_points(walked.raw()); + if (UNEXPECTED(result.isUndef())) return zv::Val(); + return pt_internal_statement_result_loop_back_edge_scope(result.raw()); +} + /* $into = array_merge($into, $more): the array itself when $into is empty and * $more a list without holes (what array_merge() returns then), appended * otherwise; false = pending exception */ diff --git a/turbo-ext/src/WhileHandler.cpp b/turbo-ext/src/WhileHandler.cpp index d87791b76ca..ec6e6bf1ae9 100644 --- a/turbo-ext/src/WhileHandler.cpp +++ b/turbo-ext/src/WhileHandler.cpp @@ -193,6 +193,10 @@ class WhileHandler bodyScopeResult = pt_internal_statement_result_filter_out_loop_exit_points(walked.raw()); if (UNEXPECTED(bodyScopeResult.isUndef())) return; backEdgeScope = pt_internal_statement_result_loop_back_edge_scope(bodyScopeResult.raw()); + if (UNEXPECTED(backEdgeScope.isUndef())) return; + if (backEdgeScope.isNull() && !bodyIsReplayable) { + backEdgeScope = ptlh::convergedPassBackEdgeScope(nodeScopeResolver, stmt, stmtsHold.raw(), bodyScope.raw(), storage.raw(), context); + } }(); pt_finally([&]() { (void) pt_mutating_scope_pop_expression_result_storage(Z_OBJ_P(scope.raw())); }); if (UNEXPECTED(EG(exception))) return zv::Val();