Skip to content

Resolve inferred template arguments from the body's usages - #6332

Merged
ondrejmirtes merged 28 commits into
2.3.xfrom
bidirectional-type-narrowing-2
Sep 7, 2026
Merged

Resolve inferred template arguments from the body's usages#6332
ondrejmirtes merged 28 commits into
2.3.xfrom
bidirectional-type-narrowing-2

Conversation

@ondrejmirtes

@ondrejmirtes ondrejmirtes commented Sep 1, 2026

Copy link
Copy Markdown
Member

Prototype of phpstan/phpstan#6732 as proposed in phpstan/phpstan#6732 (comment) — template arguments inferred from new/generic-call arguments are unresolved until the body's usages resolve them. Bleeding edge only (featureToggles.unresolvedTemplateArguments); the last commit is a TMP commit enabling it for everyone to see the effect across the CI pipeline — to be reverted before merging.

What was done

  1. UnresolvedTemplateArgumentType marker (CompoundType, opaque equals per site + template name, delegates to initial/default/bound) + unit test.
  2. TemplateArgumentFrame (per-body, on the scope-shared ExpressionResultStorageStack, never a service) + TemplateArgumentObserver (sends through ancestor mapping with an explicit effective-variance table; method-call lower bounds) + unit tests.
  3. Two-pass driver in NodeScopeResolver::doProcessStmtNodes: pass 1 records (Node, Scope) pairs with gatherers suspended; pass 2 re-walks only statements that own a site, mention a differing variable, or sit under labels — everything else replays with its recorded holder delta; early exit on convergence.
  4. Producers for new (no more generalizeInferredTemplateType under the toggle), all send hooks (property/static/@var/argument/return/arrow-fn), bare-marker containment, ClosureTypeResolver cache key. Tests: nsrt/bug-6732.php + Properties/Methods/Functions rule fixtures.
  5. Call-site producer: ResolvedFunctionVariant::getReturnTypeWithUnresolvedTemplateArguments(), TemplateArgumentFrame::returnTypeOfCall() at every engine call site; synthetic offsetGet/__toString calls tagged as non-sites. Extensions keep reading getReturnType(), which still generalizes.
  6. PHPSTAN_TEMPLATE_CLAMP_STATS counters and a driver test proving the "10 unrelated lines are replayed, 3 re-walked" shape.

Semantics: no-send case = precise union of the initial type and method-call lower bounds (A<''|1|2>); call results with function-level templates inside generic return types are clamped too; new Foo() with nothing inferred → never; first accepting invariant send wins (no widening), covariant targets never clamp a known initial type, contravariant targets are lower bounds. With the toggle off (and for top-level code) the legacy path is untouched.

Verification: all suites, make phpstan, PHPSTAN_GUARD_NW=1 (self-analysis + nsrt), and the suites again with the turbo build loaded — green; zero unresolved( in any output. The issue's playground sample now reports the real hole (return $c as Collection<int> from a Collection<string> function); the body-local WeakMap example from the thread loses its false positive.

Numbers

ABBA on the same build, toggle flipped, --debug, quiet machine:

target n pairs user CPU RSS
src/Type (2 sites in 8,399 bodies) 8 −0.35 % (t −0.86) −0.16 %
nsrt corpus (146 sites in 5,868 bodies) 3 −0.97 % (t −1.53) +0.24 %

Both indistinguishable from zero; the site-heavy corpus is no worse than the site-free one. Counters on full src: 105 of 14,939 bodies have sites (0.7 %), 595 statements replayed vs 307 re-walked, 21 early exits.

On/off error diff over the nsrt corpus: +3 true positives (bug-6732), −2 false positives (bug-13332), −1 detection (bug-6993, the lower-bound union widening — by design), and self-out's a<123> no longer satisfying $this is a<int> (an unreachable-code report appears). Expectation changes to review: discussion-6993 detection lost; LruCacheTest needed @var LruCache<string>; bug-5372 / bug-4590 / generics-infer-collection errors gone by design; bug-10254 Option<3>.

What's left

  • Not benchmarked: full src and a real downstream generics-heavy project (slevomat / ShipMonk via the phar) — the two measured targets bound the cost but a downstream error-set classification is still owed.
  • Follow-ups not in this cut: parameter default values (__construct(WeakMap $m = new WeakMap()) still generalizes — the @param type is the obvious send target), file top-level statements, closure/arrow arguments as sends, dim-fetch property targets, generator @return sends; possibly deleting NewAssignedToPropertyVisitor now that pass 2 is cheap.
  • Decisions: the flagged expectation changes above, and whether "first invariant send wins, later sends error" vs. the union fallback is the long-term semantics.
  • Housekeeping: the PHPSTAN_TEMPLATE_ARGUMENTS_DEBUG echo could be dropped; the TMP commit must be reverted.
  • Two upstream bugs found on the way (on 2.2.x too): declare(strict_types=1) mid-method makes MissingReturnRule throw Internal error; a parse error in any file suppresses every other file's errors in the same run.

Regression tests for issues this closes

Playground samples verbatim; each is green with the feature on and red with the toggle off. Not fixed by this branch (checked): #8441 (new Collection() whose T comes from a parameter default = null — a default-inferred argument still counts as a known initial), #13325 (RegexIterator stub, third argument resolves to Traversable<mixed, mixed>). #8031: the original sample was already clean on the base; its later playground (940f82a4) is fixed and tested.

Closes phpstan/phpstan#6732
Closes phpstan/phpstan#12704
Closes phpstan/phpstan#10419
Closes phpstan/phpstan#14647
Closes phpstan/phpstan#13431
Closes phpstan/phpstan#12601
Closes phpstan/phpstan#12490
Closes phpstan/phpstan#12420
Closes phpstan/phpstan#11835
Closes phpstan/phpstan#11435
Closes phpstan/phpstan#10290
Closes phpstan/phpstan#10289
Closes phpstan/phpstan#5741
Closes phpstan/phpstan#8031

Phar numbers (fork + turbo, cold cache, LC_ALL=C /usr/bin/time -l)

A = resolve-type-rewrite-2 tip, B = this branch. Runs whose worker died were excluded — every such death was the same PHP 8.5.5 zend_lazy_object_init → ZEND_ASSIGN_OBJ → zend_rc_dtor_func(NULL) segfault, hitting A and B alike (and other benchmark sessions on the same machine today).

Slevomat (9 valid of 12, A n=4 / B n=5): user 607.1 → 614.5 s (+1.2 %, Welch t 1.27), wall 81.0 → 81.6 s (+0.7 %), maxrss 1179 → 1214 MB (+2.9 %, t 0.74), peak main 566 → 564 MB (−0.4 %), largest worker 888 → 912 MB (+2.8 %, t 1.32). Nothing significant.

ShipMonk: only the base has a complete run (A: user 1424.8 s, wall 226 s, maxrss 3.27 GB, peak main 2.97 GB, largest worker 1.09 GB, 38 errors). The first B phar predated the never-recursion fix and ran a worker into the 8 GB limit within a minute — ShipMonk contains that never-to-never shape; the re-run with the fixed phar was stopped before it started. ShipMonk B numbers are still owed.

Error diff on Slevomat: B reports 69 errors A does not; 67 are one pattern — QueryResultSet<*NEVER*>: a @template-covariant TValue class whose constructor never mentions TValue, instantiated with new QueryResultSet($query) and only read through T-returning methods (toArray(), toIterable(), a @var-narrowed single result). Under "nothing inferred → never" that resolves to never and every read reports; the base gave QueryResultSet<mixed>. Open design question: fall back to the bound when the object was read but never sent, and/or treat a bare-marker value flowing into a declared type (@var Product $p = $set->getSingleResult()) as an upper-bound send that resolves T.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GNLiox4nj8c7YstGfsuZ39

@staabm

staabm commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

@zonuexe you might be interessted in this one

@ondrejmirtes
ondrejmirtes force-pushed the bidirectional-type-narrowing-2 branch from 8c59755 to 91d1ea8 Compare September 6, 2026 06:46
@ondrejmirtes ondrejmirtes changed the title Resolve inferred template arguments from the body's usages (#6732 prototype) Resolve inferred template arguments from the body's usages Sep 6, 2026
@ondrejmirtes
ondrejmirtes force-pushed the bidirectional-type-narrowing-2 branch 3 times, most recently from df3e4b8 to 0a7cddc Compare September 7, 2026 15:14
ondrejmirtes and others added 16 commits September 7, 2026 17:14
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GNLiox4nj8c7YstGfsuZ39
Each playground sample, verbatim under its own namespace, analyses clean
with the feature on and reports the original false positive with the
toggle off.

Closes phpstan/phpstan#12704
Closes phpstan/phpstan#12576
Closes phpstan/phpstan#10419
Closes phpstan/phpstan#14647
Closes phpstan/phpstan#13431
Closes phpstan/phpstan#12601
Closes phpstan/phpstan#12490
Closes phpstan/phpstan#12420
Closes phpstan/phpstan#11835
Closes phpstan/phpstan#11435
Closes phpstan/phpstan#10290
Closes phpstan/phpstan#10289
Closes phpstan/phpstan#5741

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GNLiox4nj8c7YstGfsuZ39
A constant-keyed array argument no longer generalizes the collection's key
template to string when the declared return type asks for the literal keys.

Closes phpstan/phpstan#8031

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GNLiox4nj8c7YstGfsuZ39
`new Generic(1)` keeps the exact argument instead of generalizing it, and
the frame's resolution is shared by both flavours, so the native one reads
it too. The offset-set observation reads the dim chain's last key, which
the dim path always has.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GNLiox4nj8c7YstGfsuZ39
Natively a template argument is never inferred and never carries what the
phpdoc flavour resolved it to: `new Foo(1)` is `Foo<mixed>` for an
unbounded template and `Foo<int>` for `@template T of int`, and a generic
call result is the bare native return type. These assertions hold on 2.3.x
and fail on this branch, which lets the frame's resolutions reach the
native types - left failing on purpose as the spec for that fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GNLiox4nj8c7YstGfsuZ39
Keep the call expression as a weak cache key, alongside the weak inference-context key. Cache identity remains exact without extending either object lifetime.

Validation: the cache lifetime assertion fails with the strong expression key and passes with weak keys.
Merge the specialized arrow-argument result back into the caller scope. This preserves constraints from captured generic values and nested generic constructors inside callbacks.

Validation: focused flow assertions and the nested constructor rule regression, reproduced before the fix.
Retain inference facts from boolean, coalescing, ternary, and match branches even when their variable state cannot continue. Include adjacent switch-termination coverage.

Validation: six previously failing terminating-expression assertions now pass.
Carry body constraints into the foreach result independently of variable-state pollution. This keeps generic sites resolvable when polluteScopeWithAlwaysIterableForeach is disabled.

Validation: the no-pollution regression fails before this change and passes after it.
Carry loop constraints into the for statement result even when the loop does not contribute continuing variable state. Cover unreachable while and foreach bodies alongside the for regression.

Validation: focused inference tests pass; the for assertion reproduced the missing constraint before the fix.
@ondrejmirtes
ondrejmirtes force-pushed the bidirectional-type-narrowing-2 branch from 0a7cddc to 66ec5e6 Compare September 7, 2026 15:15
@ondrejmirtes
ondrejmirtes merged commit 3b094df into 2.3.x Sep 7, 2026
15 of 16 checks passed
@ondrejmirtes
ondrejmirtes deleted the bidirectional-type-narrowing-2 branch September 7, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment