Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
18 changes: 14 additions & 4 deletions src/Analyser/ExprHandler/Helper/ClosureTypeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ public function getClosureType(
}

$cachedTypes = $this->findCachedTypes($expr);
$cacheKey = $this->closureContextCacheKey($scope, $expr, $callableParameters, $parameters);
$cacheKey = $this->closureContextCacheKey($scope, $expr, $callableParameters, $parameters, $contextFree);
if (array_key_exists($cacheKey, $cachedTypes)) {
return $this->createClosureTypeFromCache($scope, $expr, $parameters, $isVariadic, $cachedTypes[$cacheKey], $contextFree);
}
Expand Down Expand Up @@ -344,6 +344,7 @@ public function buildClosureTypeForClosure(
$expr,
$native ? $nativeCallableParameters : $callableParameters,
$parameters,
$contextFree,
),
$native,
$storage,
Expand Down Expand Up @@ -386,6 +387,7 @@ public function buildClosureTypeForArrowFunction(
$expr,
$native ? $nativeCallableParameters : $callableParameters,
$parameters,
$contextFree,
), $contextFree);
}

Expand All @@ -395,10 +397,18 @@ public function buildClosureTypeForArrowFunction(
* feeds in - array_map style callers type the same closure node per
* element through the callable parameters.
*
* The function-call stack is not part of it: the body is walked on a scope
* with an empty stack, and what the stack contributes when the caller
* passes no parameter types (the innermost parameter's callable type) is
* already in the key as the callable parameters. Keying on the stack made
* the same closure, typed the same way, miss the cache when it was asked
* from inside the call it is passed to and again from outside it.
* $contextFree still tells apart a closure typed with no context at all.
*
* @param array<ParameterReflection>|null $callableParameters
* @param array<ParameterReflection> $parameters
*/
private function closureContextCacheKey(MutatingScope $scope, Node\Expr\Closure|ArrowFunction $expr, ?array $callableParameters, array $parameters): string
private function closureContextCacheKey(MutatingScope $scope, Node\Expr\Closure|ArrowFunction $expr, ?array $callableParameters, array $parameters, bool $contextFree): string
{
$parts = [];
foreach ($callableParameters ?? $parameters as $parameter) {
Expand All @@ -410,7 +420,7 @@ private function closureContextCacheKey(MutatingScope $scope, Node\Expr\Closure|
// for the second pass change its type
$frame = $scope->getCurrentTemplateArgumentFrame();

return $scope->getClosureScopeCacheKey($this->freeVariableRoots($expr)) . '/' . implode('|', $parts) . ($scope->nativeTypesPromoted ? '/native' : '/phpdoc')
return $scope->getClosureScopeCacheKey($this->freeVariableRoots($expr)) . '/' . implode('|', $parts) . ($contextFree ? '/free' : '') . ($scope->nativeTypesPromoted ? '/native' : '/phpdoc')
. ($frame !== null ? $frame->getResolutionCacheKeySuffix() : '');
}

Expand Down Expand Up @@ -912,7 +922,7 @@ private function assembleClosureType(
$impurePointsForClosureType = array_map(static fn (ImpurePoint $impurePoint) => new SimpleImpurePoint($impurePoint->getIdentifier(), $impurePoint->getDescription(), $impurePoint->isCertain()), $impurePoints);

$cachedTypes = $this->findCachedTypes($expr);
$cacheKey ??= $this->closureContextCacheKey($scope, $expr, null, $parameters);
$cacheKey ??= $this->closureContextCacheKey($scope, $expr, null, $parameters, $contextFree);
$cachedTypes[$cacheKey] = [
'returnType' => $returnType,
'throwPoints' => $throwPointsForClosureType,
Expand Down
11 changes: 0 additions & 11 deletions src/Analyser/MutatingScope.php
Original file line number Diff line number Diff line change
Expand Up @@ -1046,17 +1046,6 @@ public function getClosureScopeCacheKey(?array $relevantRoots = null): string
}
$parts[] = sprintf('%s::%s', $exprString, $expressionTypeHolder->getType()->describe(VerbosityLevel::cache()));
}
$parts[] = '---';

$parts[] = sprintf(':%d', count($this->inFunctionCallsStack));
foreach ($this->inFunctionCallsStack as [, $parameter]) {
if ($parameter === null) {
$parts[] = ',null';
continue;
}

$parts[] = sprintf(',%s', $parameter->getType()->describe(VerbosityLevel::cache()));
}

return md5(implode("\n", $parts));
}
Expand Down
17 changes: 17 additions & 0 deletions tests/PHPStan/Analyser/AnalyserIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Bug4288\MyClass;
use Bug4713\Service;
use ExtendingKnownClassWithCheck\Foo;
use PHPStan\Analyser\Generics\TemplateArgumentStats;
use PHPStan\Reflection\InitializerExprContext;
use PHPStan\Reflection\InitializerExprTypeResolver;
use PHPStan\Reflection\SignatureMap\SignatureMapProvider;
Expand Down Expand Up @@ -1226,6 +1227,22 @@ public function testBug7927(): void
$this->assertSame('Enum case Bug7927\Test::Two does not have a value but the enum is backed with the "int" type.', $errors[1]->getMessage());
}

public function testClosureInGenericCallIsWalkedOncePerTyping(): void
{
TemplateArgumentStats::reset();
TemplateArgumentStats::$enabled = true;
try {
// each closure is asked for with the same parameter types from inside
// the call (its argument) and from outside it (the call's type) - one
// body walk answers both
$errors = $this->runAnalyse(__DIR__ . '/data/closure-walks-in-generic-call.php');
$this->assertNoErrors($errors);
$this->assertSame(2, TemplateArgumentStats::getCounters()['closureTypeBodyWalks']);
} finally {
TemplateArgumentStats::$enabled = false;
}
}

public static function getAdditionalConfigFiles(): array
{
return [
Expand Down
42 changes: 42 additions & 0 deletions tests/PHPStan/Analyser/data/closure-walks-in-generic-call.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
<?php declare(strict_types = 1);

namespace ClosureWalksInGenericCall;

/**
* @template T
* @template U
*
* @param list<T> $list
* @param callable(T): U $fn
*
* @return list<U>
*/
function mapList(array $list, callable $fn): array
{
$result = [];
foreach ($list as $item) {
$result[] = $fn($item);
}

return $result;
}

/**
* @param list<int> $integers
* @return list<array{int}>
*/
function wrap(array $integers): array
{
return mapList($integers, fn ($i) => [$i]);
}

/**
* @param list<int> $integers
* @return list<string>
*/
function stringify(array $integers): array
{
return mapList($integers, function ($i) {
return (string) $i;
});
}
13 changes: 8 additions & 5 deletions turbo-ext/src/ClosureTypeResolver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,7 @@ class ClosureTypeResolver
if (shallow) return signatureClosureType(scope, expr, p.parameters.raw(), p.isVariadic);

zv::Val cachedTypes = findCachedTypes(expr);
zv::Str cacheKey = closureContextCacheKey(scope, expr, p.callableParameters.isNull() ? NULL : p.callableParameters.raw(), p.parameters.raw());
zv::Str cacheKey = closureContextCacheKey(scope, expr, p.callableParameters.isNull() ? NULL : p.callableParameters.raw(), p.parameters.raw(), p.contextFree);
if (UNEXPECTED(cacheKey.isNull())) return zv::Val();
zval *cached = zend_symtable_find(Z_ARRVAL_P(cachedTypes.raw()), cacheKey.get());
if (cached != NULL) {
Expand Down Expand Up @@ -456,7 +456,7 @@ class ClosureTypeResolver

/* the cache key of a flavour build: closureContextCacheKey($native ?
* $scope->doNotTreatPhpDocTypesAsCertain() : $scope, $expr, $native ?
* $nativeCallableParameters : $callableParameters, $parameters) */
* $nativeCallableParameters : $callableParameters, $parameters, $contextFree) */
zv::Str flavourCacheKey(zval *scope, zval *expr, Parameters &p, bool native)
{
zv::Val nativeScope;
Expand All @@ -467,11 +467,11 @@ class ClosureTypeResolver
keyScope = nativeScope.raw();
}
zv::Val &callable = native ? p.nativeCallableParameters : p.callableParameters;
return closureContextCacheKey(keyScope, expr, callable.isNull() ? NULL : callable.raw(), p.parameters.raw());
return closureContextCacheKey(keyScope, expr, callable.isNull() ? NULL : callable.raw(), p.parameters.raw(), p.contextFree);
}

/* Mirrors closureContextCacheKey() ($callableParameters NULL for null) */
zv::Str closureContextCacheKey(zval *scope, zval *expr, zval *callableParameters, zval *parameters)
zv::Str closureContextCacheKey(zval *scope, zval *expr, zval *callableParameters, zval *parameters, bool contextFree)
{
zval *cacheLevel = pt_verbosity_level_singleton(PT_VERBOSITY_LEVEL_CACHE);
if (UNEXPECTED(cacheLevel == NULL)) return zv::Str();
Expand Down Expand Up @@ -531,6 +531,9 @@ class ClosureTypeResolver
smart_str_append(&key, parts.s);
}
smart_str_free(&parts);
if (contextFree) {
smart_str_appendl(&key, "/free", sizeof("/free") - 1);
}
if (promoted) {
smart_str_appendl(&key, "/native", sizeof("/native") - 1);
} else {
Expand Down Expand Up @@ -1275,7 +1278,7 @@ class ClosureTypeResolver

zv::Str ownKey;
if (cacheKey == NULL) {
ownKey = closureContextCacheKey(scope, expr, NULL, parameters);
ownKey = closureContextCacheKey(scope, expr, NULL, parameters, contextFree);
if (UNEXPECTED(ownKey.isNull())) return zv::Val();
cacheKey = ownKey.get();
}
Expand Down
49 changes: 0 additions & 49 deletions turbo-ext/src/MutatingScope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2367,55 +2367,6 @@ class MutatingScope
smart_str_appendl(&parts, "::", 2);
smart_str_append(&parts, Z_STR_P(description.raw()));
}
separate();
smart_str_appendl(&parts, "---", 3);

if (UNEXPECTED(!requireSlot(PT_MS_PROP_IN_FUNCTION_CALLS_STACK, "inFunctionCallsStack"))) {
smart_str_free(&parts);
return zv::Val();
}
zv::ArrRef stack(slot(PT_MS_PROP_IN_FUNCTION_CALLS_STACK).raw());
separate();
smart_str_appendc(&parts, ':');
smart_str_append_long(&parts, (zend_long) stack.size());
for (auto entry : stack) {
/* [, $parameter] */
zv::Ref pair = entry.value().deref();
zval *parameter = NULL;
if (pair.isArray()) {
parameter = zend_hash_index_find(Z_ARRVAL_P(pair.raw()), 1);
if (parameter == NULL) {
zend_error(E_WARNING, "Undefined array key 1");
if (UNEXPECTED(EG(exception))) {
smart_str_free(&parts);
return zv::Val();
}
}
}
separate();
if (parameter == NULL || Z_TYPE_P(parameter) == IS_NULL) {
smart_str_appendl(&parts, ",null", 5);
continue;
}
ZVAL_DEREF(parameter);
if (UNEXPECTED(Z_TYPE_P(parameter) != IS_OBJECT)) {
zend_throw_error(NULL, "Call to a member function getType() on %s", zend_zval_value_name(parameter));
smart_str_free(&parts);
return zv::Val();
}
zv::Val type = pt_type_call(Z_OBJ_P(parameter), PT_LC("gettype"), 0, NULL);
if (UNEXPECTED(type.isUndef())) {
smart_str_free(&parts);
return zv::Val();
}
zv::Val description = describeAt(type, cacheLevel);
if (UNEXPECTED(description.isUndef())) {
smart_str_free(&parts);
return zv::Val();
}
smart_str_appendc(&parts, ',');
smart_str_append(&parts, Z_STR_P(description.raw()));
}
smart_str_0(&parts);

/* md5(implode("\n", $parts)) */
Expand Down
59 changes: 30 additions & 29 deletions turbo-ext/src/generated/ClosureTypeResolver.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,14 @@ inline constexpr char strings[] =
"buildClosureTypeForArrowFunction\0" /* 827 */
"callableParameters\0" /* 860 */
"parameters\0" /* 879 */
"closureContextCacheKey\0" /* 890 */
"freeVariableRoots\0" /* 913 */
"PhpParser\\Node\\Expr\0" /* 931 */
"readScope\0" /* 951 */
"readExprType\0" /* 961 */
"isVariadic\0" /* 974 */
"cacheKey\0" /* 985 */
"contextFree\0" /* 994 */
"contextFree\0" /* 890 */
"closureContextCacheKey\0" /* 902 */
"freeVariableRoots\0" /* 925 */
"PhpParser\\Node\\Expr\0" /* 943 */
"readScope\0" /* 963 */
"readExprType\0" /* 973 */
"isVariadic\0" /* 986 */
"cacheKey\0" /* 997 */
"buildClosureTypeFromClosureWalk\0" /* 1006 */
"resolveArrowFunctionReturnType\0" /* 1038 */
"deriveOnlyNeverExecutionEnds\0" /* 1069 */
Expand Down Expand Up @@ -138,28 +138,29 @@ inline constexpr reg::PackedArg args[] = {
reg::packed(348, 0, 353), /* closureContextCacheKey $expr */
reg::packed(860, MAY_BE_NULL | MAY_BE_ARRAY), /* closureContextCacheKey $callableParameters */
reg::packed(879, MAY_BE_ARRAY), /* closureContextCacheKey $parameters */
reg::packed(890, MAY_BE_BOOL), /* closureContextCacheKey $contextFree */
reg::packed(324, MAY_BE_STRING), /* closureContextCacheKey return */
reg::packed(348, 0, 353), /* freeVariableRoots $expr */
reg::packed(324, MAY_BE_NULL | MAY_BE_ARRAY), /* freeVariableRoots return */
reg::packed(482, MAY_BE_NULL, 490), /* readExprType $storage */
reg::packed(348, 0, 931), /* readExprType $expr */
reg::packed(951, 0, 437), /* readExprType $readScope */
reg::packed(348, 0, 943), /* readExprType $expr */
reg::packed(963, 0, 437), /* readExprType $readScope */
reg::packed(729, MAY_BE_BOOL), /* readExprType $native */
reg::packed(324, 0, 549), /* readExprType return */
reg::packed(431, 0, 437), /* buildClosureTypeFromClosureWalk $scope */
reg::packed(348, 0, 607), /* buildClosureTypeFromClosureWalk $expr */
reg::packed(879, MAY_BE_ARRAY), /* buildClosureTypeFromClosureWalk $parameters */
reg::packed(974, MAY_BE_BOOL), /* buildClosureTypeFromClosureWalk $isVariadic */
reg::packed(986, MAY_BE_BOOL), /* buildClosureTypeFromClosureWalk $isVariadic */
reg::packed(635, MAY_BE_ARRAY), /* buildClosureTypeFromClosureWalk $returnStatements */
reg::packed(652, MAY_BE_ARRAY), /* buildClosureTypeFromClosureWalk $yieldStatements */
reg::packed(668, MAY_BE_ARRAY), /* buildClosureTypeFromClosureWalk $executionEnds */
reg::packed(682, MAY_BE_ARRAY), /* buildClosureTypeFromClosureWalk $throwPoints */
reg::packed(694, MAY_BE_ARRAY), /* buildClosureTypeFromClosureWalk $impurePoints */
reg::packed(707, MAY_BE_ARRAY), /* buildClosureTypeFromClosureWalk $invalidateExpressions */
reg::packed(985, MAY_BE_NULL | MAY_BE_STRING, reg::NoString, false, false, 531), /* buildClosureTypeFromClosureWalk $cacheKey */
reg::packed(997, MAY_BE_NULL | MAY_BE_STRING, reg::NoString, false, false, 531), /* buildClosureTypeFromClosureWalk $cacheKey */
reg::packed(729, MAY_BE_BOOL, reg::NoString, false, false, 476), /* buildClosureTypeFromClosureWalk $native */
reg::packed(482, MAY_BE_NULL, 490, false, false, 531), /* buildClosureTypeFromClosureWalk $storage */
reg::packed(994, MAY_BE_BOOL, reg::NoString, false, false, 476), /* buildClosureTypeFromClosureWalk $contextFree */
reg::packed(890, MAY_BE_BOOL, reg::NoString, false, false, 476), /* buildClosureTypeFromClosureWalk $contextFree */
reg::packed(324, 0, 567), /* buildClosureTypeFromClosureWalk return */
reg::packed(431, 0, 437), /* resolveArrowFunctionReturnType $scope */
reg::packed(816, 0, 437), /* resolveArrowFunctionReturnType $arrowScope */
Expand All @@ -184,21 +185,21 @@ inline constexpr reg::PackedArg args[] = {
reg::packed(431, 0, 437), /* createClosureTypeFromCache $scope */
reg::packed(348, 0, 353), /* createClosureTypeFromCache $expr */
reg::packed(879, MAY_BE_ARRAY), /* createClosureTypeFromCache $parameters */
reg::packed(974, MAY_BE_BOOL), /* createClosureTypeFromCache $isVariadic */
reg::packed(986, MAY_BE_BOOL), /* createClosureTypeFromCache $isVariadic */
reg::packed(1173, MAY_BE_ARRAY), /* createClosureTypeFromCache $cachedClosureData */
reg::packed(994, MAY_BE_BOOL), /* createClosureTypeFromCache $contextFree */
reg::packed(890, MAY_BE_BOOL), /* createClosureTypeFromCache $contextFree */
reg::packed(324, 0, 567), /* createClosureTypeFromCache return */
reg::packed(431, 0, 437), /* assembleClosureType $scope */
reg::packed(348, 0, 353), /* assembleClosureType $expr */
reg::packed(879, MAY_BE_ARRAY), /* assembleClosureType $parameters */
reg::packed(974, MAY_BE_BOOL), /* assembleClosureType $isVariadic */
reg::packed(986, MAY_BE_BOOL), /* assembleClosureType $isVariadic */
reg::packed(1218, 0, 549), /* assembleClosureType $returnType */
reg::packed(682, MAY_BE_ARRAY), /* assembleClosureType $throwPoints */
reg::packed(694, MAY_BE_ARRAY), /* assembleClosureType $impurePoints */
reg::packed(707, MAY_BE_ARRAY), /* assembleClosureType $invalidateExpressions */
reg::packed(1229, MAY_BE_ARRAY), /* assembleClosureType $usedVariables */
reg::packed(985, MAY_BE_NULL | MAY_BE_STRING, reg::NoString, false, false, 531), /* assembleClosureType $cacheKey */
reg::packed(994, MAY_BE_BOOL, reg::NoString, false, false, 476), /* assembleClosureType $contextFree */
reg::packed(997, MAY_BE_NULL | MAY_BE_STRING, reg::NoString, false, false, 531), /* assembleClosureType $cacheKey */
reg::packed(890, MAY_BE_BOOL, reg::NoString, false, false, 476), /* assembleClosureType $contextFree */
reg::packed(324, 0, 567), /* assembleClosureType return */
};
using Sig = reg::Sig<strings, args>;
Expand All @@ -212,17 +213,17 @@ inline constexpr sigtab::Sig findCachedTypes = { { 415 /* findCachedTypes */, 1,
inline constexpr sigtab::Sig getClosureType = { { 592 /* getClosureType */, 2, 7, 5, 12, ZEND_ACC_PUBLIC } };
inline constexpr sigtab::Sig buildClosureTypeForClosure = { { 755 /* buildClosureTypeForClosure */, 8, 13, 12, 25, ZEND_ACC_PUBLIC } };
inline constexpr sigtab::Sig buildClosureTypeForArrowFunction = { { 827 /* buildClosureTypeForArrowFunction */, 6, 26, 10, 36, ZEND_ACC_PUBLIC } };
inline constexpr sigtab::Sig closureContextCacheKey = { { 890 /* closureContextCacheKey */, 4, 37, 4, 41, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig freeVariableRoots = { { 913 /* freeVariableRoots */, 1, 42, 1, 43, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig readExprType = { { 961 /* readExprType */, 4, 44, 4, 48, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig buildClosureTypeFromClosureWalk = { { 1006 /* buildClosureTypeFromClosureWalk */, 10, 49, 14, 63, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig resolveArrowFunctionReturnType = { { 1038 /* resolveArrowFunctionReturnType */, 3, 64, 5, 69, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig deriveOnlyNeverExecutionEnds = { { 1069 /* deriveOnlyNeverExecutionEnds */, 1, 70, 1, 71, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig buildDeclaredParameters = { { 1098 /* buildDeclaredParameters */, 2, 72, 2, 74, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig getDeclaredClosureType = { { 1122 /* getDeclaredClosureType */, 2, 75, 2, 77, ZEND_ACC_PUBLIC } };
inline constexpr sigtab::Sig buildParametersAndAcceptors = { { 1145 /* buildParametersAndAcceptors */, 2, 78, 5, 83, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig createClosureTypeFromCache = { { 1191 /* createClosureTypeFromCache */, 6, 84, 6, 90, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig assembleClosureType = { { 1243 /* assembleClosureType */, 9, 91, 11, 102, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig closureContextCacheKey = { { 902 /* closureContextCacheKey */, 5, 37, 5, 42, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig freeVariableRoots = { { 925 /* freeVariableRoots */, 1, 43, 1, 44, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig readExprType = { { 973 /* readExprType */, 4, 45, 4, 49, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig buildClosureTypeFromClosureWalk = { { 1006 /* buildClosureTypeFromClosureWalk */, 10, 50, 14, 64, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig resolveArrowFunctionReturnType = { { 1038 /* resolveArrowFunctionReturnType */, 3, 65, 5, 70, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig deriveOnlyNeverExecutionEnds = { { 1069 /* deriveOnlyNeverExecutionEnds */, 1, 71, 1, 72, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig buildDeclaredParameters = { { 1098 /* buildDeclaredParameters */, 2, 73, 2, 75, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig getDeclaredClosureType = { { 1122 /* getDeclaredClosureType */, 2, 76, 2, 78, ZEND_ACC_PUBLIC } };
inline constexpr sigtab::Sig buildParametersAndAcceptors = { { 1145 /* buildParametersAndAcceptors */, 2, 79, 5, 84, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig createClosureTypeFromCache = { { 1191 /* createClosureTypeFromCache */, 6, 85, 6, 91, ZEND_ACC_PRIVATE } };
inline constexpr sigtab::Sig assembleClosureType = { { 1243 /* assembleClosureType */, 9, 92, 11, 103, ZEND_ACC_PRIVATE } };
} // namespace sig

} // namespace ptdecl::ClosureTypeResolver
Expand Down
Loading