fix(compiler): fix stack overflow when checking assignability of recursive types - #11779
fix(compiler): fix stack overflow when checking assignability of recursive types#11779JoshLove-msft wants to merge 2 commits into
Conversation
…rsive types `isTypeAssignableToInternal` created a brand new relation cache for every nested call instead of forwarding the one it was given, so the "in progress" entry seeded by `areModelsRelated` only survived a single level and mutually recursive models recursed forever. Unions were never seeded at all, so any union reaching itself did the same. Forwarding the cache alone is not enough: the cache stored only the `Related` result and dropped the errors, and `areModelsRelated` turns a result with no errors back into `Related.true`. The cache now stores the errors alongside the result. A purely cyclic union describes an empty set of values, so it is vacuously assignable to anything, and the dual seed is used on the target side where being assignable to a union only requires one variant to match. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be5c2e95-cfc7-417c-bc70-b34cf66bbea4
commit: |
There was a problem hiding this comment.
Pull request overview
This PR fixes a compiler stack overflow in assignability checks involving recursive types by making the relation cache effective across nested assignability calls and by correctly seeding union relations to terminate cycles.
Changes:
- Forward a single relation cache through nested
isTypeAssignableToInternalcalls and cache both relation results and associated errors. - Seed relation cache entries for source unions (and for assignability-to-union checks) before walking variants to ensure cyclic unions terminate.
- Add regression tests covering self-recursive and mutually recursive models/unions, plus mismatch cases deep in cycles.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/compiler/src/core/type-relation-checker.ts | Reworks relation caching to include errors and ensures recursion termination for cyclic unions/models. |
| packages/compiler/test/checker/relation.test.ts | Adds regression tests for recursive models/unions and cyclic assignability scenarios. |
| .chronus/changes/fix-relation-checker-recursion-2026-8-26.md | Adds a compiler changelog entry documenting the stack overflow fix and affected cases. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
All changed packages have been documented.
Show changes
|
|
You can try these changes here
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
packages/compiler/src/core/type-relation-checker.ts:1120
isAssignableToUnioncreates the sameunassignablediagnostic twice (once for the seeded cache entry and again on the final return). This does extra work on the common failure path and can be avoided by reusing a singleerrorsarray for both the seed and the return value.
relationCache.set(
[source, target],
[Related.false, [createUnassignableDiagnostic(source, target, diagnosticTarget)]],
);
isTypeAssignableToInternalcreated a brand new relation cache for every nested call instead of forwarding the one it was given, so the "in progress" entry seeded byareModelsRelatedonly survived a single level. Two mutually recursive models therefore recursed forever. Unions were never seeded at all, so any union reaching itself did the same.This reproduces on
maintoday through a regular template constraint, so it can be pasted directly into the playground:The compiler crashes with
RangeError: Maximum call stack size exceeded.What changed
isTypeAssignableToInternalnow forwards therelationCacheit was given instead of allocating a fresh one.[Related, errors]instead of justRelated. Forwarding the cache alone was not safe: the old code returned[cached, []]andareModelsRelatedturns a result with no errors back intoRelated.true, so a cachedfalsewould flip totrue.isTypeAssignableToWorkerandisAssignableToUnionnow seed the cache before walking variants, the same wayareModelsRelatedalready did.Semantics of a cyclic union
A purely cyclic union such as
union Loop { self: Loop }has no way to produce a value, so it describes the empty set and is vacuously assignable to anything, exactly likenever. That is why the source side is seeded withRelated.maybe.The target side is the dual: being assignable to a union only requires one variant to match, so a cycle brings no new information and is seeded with
Related.false.Validation
tsc -p tsconfig.build.json --noEmitclean@typespec/openapi32,578 tests pass, plus http, json-schema, versioning, rest, events, sse, streams, and xmlpnpm lintclean--generated by Copilot