Repository navigation
Skip combined-constraint check while measuring variances to avoid unbounded chase - #64528
Alexis Delrieu (Amatewasu) wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The compiler fix lacks a checked-in regression test for the unbounded-chase scenario.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Prevents unbounded type-relation expansion during variance measurement.
Changes:
- Skips combined-constraint checks while the variance stack is active.
- Documents the recursive growth scenario.
| File | Description |
|---|---|
tsc/internal/checker/relater.go |
Guards combined-constraint evaluation during variance measurement. |
| // that the relation cache and the deeply-nested-type guards never recognize as | ||
| // recursive, and the chase grows without bound (see microsoft/TypeScript#64423). | ||
| // The check never contributes to the measured relation between the marker types. | ||
| if result == TernaryFalse && len(r.c.varianceStack) == 0 && (source.flags&TypeFlagsIntersection != 0 || source.flags&TypeFlagsTypeParameter != 0 && target.flags&TypeFlagsUnion != 0) { |
There was a problem hiding this comment.
LLM-generated test added
Regression test checked in as bd456af6e3 (tests/cases/compiler/effectiveConstraintChaseDuringVarianceMeasurement.ts plus .types/.symbols baselines), in response to the review comment asking for a checked-in test for the variance-stack guard.
-
Minimized from the real repro. It is hand-distilled from the three.js TSL node types that triggered TypeScript 7.0.2 silently runs out of memory while typechecking a project that TypeScript 6.0.3 checks successfully #64423. Only the constructs needed to drive the combined-constraint comparison in
structuredTypeRelatedToduring variance measurement were kept — resolvinguv().div(vec2())measures the variances of the generic extension interfaces, whose hoisted constraints form deferred conditional intersections. -
It fails without the fix. On the pre-fix revision (
673a5f17d7) this file enters the unbounded relation chase and the process dies withruntime: out of memory. Removing or weakening the variance-stack guard reintroduces this immediately, so CI catches it instead of the regression going unnoticed. -
It passes with the fix and pins the exact output. With the guard, the file checks to completion in under a second with no diagnostics, and the baselines record the precise inferred types (e.g.
x : Node<"vec2">). This also guards the opposite failure mode: a future change that "fixes" a hang by making the checker give up and infer different types would fail the baseline comparison.
Reproducing the pre-fix failure locally:
git show 673a5f17d7:tsc/internal/checker/relater.go > tsc/internal/checker/relater.go
cd tsc && GOMAXPROCS=2 timeout 120 go test ./internal/testrunner \
-run 'TestLocal/effectiveConstraintChaseDuringVarianceMeasurement'
# pre-fix: never terminates (chase grows until OOM) / fixed: "ok" in ~1sThere was a problem hiding this comment.
What do you think Jake Bailey (@jakebailey)? :)
Verified against the private project from the issueI ran the full typecheck of the actual project behind #64423 (a private React 19 + R3F + TSX app, ~855 files) with binaries built from this PR branch vs. unpatched
Diagnostics parity holds: the patched TS 7 reports exactly the same result as TS 6 (zero errors) on the full project, so the guard is not observable in output — it only stops the runaway. Caveats, for completeness:
|
This comment was marked as resolved.
This comment was marked as resolved.
The combined-constraint comparison in `structuredTypeRelatedTo` hoists the constraints of the instantiable constituents of an intersection source into a combined constraint and compares that constraint against the target. While measuring variances, the hoisted constraint may itself be an intersection with instantiable constituents (e.g. deferred conditional types, as produced by heavily overloaded generic builder types), so a failed comparison re-enters the same logic with a structurally new and ever larger source that neither the relation cache nor the deeply-nested-type guards recognize as recursive, and the chase grows without bound until the process runs out of memory. Skip the check while the variance stack is non-empty; it never contributes to the measured relation between the marker types. Fixes microsoft#64423
…e measurement Adds a minimized compiler fixture distilled from the three.js TSL node types (microsoft#64423). Resolving `uv().div(vec2())` measures the variances of generic extension interfaces whose hoisted constraints form deferred conditional intersections; without the variance-stack guard in `structuredTypeRelatedTo` the combined-constraint comparison re-enters itself with structurally new, ever larger sources and the chase grows without bound until the process exhausts memory. With the guard the file checks to completion with no diagnostics, and the checked-in baselines pin that result so the guard cannot be removed or weakened unnoticed. Verified against the pre-fix revision that this fixture grows the relation chase until the runtime runs out of memory, and that the fixed revision terminates cleanly.
bd456af to
f326b55
Compare

Fixes #64423
Problem
TypeScript 7 runs out of memory (unbounded growth, zero diagnostics) on programs that TypeScript 6 checks fine. Root cause: the combined-constraint comparison in
structuredTypeRelatedTohoists the constraints of the instantiable constituents of an intersection source into a combined constraint and compares it against the target. While measuring variances, the hoisted constraint may itself be an intersection with instantiable constituents (e.g. deferred conditional types produced by heavily overloaded generic builder types), so a failed comparison re-enters the same logic with a structurally new and ever larger source that neither the relation cache nor the deeply-nested-type guards recognize as recursive — the chase grows without bound until the process dies.Fix
Skip the combined-constraint check while the variance stack is non-empty; it never contributes to the measured relation between the marker types.
Verification (both repros from the issue)
@react-three/fiberIntrinsicElementsaugmentation +<SvgIcon />, exact versions from the issue): before — killed at a 3 GiB / 6 GiB address-space cap with zero output; after — ~4.4 s, ~700 MB RSS, exit 0.three/tslrepro (uv().div(vec2(1, 1)), three@0.185.1 + @types/three@0.185.1): before — killed at the cap with zero output; after — ~3.1 s, ~280 MB RSS, exit 0.go test ./...passes (only pre-existing environmental failure ininternal/astnav, which requires a locally built JStypescriptpackage in rootnode_modules; fails identically on unpatchedmain).Note on the approach
An earlier variant of this change additionally capped the chase depth (a counter on
Relaterlimiting nested combined-constraint comparisons to 3). That cap regressedtests/cases/compiler/indexedAccessAndNullableNarrowing.ts(new false TS2345 errors on thesyncStoreProppattern from #57693, which both TS 6 and unpatched TS 7 accept), and the depth needed by legitimate code is not bounded by any small constant, so the cap was dropped. The variance-stack guard alone fixes both repros.Disclosure
This change was generated by an LLM (as was the investigation in the issue thread).
Checklist notes
npx hereby lint/check:format(no rootnode_modulesin this checkout); rangofmt+go vet(clean) and the full Go test suite instead.mainbranchgo test ./...passesnpx hereby lintnpx hereby check:format