Fix stack overflow crash in base constraint resolution#4242
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adjusts base-constraint resolution for conditional types in the Go checker to avoid stack overflows by ensuring distributive-conditional base-constraint computation stays within the getResolvedBaseConstraint recursion limiter. It also adds a regression test and updates affected baselines.
Changes:
- Update
computeBaseConstraintto inline distributive-conditional constraint computation forTypeFlagsConditional. - Add a new compiler regression test
infiniteConstraints2.ts. - Update submodule baseline diagnostics for
infiniteConstraints.tsand add new baselines forinfiniteConstraints2.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
internal/checker/checker.go |
Changes conditional-type base constraint resolution to avoid unbounded recursion/stack overflow. |
testdata/tests/cases/compiler/infiniteConstraints2.ts |
Adds a repro-style regression test for the conditional-type constraint recursion scenario. |
testdata/baselines/reference/compiler/infiniteConstraints2.types |
New baseline for the added regression test. |
testdata/baselines/reference/compiler/infiniteConstraints2.symbols |
New baseline for the added regression test. |
testdata/baselines/reference/submodule/compiler/infiniteConstraints.errors.txt |
Updates expected diagnostics (now includes recursion limiter diagnostics instead of crashing). |
testdata/baselines/reference/submodule/compiler/infiniteConstraints.errors.txt.diff |
Corresponding submodule baseline diff update. |
| d := t.AsConditionalType() | ||
| if d.root.isDistributive && c.cachedTypes[CachedTypeKey{kind: CachedTypeKindRestrictiveInstantiation, typeId: t.id}] != t { |
|
|
||
| export type Prepend<Elm, T extends unknown[]> = | ||
| T extends unknown ? | ||
| ((arg: Elm, ...rest: T) => void) extends ((...args: infer T2) => void) ? T2 : |
|
|
||
| type T1<B extends { [K in keyof B]: Extract<B[Exclude<keyof B, K>], { val: string }>["val"] }> = B; | ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| !!! error TS2321: Excessive stack depth comparing types '"val"' and 'keyof Extract<B[Exclude<keyof B, K>], { val: string; }>'. |
There was a problem hiding this comment.
This excessive stack depth error occurs because we recursively build up a relation stack of 100 keyof XXX types, where XXX is an inifinitely recurring conditional type. We could silence this error (and instead produce a "maybe" result) by introducing distinct recursion identities for keyof types, but the error seems reasonable.
|
@typescript-bot test it |
|
@ahejlsberg Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@ahejlsberg Here are the results of running the top 400 repos with tsc comparing Everything looks good! |
jakebailey
left a comment
There was a problem hiding this comment.
I feel like a comment would be worth it, but not blocking I suppose
The whole purpose of |
* origin/main: Make LS checkers time out after being idle, segment based on use (take 2) (microsoft#3435) Fix external helper crash after module-status updates (microsoft#3994) Allow LSP server to use builtin watcher on Windows and macOS (microsoft#4288) Rebuilt CLI watcher around new `fswatch` package (microsoft#4026) Fix stack overflow crash in base constraint resolution (microsoft#4242) # Conflicts: # internal/project/session.go
This PR modifies base constraint resolution for conditional types to ensure the logic is protected by the recursion limiter in
getResolvedBaseConstraint.Fixes microsoft/TypeScript#63269.
Fixes microsoft/TypeScript#62966.