Single-pass expression analysis groundwork - answer type questions from ExpressionResults - #5857
Open
ondrejmirtes wants to merge 91 commits into
Open
Single-pass expression analysis groundwork - answer type questions from ExpressionResults#5857ondrejmirtes wants to merge 91 commits into
ondrejmirtes wants to merge 91 commits into
Conversation
ondrejmirtes
force-pushed
the
resolve-type-rewrite-2
branch
3 times, most recently
from
June 12, 2026 17:15
ebb19a0 to
457689b
Compare
staabm
reviewed
Jun 12, 2026
| return $this->withFlavor(false); | ||
| } | ||
|
|
||
| private function withFlavor(bool $fiber): self |
Contributor
There was a problem hiding this comment.
should this read withFiber?
ondrejmirtes
force-pushed
the
resolve-type-rewrite-2
branch
2 times, most recently
from
June 19, 2026 11:44
eb31077 to
59cbf22
Compare
ondrejmirtes
force-pushed
the
resolve-type-rewrite-2
branch
from
June 20, 2026 11:56
59cbf22 to
125cf22
Compare
ondrejmirtes
force-pushed
the
resolve-type-rewrite-2
branch
4 times, most recently
from
July 6, 2026 22:20
f98892f to
4455baa
Compare
ondrejmirtes
force-pushed
the
resolve-type-rewrite-2
branch
from
July 16, 2026 14:56
61fe06e to
e38aadd
Compare
ondrejmirtes
referenced
this pull request
Jul 23, 2026
Every property fetch / method call resolves its type by walking down to the chain root to detect a nullsafe operator (NullsafeShortCircuitingHelper), costing O(N²) walk steps per chain of depth N — with or without an actual nullsafe operator in the chain. Deep loop-wrapped plain chains make that walk dominate: 3.71s -> 3.14s wall (-15%), -18% user CPU from the recursion-to-loop rewrite. The real-world counterpart is Symfony TreeBuilder fluent chains (300+ calls in one statement) in Sylius bundle Configuration classes, which dropped up to 23% per file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016szvNF5RXhACdfMQNc6DVL
ondrejmirtes
force-pushed
the
resolve-type-rewrite-2
branch
12 times, most recently
from
July 28, 2026 17:31
fb22d34 to
84b1614
Compare
The branch's own fixpoint-replay commits were dropped during the rebase over the merged extraction chunks (#6249); this restores the replay in upstream's final form - raw-recorded pairs wrapped at replay time, the RecordingNodeCallback short-circuit in callNodeCallback - woven into the branch's handler shapes (ambient storage push around pass walks, the deferred While_ statement callback, the on-demand falsey cond re-pricing). replayRecording() binds the storage through the scope's push/pop like every other branch-side ambient binding. The consume mechanism (#6251, closed unmerged) is gone entirely: convergence behavior on this branch is now byte-for-byte upstream's algorithm. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnwgpaeUXRkgSDyg95tfK8
The rebase over the merged gathering cleanup (#6258) kept the branch's wrapper-based gatherer sites while the base deleted GatheringNodeCallback; this converts them to the upstream frame mechanism - pushNodeGatherer()/popNodeGatherer() on NodeScopeResolver, fed the raw walk scope by callNodeCallback() and per replayed pair by replayRecording() - woven into the branch's shapes: the per-body storages of method/function bodies, the ambient storage pushes, and the scope-bound replayRecording() signature. Gatherer bodies are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnwgpaeUXRkgSDyg95tfK8
…and remembered call values
A census showed 38% of truthy and 76% of falsey overrides are never read; holding the source ExpressionResult instead of the eagerly derived scope defers the applySpecifiedTypes to first use. Measured perf-neutral on self-analysis-scale corpora (the saved derivations are cheap) - the change stands on not doing unread work and not pinning derived scopes.
processArgs() captures an ExpressionResult for every argument it walks, so the readers of ArgsResult no longer decide per call site whether one might be missing: * $argResults is a required constructor parameter. * The nullable accessor is renamed to findArgResult() and documented as a membership query - "is this expression one of the call's arguments?" - for the two call sites that ask about arbitrary expressions (the callee name and synthetic nodes in FuncCallHandler's getType bridge, the narrowed subexpressions of a SpecifiedTypes entry in ImpossibleCheckTypeHelper). Everything holding an actual argument uses requireArgResult(). * DynamicReturnTypeStoragePrimer no longer takes an argument list and looks each argument up: it primes the captured results themselves, keyed by the very expression each was processed for. The list it used to receive was the normalized one, which contains arguments that were never processed - the default-value arguments ArgumentsNormalizer synthesizes for omitted optional parameters, and arguments an invalid call's normalization drops (a duplicate named argument overwriting a positional one). * The array_key_first()/array_key_last()/array_find_key() narrowing reads the normalized argument, which is the one processArgs() walked. * processArgs()'s own side-effects branch reads the captured result through a require, instead of falling back to a storage lookup that never fired (0 misses in 4790 lookups analysing src/, 837 analysing vendor/).
CalledMethodProcessor decided whether an execution end evaluates to an explicit never by reading the statement's expression through the may-or-may-not-be-stored lookup. The node now carries the very ExpressionResult the expression was processed into, so the read is the result's own type on the end scope. The result is null exactly when there is no processed expression behind the end: the statement is not an expression statement, or it is the synthetic statement wrapping a closure with an empty body. For every real expression statement the result is there - 9050 ends across the test suite and src/, none missing.
The either-branch union recovery and the disjunction-holder projection pin their target as tracked (hasExpressionType() yes) before reading its type, so readScopeStateOrSyntheticType()'s on-demand walk was unreachable there - the call sites read as undecided about whether the expression was analysed while in fact they had just decided it. They now read through requireScopeStateType(), which answers from the scope state and throws instead of silently walking. The targets themselves have no ExpressionResult to read: the projection discovers them from the conditional holders registered on the applying scope, not from a walk of its own.
The boolean-decomposition recipe read each condition subject's current type by asking the applying scope for its state. It now reads it through the result of the operand walk that produced the narrowing, captured with the rest of the entry: the result answers from the applying scope's state where that scope owns the expression, and consults the expression-type-resolver extensions the plain state read skipped. A subject no walk produced keeps the state read - a narrowing extension is free to specify a type for an expression the source never evaluated on its own, which is 114 of 30148 condition entries analysing src/. Error sets over src/ and tests/PHPStan/Analyser are identical before and after, and the nsrt type assertions are unchanged.
The single-pass rewrite moved the void->null projection to the value-read
boundary of every ExpressionResult, which widened it: a `void`-typed
parameter, a call through a callable value (`$f()`), and the native-type
flavour of every call all started evaluating to `null`. That silently
changed 26 test expectations - `assertType('void', ...)`, "expects int,
void given", "not subtype of native type void" - all of which are restored
here.
The projection is back to where it was before the rewrite: the phpdoc-type
flavour of a call to a resolved function or method. Both call handlers
short-circuited the native flavour before reaching the transformer, and
only the named-function and method paths ever reached it, so a dynamic
callee and every non-call expression kept void. Rules that flag a void
value being used read that type, so widening the projection had made them
silent.
The raw type an ExpressionResult carries still keeps void, which is what
getKeepVoidType() answers from.
The native side of the guard is all here - PharForkGuard.cpp, the Runtime::enablePharForkGuard() registration in main.cpp - but the PHP-side call that arms it was lost, so nothing ever called it: a previous rebase resolved a conflict in this file (the expected-version line) by taking the branch's whole copy, which predated the guard, and the deletion rode into the bump commit. Without it a phar running forked workers shares one archive fd whose seek cursor the children race on.
ondrejmirtes
force-pushed
the
resolve-type-rewrite-2
branch
from
August 30, 2026 19:39
a57f49d to
2f49907
Compare
MutatingScope::getType() resolves the late-resolvable types of whatever it returns. The three reads that go to the tracked expression holder directly, to avoid re-entering that method, skipped the resolution - so an expression whose holder still carries a conditional type was answered with the raw conditional. A narrowed one made it visible: intersecting the asserted type into `($success is true ? T : ErrorPayload)` gave `($success is true ? MessagePayload : ErrorPayload)&MessagePayload` where the conditional resolves to `MessagePayload|ErrorPayload` and the whole intersection to `MessagePayload`.
simple-downgrade cannot rewrite a named argument whose receiver it does not resolve - a private method on $this, and the two container-chain calls - so these three reached the 7.4 lint job as named arguments and failed to parse. The parameter before $storage is a bool defaulting to false in all three signatures; spelling it out keeps the call positional.
simple-downgrade cannot rewrite `?->`, and build/PHPStan is part of the downgraded tree, so the 7.4 lint job could not parse the rule. Reading the class reflection into a variable and checking it explicitly says the same thing - the name it now holds cannot collide with the loop below, hence $currentClassReflection. The baseline is regenerated rather than edited: two of its entries sat in positions a regeneration does not produce, which is what the Generate baseline job compares against. Same 306 entries, 608 errors.
Its fixture is PHP 8.0 code (the file carries `// lint >= 8.0`), and the expectation it asserts does not hold on a 7.4 runtime - the only failure in the 7.4 leg, 1 of 16770.
findScopeStateType() read a variable by name, and otherwise read the tracked expression - but it excluded every Variable from that second branch, so a variable whose name is an expression ($$name) matched neither and the method answered null. Its callers had already pinned the expression as tracked, so requireScopeStateType() turned that into a ShouldNotHappenException: Internal error: PhpParser\Node\Expr\Variable on line 426 is not tracked on the scope it was pinned as tracked on. which crashed the analysis of briannesbitt/Carbon. Only the by-name read has to skip such a variable; the scope tracks it like any other expression.
getKeepVoidType() answered from the result's own raw type, which skips the
holder tracked for the expression - so a match arm body narrowed by that
arm's own condition was read at its declared type. A property fetch lost
the narrowing (a local variable did not, its own type reads the scope):
match (true) {
$this->shipment instanceof Shipment => $this->shipment,
$this->shipment instanceof ReturnShipment => throw ...,
}
gave BaseShipment instead of Shipment, so the match's return type no longer
satisfied the declared Shipment. The raw type is still the answer whenever
it carries void - that is the whole point of this read - but with no void to
keep it is an ordinary value read and goes through getType().
A multi-condition arm subtracts its conditions from the subject through a
synthetic in_array() whose haystack was built from the arm's own condition
nodes. The falsey narrowing of that call is evaluated on the scope carrying
each condition's own falsey narrowing, and `$subject === $cond` specifies
both sides - so on that scope a condition node is itself narrowed to never.
Re-pricing the haystack there collapsed every condition the subject had been
narrowed down to, and the arm stopped subtracting them:
match ($this->get()) { // ?E
E::A, E::B => true,
null, E::C, E::D => false, // haystack: array{null, E::C, *NEVER*}
};
reported E::D as unhandled. The haystack now carries the conditions' walked
types, which are what it always meant, and cannot be re-narrowed by the
scope it is read on. Pricing it on the arm's entry scope instead was the
other candidate and is wrong - it over-subtracts (bug-10128).
An execution end is collected from wherever execution ended, which can be a nested walk whose results never reach the storage frame the ExecutionEndNode is built in. Requiring the expression's result there crashed the analysis of drupal/drupal: Internal error: PhpParser\Node\Expr\Assign on line 126 has no stored ExpressionResult - it was not processed by processExprNode(). $stmt->expr of an expression statement is the only Assign any readStoredResult() caller passes, and carrying the result was an optimisation over the previous may-or-may-not-be-stored read - so its absence is not an invariant violation, just an end whose expression this frame cannot answer for.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Groundwork for the "new world" where an expression is traversed once: after
processExpr, itsExpressionResultknows the before/after scopes, the type (typeCallback) and the narrowing (specifyTypesCallback), composed from child results instead of re-walking subtrees. Handlers then stop implementingTypeResolvingExprHandler; the old entry points (MutatingScope::resolveType, theTypeSpecifierdispatcher) are guarded behindNewWorld::disableOldWorld()and get mass-deleted in PHPStan 3.0.What's on the branch, bottom up:
ExpressionResultFactory: old-world type resolution entry points throw whenNewWorld::disableOldWorld()is flipped (the migration meter); allExpressionResultconstruction goes through a generated factory.ExpressionResultcarriesbeforeScope,expr,typeCallback,specifyTypesCallbackand is stored per node inExpressionResultStorage(layered O(1)duplicate()), replacing the stored before-Scope.ExprHandler/TypeResolvingExprHandlersplit:resolveType/specifyTypesmove to the sub-interface so handlers can shed them one by one.ExpressionResultStorageStack: old-world consumers (TypeSpecifier dispatcher, extensions, rules below PHP 8.1, unconverted handlers'resolveType) keep working for converted handlers' nodes. Every scope shares the stack created by its internal scope factory;NodeScopeResolverpushes the storage of the analysis in progress throughMutatingScope::pushExpressionResultStorage()(always popped infinally, throwing on imbalance), andMutatingScopeanswers from the stored result - or processes a synthetic node on demand. Scopes never reference a storage directly, so nothing pins the result graph with the cycle collector disabled inbin/phpstan. Also addsMutatingScope::applySpecifiedTypes-filterBySpecifiedTypeswithoutScope::getType().ScalarHandlerandArrayHandlerno longer implementTypeResolvingExprHandler. The array migration is a precision win the old world cannot reach: each item type is captured at its own evaluation point, so[$b = 1, $b + 1, $c = $b, $c + 2, $c++, $c]infersarray{1, 2, 1, 3, 1, 2}.Verified: full test suite green,
make phpstanclean, and analysis memory back at baseline (no leak from the result graph despitegc_disable()).Closes phpstan/phpstan#13944
Closes phpstan/phpstan#12207
Closes phpstan/phpstan#7155
Closes phpstan/phpstan#14396
Closes phpstan/phpstan#11953
Closes phpstan/phpstan#12780
🤖 Generated with Claude Code
Closes phpstan/phpstan#14999
Closes phpstan/phpstan#13334
Closes phpstan/phpstan#15004