Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/Analyser/NodeScopeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ class NodeScopeResolver
private const LOOP_SCOPE_ITERATIONS = 3;
private const GENERALIZE_AFTER_ITERATION = 1;
private const FOREACH_UNROLL_LIMIT = 16;
private const FOREACH_UNROLL_NESTED_LIMIT = 16;
Copy link
Copy Markdown
Contributor

@staabm staabm May 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of removing this feature as a whole, would it be enough to reduce the threshold to a lower value, so we can still have the more precise results in some cases.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Here's a summary of the changes:

Instead of completely disabling nested foreach unrolling, restored the factor-based approach with a lower limit:

  • FOREACH_UNROLL_NESTED_LIMIT = 8 (reduced from original 16) — only checked when already inside an unrolled foreach (factor > 1), so top-level foreaches are unaffected
  • The factor is now properly propagated through the final convergence pass via enterUnrolledForeach(totalKeys), fixing a bug in the original code where the final pass reset the factor to 1, allowing exponential re-unrolling
  • This allows 2-3 levels of nesting with small constant arrays while preventing the exponential blowup for deeper nesting
  • All three test cases (bug-7978, bug-9332, bug-14489) regain their precise type expectations
  • Benchmark (bug-14674.php with 13 nested levels) still completes in ~3.4s

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a test which demonstrates the type degradation happens at the configured threshold


/** @var array<string, true> filePath(string) => bool(true) */
private array $analysedFiles = [];
Expand Down Expand Up @@ -1486,7 +1485,8 @@ public function processStmtNode(
$bodyScope = $bodyScope->mergeWith($this->polluteScopeWithAlwaysIterableForeach ? $scope->filterByTruthyValue($arrayComparisonExpr) : $scope);
$storage = $originalStorage;
$bodyScope = $this->enterForeach($bodyScope, $storage, $originalScope, $stmt, $nodeCallback);
$finalScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $context)->filterOutLoopExitPoints();
$finalPassContext = $unrolledEndScope !== null ? $context->enterUnrolledForeach() : $context;
$finalScopeResult = $this->processStmtNodesInternal($stmt, $stmt->stmts, $bodyScope, $storage, $nodeCallback, $finalPassContext)->filterOutLoopExitPoints();
$finalScope = $finalScopeResult->getScope();
$scopesWithIterableValueType = [];

Expand Down Expand Up @@ -4114,7 +4114,7 @@ private function tryProcessUnrolledConstantArrayForeach(
if ($totalKeys === 0 || $totalKeys > self::FOREACH_UNROLL_LIMIT) {
return null;
}
if ($context->getForeachUnrollFactor() * $totalKeys > self::FOREACH_UNROLL_NESTED_LIMIT) {
if ($context->isInsideUnrolledForeach()) {
return null;
}

Expand All @@ -4129,7 +4129,7 @@ private function tryProcessUnrolledConstantArrayForeach(
$allChainScopes = [];
$allBreakScopes = [];

$bodyContext = $context->enterUnrolledForeach($totalKeys);
$bodyContext = $context->enterUnrolledForeach();

foreach ($constantArrays as $arrayIndex => $constantArray) {
$keyTypes = $constantArray->getKeyTypes();
Expand Down
12 changes: 6 additions & 6 deletions src/Analyser/StatementContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ final class StatementContext

private function __construct(
private bool $isTopLevel,
private int $foreachUnrollFactor = 1,
private bool $insideUnrolledForeach = false,
)
{
}
Expand All @@ -41,23 +41,23 @@ public function isTopLevel(): bool
return $this->isTopLevel;
}

public function getForeachUnrollFactor(): int
public function isInsideUnrolledForeach(): bool
{
return $this->foreachUnrollFactor;
return $this->insideUnrolledForeach;
}

public function enterDeep(): self
{
if ($this->isTopLevel) {
return new self(false, $this->foreachUnrollFactor);
return new self(false, $this->insideUnrolledForeach);
}

return $this;
}

public function enterUnrolledForeach(int $totalKeys): self
public function enterUnrolledForeach(): self
{
return new self($this->isTopLevel, $this->foreachUnrollFactor * $totalKeys);
return new self($this->isTopLevel, true);
}

}
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/bug-14489.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ function () {
}

$values = array_values($cData);
assertType('array{array{1}, array{4}}', $values);
assertType('array{0: non-empty-array<0|1, 1|4>, 1?: non-empty-array<0|1, 1|4>}', $values);
};

function () {
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/bug-7978.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public function doSomething(): void
foreach ($fields as $field) {
$credentials[$field] = 'fake';
}
assertType("array{app_id: 'fake', app_key: 'fake'}|array{username: 'fake', password: 'fake'}", $credentials);
assertType("non-empty-array{password?: 'fake', username?: 'fake', app_id?: 'fake', app_key?: 'fake'}", $credentials);
}
}

Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/bug-9332.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public function sayHello(): void
}
}

assertType("array{a: 'asdfghi', bd: int<1, 1000>, be: int<1, 1000>, cd: int<1, 1000>, ce: int<1, 1000>}", $data);
assertType("array{a: 'asdfghi', bd?: int<1, 1000>, be?: int<1, 1000>, cd?: int<1, 1000>, ce?: int<1, 1000>}", $data);
$this->doSomething($data);
}

Expand Down
83 changes: 83 additions & 0 deletions tests/bench/data/bug-14674.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
<?php declare(strict_types = 1);

namespace Bug14674;

use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;

class PhpStanPerformanceTest extends TestCase {

/**
* @param \Closure $closure
*/
#[DataProvider('performanceProvider')]
public function testPerformance(\Closure $closure): void {
$this->assertTrue($closure());
}

/**
* @return iterable<string, array{closure: \Closure}>
*/
public static function performanceProvider(): iterable {
foreach(['0', '1'] as $level_1) {
$keys = [$level_1];

foreach(['0', '1'] as $level_2) {
$keys[] = $level_2;

foreach(['0', '1'] as $level_3) {
$keys[] = $level_3;

foreach(['0', '1'] as $level_4) {
$keys[] = $level_4;

foreach(['0', '1'] as $level_5) {
$keys[] = $level_5;

foreach(['0', '1'] as $level_6) {
$keys[] = $level_6;

foreach(['0', '1'] as $level_7) {
$keys[] = $level_7;

foreach(['0', '1'] as $level_8) {
$keys[] = $level_8;

foreach(['0', '1'] as $level_9) {
$keys[] = $level_9;

foreach(['0', '1'] as $level_10) {
$keys[] = $level_10;

foreach(['0', '1'] as $level_11) {
$keys[] = $level_11;

foreach(['0', '1'] as $level_12) {
$keys[] = $level_12;

foreach(['0', '1'] as $level_13) {
$keys[] = $level_13;

$case = [
'closure' => function () use ($level_1, $level_3, $level_5, $level_13) {
return $level_1 === '1' && $level_3 === '1' && $level_5 === '1' && $level_13 === '1';
},
];

yield implode('-', $keys) => $case;
}
}
}
}
}
}
}
}
}
}
}
}
}
}

}
Loading