Skip to content

Commit

Permalink
Rework match expression analysis with enum with performance in mind
Browse files Browse the repository at this point in the history
  • Loading branch information
@ondrejmirtes
ondrejmirtes committed Jul 3, 2024
1 parent 344c21f commit b98abe0
Show file tree
Hide file tree
Showing 6 changed files with 1,061 additions and 13 deletions.
94 changes: 91 additions & 3 deletions src/Analyser/MutatingScope.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1635,10 +1635,92 @@ static function (Node $node, Scope $scope) use ($arrowScope, &$arrowFunctionImpu
return $generatorReturnType;
} elseif ($node instanceof Expr\Match_) {
$cond = $node->cond;
$condType = $this->getType($cond);
$types = [];

$matchScope = $this;
foreach ($node->arms as $arm) {
$arms = $node->arms;
if ($condType->isEnum()->yes()) {
// enum match analysis would work even without this if branch
// but would be much slower
// this avoids using ObjectType::$subtractedType which is slow for huge enums
// because of repeated union type normalization
$enumCases = $condType->getEnumCases();
if (count($enumCases) > 0) {
$indexedEnumCases = [];
foreach ($enumCases as $enumCase) {
$indexedEnumCases[strtolower($enumCase->getClassName())][$enumCase->getEnumCaseName()] = $enumCase;
}
$unusedIndexedEnumCases = $indexedEnumCases;

foreach ($arms as $i => $arm) {
if ($arm->conds === null) {
continue;
}

$conditionCases = [];
foreach ($arm->conds as $armCond) {
if (!$armCond instanceof Expr\ClassConstFetch) {
continue 2;
}
if (!$armCond->class instanceof Name) {
continue 2;
}
if (!$armCond->name instanceof Node\Identifier) {
continue 2;
}
$fetchedClassName = $this->resolveName($armCond->class);
$loweredFetchedClassName = strtolower($fetchedClassName);
if (!array_key_exists($loweredFetchedClassName, $indexedEnumCases)) {
continue 2;
}

$caseName = $armCond->name->toString();
if (!array_key_exists($caseName, $indexedEnumCases[$loweredFetchedClassName])) {
continue 2;
}

$conditionCases[] = $indexedEnumCases[$loweredFetchedClassName][$caseName];
unset($unusedIndexedEnumCases[$loweredFetchedClassName][$caseName]);
}

$conditionCasesCount = count($conditionCases);
if ($conditionCasesCount === 0) {
throw new ShouldNotHappenException();
} elseif ($conditionCasesCount === 1) {
$conditionCaseType = $conditionCases[0];
} else {
$conditionCaseType = new UnionType($conditionCases);
}

$types[] = $matchScope->addTypeToExpression(
$cond,
$conditionCaseType,
)->getType($arm->body);
unset($arms[$i]);
}

$remainingCases = [];
foreach ($unusedIndexedEnumCases as $cases) {
foreach ($cases as $case) {
$remainingCases[] = $case;
}
}

$remainingCasesCount = count($remainingCases);
if ($remainingCasesCount === 0) {
$remainingType = new NeverType();
} elseif ($remainingCasesCount === 1) {
$remainingType = $remainingCases[0];
} else {
$remainingType = new UnionType($remainingCases);
}

$matchScope = $matchScope->addTypeToExpression($cond, $remainingType);
}
}

foreach ($arms as $arm) {
if ($arm->conds === null) {
if ($node->hasAttribute(self::KEEP_VOID_ATTRIBUTE_NAME)) {
$arm->body->setAttribute(self::KEEP_VOID_ATTRIBUTE_NAME, $node->getAttribute(self::KEEP_VOID_ATTRIBUTE_NAME));
Expand Down Expand Up @@ -3842,7 +3924,7 @@ public function specifyExpressionType(Expr $expr, Type $type, Type $nativeType,
$nativeTypes = $scope->nativeExpressionTypes;
$nativeTypes[$exprString] = new ExpressionTypeHolder($expr, $nativeType, $certainty);

return $this->scopeFactory->create(
$scope = $this->scopeFactory->create(
$this->context,
$this->isDeclareStrictTypes(),
$this->getFunction(),
Expand All @@ -3860,6 +3942,12 @@ public function specifyExpressionType(Expr $expr, Type $type, Type $nativeType,
$this->parentScope,
$this->nativeTypesPromoted,
);

if ($expr instanceof AlwaysRememberedExpr) {
return $scope->specifyExpressionType($expr->expr, $type, $nativeType, $certainty);
}

return $scope;
}

public function assignExpression(Expr $expr, Type $type, ?Type $nativeType = null): self
Expand Down Expand Up @@ -4074,7 +4162,7 @@ private function setExpressionCertainty(Expr $expr, TrinaryLogic $certainty): se
);
}

private function addTypeToExpression(Expr $expr, Type $type): self
public function addTypeToExpression(Expr $expr, Type $type): self
{
$originalExprType = $this->getType($expr);
$nativeType = $this->getNativeType($expr);
Expand Down
139 changes: 135 additions & 4 deletions src/Analyser/NodeScopeResolver.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -203,11 +203,13 @@
use function is_array;
use function is_int;
use function is_string;
use function ksort;
use function sprintf;
use function str_starts_with;
use function strtolower;
use function trim;
use const PHP_VERSION_ID;
use const SORT_NUMERIC;

class NodeScopeResolver
{
Expand Down Expand Up @@ -3372,6 +3374,7 @@ static function (Node $node, Scope $scope) use ($nodeCallback): void {
$hasYield = true;
} elseif ($expr instanceof Expr\Match_) {
$deepContext = $context->enterDeep();
$condType = $scope->getType($expr->cond);
$condResult = $this->processExprNode($stmt, $expr->cond, $scope, $nodeCallback, $deepContext);
$scope = $condResult->getScope();
$hasYield = $condResult->hasYield();
Expand All @@ -3381,11 +3384,137 @@ static function (Node $node, Scope $scope) use ($nodeCallback): void {
$armNodes = [];
$hasDefaultCond = false;
$hasAlwaysTrueCond = false;
foreach ($expr->arms as $arm) {
$arms = $expr->arms;
if ($condType->isEnum()->yes()) {
// enum match analysis would work even without this if branch
// but would be much slower
// this avoids using ObjectType::$subtractedType which is slow for huge enums
// because of repeated union type normalization
$enumCases = $condType->getEnumCases();
if (count($enumCases) > 0) {
$indexedEnumCases = [];
foreach ($enumCases as $enumCase) {
$indexedEnumCases[strtolower($enumCase->getClassName())][$enumCase->getEnumCaseName()] = $enumCase;
}
$unusedIndexedEnumCases = $indexedEnumCases;
foreach ($arms as $i => $arm) {
if ($arm->conds === null) {
continue;
}

$condNodes = [];
$conditionCases = [];
foreach ($arm->conds as $cond) {
if (!$cond instanceof Expr\ClassConstFetch) {
continue 2;
}
if (!$cond->class instanceof Name) {
continue 2;
}
if (!$cond->name instanceof Node\Identifier) {
continue 2;
}
$fetchedClassName = $scope->resolveName($cond->class);
$loweredFetchedClassName = strtolower($fetchedClassName);
if (!array_key_exists($loweredFetchedClassName, $indexedEnumCases)) {
continue 2;
}

if (!array_key_exists($loweredFetchedClassName, $unusedIndexedEnumCases)) {
throw new ShouldNotHappenException();
}

$caseName = $cond->name->toString();
if (!array_key_exists($caseName, $indexedEnumCases[$loweredFetchedClassName])) {
continue 2;
}

$enumCase = $indexedEnumCases[$loweredFetchedClassName][$caseName];
$conditionCases[] = $enumCase;
$armConditionScope = $matchScope;
if (!array_key_exists($caseName, $unusedIndexedEnumCases[$loweredFetchedClassName])) {
// force "always false"
$armConditionScope = $armConditionScope->removeTypeFromExpression(
$expr->cond,
$enumCase,
);
} elseif (count($unusedIndexedEnumCases[$loweredFetchedClassName]) === 1) {
$hasAlwaysTrueCond = true;

// force "always true"
$armConditionScope = $armConditionScope->addTypeToExpression(
$expr->cond,
$enumCase,
);
}

$this->processExprNode($stmt, $cond, $armConditionScope, $nodeCallback, $deepContext);

$condNodes[] = new MatchExpressionArmCondition(
$cond,
$armConditionScope,
$cond->getStartLine(),
);

unset($unusedIndexedEnumCases[$loweredFetchedClassName][$caseName]);
}

$conditionCasesCount = count($conditionCases);
if ($conditionCasesCount === 0) {
throw new ShouldNotHappenException();
} elseif ($conditionCasesCount === 1) {
$conditionCaseType = $conditionCases[0];
} else {
$conditionCaseType = new UnionType($conditionCases);
}

$matchArmBodyScope = $matchScope->addTypeToExpression(
$expr->cond,
$conditionCaseType,
);
$matchArmBody = new MatchExpressionArmBody($matchArmBodyScope, $arm->body);
$armNodes[$i] = new MatchExpressionArm($matchArmBody, $condNodes, $arm->getStartLine());

$armResult = $this->processExprNode(
$stmt,
$arm->body,
$matchArmBodyScope,
$nodeCallback,
ExpressionContext::createTopLevel(),
);
$armScope = $armResult->getScope();
$scope = $scope->mergeWith($armScope);
$hasYield = $hasYield || $armResult->hasYield();
$throwPoints = array_merge($throwPoints, $armResult->getThrowPoints());
$impurePoints = array_merge($impurePoints, $armResult->getImpurePoints());

unset($arms[$i]);
}

$remainingCases = [];
foreach ($unusedIndexedEnumCases as $cases) {
foreach ($cases as $case) {
$remainingCases[] = $case;
}
}

$remainingCasesCount = count($remainingCases);
if ($remainingCasesCount === 0) {
$remainingType = new NeverType();
} elseif ($remainingCasesCount === 1) {
$remainingType = $remainingCases[0];
} else {
$remainingType = new UnionType($remainingCases);
}

$matchScope = $matchScope->addTypeToExpression($expr->cond, $remainingType);
}
}
foreach ($arms as $i => $arm) {
if ($arm->conds === null) {
$hasDefaultCond = true;
$matchArmBody = new MatchExpressionArmBody($matchScope, $arm->body);
$armNodes[] = new MatchExpressionArm($matchArmBody, [], $arm->getStartLine());
$armNodes[$i] = new MatchExpressionArm($matchArmBody, [], $arm->getStartLine());
$armResult = $this->processExprNode($stmt, $arm->body, $matchScope, $nodeCallback, ExpressionContext::createTopLevel());
$matchScope = $armResult->getScope();
$hasYield = $hasYield || $armResult->hasYield();
Expand Down Expand Up @@ -3438,7 +3567,7 @@ static function (Node $node, Scope $scope) use ($nodeCallback): void {
$bodyScope = $this->processExprNode($stmt, $filteringExpr, $matchScope, static function (): void {
}, $deepContext)->getTruthyScope();
$matchArmBody = new MatchExpressionArmBody($bodyScope, $arm->body);
$armNodes[] = new MatchExpressionArm($matchArmBody, $condNodes, $arm->getStartLine());
$armNodes[$i] = new MatchExpressionArm($matchArmBody, $condNodes, $arm->getStartLine());

$armResult = $this->processExprNode(
$stmt,
Expand All @@ -3460,7 +3589,9 @@ static function (Node $node, Scope $scope) use ($nodeCallback): void {
$throwPoints[] = ThrowPoint::createExplicit($scope, new ObjectType(UnhandledMatchError::class), $expr, false);
}

$nodeCallback(new MatchExpressionNode($expr->cond, $armNodes, $expr, $matchScope), $scope);
ksort($armNodes, SORT_NUMERIC);

$nodeCallback(new MatchExpressionNode($expr->cond, array_values($armNodes), $expr, $matchScope), $scope);
} elseif ($expr instanceof AlwaysRememberedExpr) {
$result = $this->processExprNode($stmt, $expr->getExpr(), $scope, $nodeCallback, $context);
$hasYield = $result->hasYield();
Expand Down
10 changes: 10 additions & 0 deletions tests/PHPStan/Analyser/AnalyserIntegrationTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1373,6 +1373,16 @@ public function testBug10867(): void
$this->assertNoErrors($errors);
}

public function testBug11263(): void
{
if (PHP_VERSION_ID < 80100) {
$this->markTestSkipped('Test requires PHP 8.1.');
}

$errors = $this->runAnalyse(__DIR__ . '/data/bug-11263.php');
$this->assertNoErrors($errors);
}

public function testBug11147(): void
{
if (PHP_VERSION_ID < 80000) {
Expand Down
Loading

0 comments on commit b98abe0

Please sign in to comment.