Skip to content

Single-pass expression analysis groundwork - answer type questions from ExpressionResults - #5857

Open
ondrejmirtes wants to merge 398 commits into
2.2.xfrom
resolve-type-rewrite-2
Open

Single-pass expression analysis groundwork - answer type questions from ExpressionResults#5857
ondrejmirtes wants to merge 398 commits into
2.2.xfrom
resolve-type-rewrite-2

Conversation

@ondrejmirtes

@ondrejmirtes ondrejmirtes commented Jun 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Groundwork for the "new world" where an expression is traversed once: after processExpr, its ExpressionResult knows the before/after scopes, the type (typeCallback) and the narrowing (specifyTypesCallback), composed from child results instead of re-walking subtrees. Handlers then stop implementing TypeResolvingExprHandler; the old entry points (MutatingScope::resolveType, the TypeSpecifier dispatcher) are guarded behind NewWorld::disableOldWorld() and get mass-deleted in PHPStan 3.0.

What's on the branch, bottom up:

  • Guards + ExpressionResultFactory: old-world type resolution entry points throw when NewWorld::disableOldWorld() is flipped (the migration meter); all ExpressionResult construction goes through a generated factory.
  • ExpressionResult carries beforeScope, expr, typeCallback, specifyTypesCallback and is stored per node in ExpressionResultStorage (layered O(1) duplicate()), replacing the stored before-Scope.
  • ExprHandler / TypeResolvingExprHandler split: resolveType/specifyTypes move 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; NodeScopeResolver pushes the storage of the analysis in progress through MutatingScope::pushExpressionResultStorage() (always popped in finally, throwing on imbalance), and MutatingScope answers 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 in bin/phpstan. Also adds MutatingScope::applySpecifiedTypes - filterBySpecifiedTypes without Scope::getType().
  • First two migrations: ScalarHandler and ArrayHandler no longer implement TypeResolvingExprHandler. 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] infers array{1, 2, 1, 3, 1, 2}.

Verified: full test suite green, make phpstan clean, and analysis memory back at baseline (no leak from the result graph despite gc_disable()).

Closes phpstan/phpstan#13944
Closes phpstan/phpstan#12207
Closes phpstan/phpstan#7155

Closes phpstan/phpstan#2032

Closes phpstan/phpstan#10786

Closes phpstan/phpstan#13253
Closes phpstan/phpstan#14396
Closes phpstan/phpstan#11953
Closes phpstan/phpstan#13802
Closes phpstan/phpstan#13789
Closes phpstan/phpstan#12780
Closes phpstan/phpstan#14914
Closes phpstan/phpstan#14908

🤖 Generated with Claude Code

@ondrejmirtes ondrejmirtes mentioned this pull request Jun 12, 2026
Closed
@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch 3 times, most recently from ebb19a0 to 457689b Compare June 12, 2026 17:15
staabm
staabm reviewed Jun 12, 2026
View reviewed changes
Comment thread src/Analyser/DirectInternalScopeFactory.php
return $this->withFlavor(false);
}

private function withFlavor(bool $fiber): self

@staabm staabm Jun 12, 2026

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.

should this read withFiber?

@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch 2 times, most recently from eb31077 to 59cbf22 Compare June 19, 2026 11:44
@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch from 59cbf22 to 125cf22 Compare June 20, 2026 11:56
@ondrejmirtes ondrejmirtes mentioned this pull request Jun 25, 2026
Closed
@SanderMuller SanderMuller mentioned this pull request Jun 25, 2026
Merged
@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch 4 times, most recently from f98892f to 4455baa Compare July 6, 2026 22:20
@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch from 61fe06e to e38aadd Compare July 16, 2026 14:56
@marcella-galambos-framna marcella-galambos-framna mentioned this pull request Jul 16, 2026
Open
ondrejmirtes referenced this pull request Jul 23, 2026
@ondrejmirtes @claude
Add bench for the expression chain walk speedup from #6084
cdc6ca2 
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
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch 12 times, most recently from fb22d34 to 84b1614 Compare July 28, 2026 17:31
ondrejmirtes and others added 25 commits July 28, 2026 20:26
@ondrejmirtes
Create the function call's impure point after its arguments are proce…
dd94fe7 
…ssed

The side-effect flip parameters (print_r's $return, var_export's, ...)
read an argument's type to decide whether the call has side effects. At
the pre-args position that argument is not walked yet; after processArgs()
its result is stored and the read is answered from it.
@ondrejmirtes @claude
Restore the Array_ import dropped when replaying onto the ScopeOps ex…
4f28cc3 
…traction

Upstream removed the then-unused import from MutatingScope; the branch's
applySpecifiedTypes still guards array literals with instanceof Array_,
which silently matched the nonexistent PHPStan\Analyser\Array_ and let
array literals into the scope's tracked expressions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Update test expectations after the rebase onto 2.2.x
886f53c 
array-merge2: restore the key order upstream moved when getSortedTypes()
stopped sorting in place. preg_replace_callback: the flags-refined closure
parameter makes the arrow body's return type precise, so returning $m[0]
under PREG_OFFSET_CAPTURE is correctly reported as returning an array where
a string is required.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Remove the superseded ScopeOps invalidation helpers after the rebase
8603959 
The branch answers invalidation checks from the per-holder index of
contained node keys in MutatingScope, so the extracted
invalidateExpressionEntries/shouldInvalidateExpression/
containsExpressionToInvalidate/buildTypeSpecifications helpers have no
callers left. Also drop the baseline entries for the unused-use closure
errors that upstream's include/require handling (#6056) resolved.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Answer invalidation checks from ScopeOps again
1622bd9 
The per-holder contained-node-keys invalidation lived as private
MutatingScope methods, out of reach of the turbo twin — with the extension
active, every one of the ~2.3M shouldInvalidateExpression() calls of a
self-analysis run paid a userland frame that 2.2.x answers natively.
Extracted back into ScopeOps statics (the shape the pre-index code had),
called unconditionally; '$this' invalidations keep the finder on the
asking scope.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Sync the turbo native twins with the single-pass engine's shadowed cl…
e07981a 
…asses

ExpressionResultStorage stores ExpressionResults in id-keyed tables with an
O(1) fallback-chain duplicate() and gains mergeResults(), mirroring the
twin's SplObjectStorage model. ExpressionTypeHolder gains the lazy
containedNodeKeys subtree index. ScopeOps: mergeVariableHolders reports
differing keys through the new by-ref parameter,
createConditionalExpressions iterates only those keys, nodeKey drops the
retired keep-void suffix, buildTypeSpecifications goes away with its PHP
twin, and shouldInvalidateExpression/invalidateExpressionEntries answer
from the holder's contained-node-keys index (built natively on first use
and cached in the holder's slot) instead of the subtree scan - measured
about -1.5% make phpstan user CPU against the 2.2.x-with-turbo control.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes
Pin turbo extension version to cac4327
5cc1a86
@ondrejmirtes @claude
Resolve the call return type once per call
49c7e55 
The call handlers' throw-point leg resolved the return type eagerly and
directly, outside the result's per-flavour memo - the first later type
read ran resolveReturnType() again (2.78 resolutions per method call on
the self-analysis corpus; dynamic return type extensions such as sprintf's
ran twice per call). The eager leg now reads through the stored
preliminary result, and the memoized value seeds the final result's
raw own-type slot via the new ExpressionResult $resolvedType parameter.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Complete the stored preliminary call result in place instead of overw…
abbbe23 
…riting it

The four call handlers (function, method, static, new) stored a preliminary
result before the throw-point leg and then built a second ExpressionResult
with the resolved scope and effects, losing every memo the preliminary had
accumulated - including the eagerly resolved return type. finalize() turns
the stored preliminary into the final result in place, carrying the
own-type and narrowing memos and dropping only the truthy/falsey scopes
derived from the preliminary scope. The $resolvedType factory seed from
the previous commit is superseded and removed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Walk closure by-ref convergence passes in deep statement context
7072851 
The closure by-ref fixpoint walked its intermediate passes with a top-level
statement context, so every inner loop re-converged from scratch on every
pass - convergence within convergence, which the loop handlers themselves
avoid by walking their passes with enterDeep(). A recursive by-ref closure
with nested loops re-created the innermost expressions' results up to 78
times; deep-context passes bring it in line with the loop handlers'
discipline, and the final top-level walk keeps full precision. Inherited
from mainline (2.2.x has the same top-level pass).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Drop unnecessary ?? null on always-set parameter types
d3c1799 
Rebase adaptation: the mainline restructuring of processArgs assigns
$parameterType on every path before the closure/arrow-function branches,
which the new nullCoalesce.unnecessary check now proves.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X5cXTK6VFMziQHez43qhea
@ondrejmirtes @claude
Drop the buildTypeSpecifications smoke block for the removed helper
eef3194 
Rebase adaptation: ScopeOps::buildTypeSpecifications() no longer exists on
this branch (superseded by the per-holder index), so the differential smoke
block resurrected from the mainline side tested a method with no PHP twin.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X5cXTK6VFMziQHez43qhea
@ondrejmirtes @claude
Fix use-statement order left by the rebase auto-merge
11a6f0e 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01X5cXTK6VFMziQHez43qhea
@ondrejmirtes @claude
Price an offset read on never as ErrorType in the narrowing-base reader
91ed434 
resolveScopeStateType() derives the base a narrowing is intersected into
from tracked state instead of re-walking. Its ArrayDimFetch case called
NeverType::getOffsetValueType() directly, yielding never - but genuine
pricing of an offset read on never yields ErrorType (a benevolent mixed).
A narrowing applied in a dead branch (is_object($x[0]) with $x already
never) intersected object against never and lost the narrowed type,
silencing every rule that reads it - e.g. the non-ignorable 'Accessing
::class constant on an expression is supported only on PHP 8.0 and later'
disappeared at phpVersion < 8.0 because ClassConstantRule bailed on the
ErrorType it derived from the never.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes @claude
Avoid the nullsafe operator - simple-downgrader passes it through
a1e3289 
The downgrade step silently leaves ?-> untouched, so the transformed
sources kept PHP 8.0 syntax and would fail to parse on PHP 7.4. Rewritten
as explicit null checks on a hoisted receiver; the first-class-callable
case was already gone (the extraction of shouldInvalidateExpression into
ScopeOps replaced $this->getNodeKey(...) with an arrow function). Verified
by running simple-downgrade 7.4 on the tree: 17 nullsafe remnants in the
output before, zero after.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VktvX3FRdnSsL66xGtiUh8
@ondrejmirtes
Use ExtensionsCollection in ExpressionResult and tag PerFileAnalysisR…
698e5e1 
…esettable via attribute
@ondrejmirtes
Construct name checks from extensions collections in ClassConstantPhp…
6777b6f 
…74RuleTest
@ondrejmirtes
Bump expected turbo version
78d3480
@ondrejmirtes @claude
Restore the default-value gate in the isset verdict
0c28cc4 
2.2.x now consults hasDefaultValue() in MutatingScope::issetCheck, so
the gate-parity alignment that removed nativeHasDefaultValue() from the
isset verdict points the other way again: a property with a default
value is always set, so isset() on it is true. Also absorb the upstream
isset-property-default-value fixture: the no-default coalesce narrows
to int here because the === null reads in the guard prove both
properties are initialized.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b
@ondrejmirtes @claude
Complete the callable-union parameter merge in createCallableParameters
1a1dcfd 
The rebase kept the stale pre-c2c1497254 shape of the acceptor merge
loop: a longer acceptor's raw parameters (possibly ExtendedDummyParameter)
were stored and the next acceptor called union() on them. Port the
upstream form - wrap every acceptor's parameters in
NativeParameterReflection up front and mark length-mismatched positions
optional.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b
@ondrejmirtes
Bump expected turbo version
ea67c11
@ondrejmirtes @claude
Absorb the upstream TypeSpecifierTest additions after the rebase
a061278 
Drop the duplicated alternative-entry harness loop (both worlds added
one) and update the new rows to this branch's richer results: the
composed nullsafe narrowing also constrains the call expression itself,
and the disjunction falsey merge keeps its sure-first description
order.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b
@ondrejmirtes @claude
Drop the upstream toSureTypes copy that flowed into the holder helper
274f320 
This branch's eager fold lives on DefaultNarrowingHelper; the copy the
rebase carried into ConditionalExpressionHolderHelper was dead and
referenced imports this file does not have.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b
@ondrejmirtes
Bump expected turbo version
8495201
@ondrejmirtes @claude
Drop the upstream duplicate of the differingKeys smoke block
a834c78 
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019wqGgaD7iqL44t1KgpJS7b
@ondrejmirtes
ondrejmirtes force-pushed the resolve-type-rewrite-2 branch from dc0e107 to a834c78 Compare July 28, 2026 18:27
ondrejmirtes and others added 4 commits July 28, 2026 23:14
@ondrejmirtes @claude
Split processAssignVar into prepareTarget and applyWrite
e685f86 
processAssignVar owned the whole assignment timeline - target sub-expression
walk, value evaluation, write - so callers had to inject the value evaluation
as a processExprCallback closure invoked once per target-shape branch, and
smuggle its products back out through by-ref captures.

The invocation point is the same in every branch: after the target's
sub-expressions (root, dimensions, receiver, dynamic name) are walked, before
the write - PHP's own evaluation order. Splitting at that boundary gives the
caller the timeline instead: prepareTarget() walks the target and captures
the per-shape intermediates in a PreparedAssignTarget, the caller processes
the assigned value inline on its scope, and applyWrite() performs the write
and its bookkeeping consuming the value result.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
@ondrejmirtes @claude
Produce the assignment target's read result inside prepareTarget
c789b9b 
`$lvalue OP= ...` reads the old value of the target, so AssignOpHandler
re-processed the target expression with a noop callback before the assign
walk - a full second walk for the ??= isset-semantics read and for plain
op= Variable/property targets.

prepareTarget() now takes an AssignTargetWalkMode instead of a bare bool:
the mode says whether target sub-expressions are walked inside
enterExpressionAssign() scopes and whether the walk also prices the whole
target as a read (plain, or with isset() semantics for ??= - carrying the
isset descriptor and the captured chain results on the PreparedAssignTarget).
The property-target pre-read is deleted outright: the walk already stores
the target's read via processExprNodeConsumingStored, composed from the
receiver's and name's stored results, so the extra walk was redundant.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
@ondrejmirtes @claude
Pass the assigned value's result into applyWrite instead of pre-stori…
a7280a3 
…ng it

applyWrite() read the value to assign back out of the expression-result
storage, so every caller had to pre-store a result for the assigned
expression first: AssignOpHandler and PreInc/PreDecHandler through the
storeProvisionalExpressionResult() crutch (a store invisible to outside
askers, existing only for this one synchronous read), processVirtualAssign()
through a plain store of the passed or fabricated result.

The caller now hands the assigned expression's result to applyWrite()
directly; the reads (value type in both flavours, truthy/falsey narrowing,
the sentinel comparisons, and the conditional-holder refinement in
currentTypeForConditionalHolder - whose storage miss would re-enter the
still-in-flight assignment and recurse) consume the threaded result, falling
back to stored results or on-demand pricing only where no result exists
(virtual assigns of non-type-carrying expressions, nested assign chains).
The provisional-store machinery is deleted; AssignOpHandler builds its
callbacks after the value evaluation, so the by-ref capture goes away too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
@ondrejmirtes @claude
Compose the ??= read from the walked target instead of a second walk
d0df9ff 
The isset-semantics read for `$lvalue ??= ...` was a full extra walk of the
target chain before the write walk - the chain's sub-expressions were
processed twice.

Each target branch now produces the read after its own children are walked,
via processExprNodeConsumingStored, so the read is composed from the stored
child results instead of re-walking them. Ordering keeps the storage
observable behavior: for an ArrayDimFetch target the write-flavoured
per-dimension results are deferred until after the composition (parked rule
asks resume with the read-flavoured results, as they did off the pre-read's
stores, and the write-flavoured results replace them in storage as before);
for property targets the existing parked-asker composition consumes and
re-anchors the read's store exactly as it consumed the pre-read's. The
fallback branch composes too - synthetic ??= nodes (e.g.
InvalidBinaryOperationRule's TypeExpr-operand clones) reach it when priced
on demand.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DaBZjgksga4c5s6Q9FniY7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment