Skip to content

Fix quadratic const-context checking for deeply nested literals - #64956

Open
祈雨 (qiyuhuating) wants to merge 1 commit into
microsoft:mainfrom
qiyuhuating:fix-is-const-context-quadratic
Open

祈雨 (qiyuhuating) wants to merge 1 commit into
microsoft:mainfrom
qiyuhuating:fix-is-const-context-quadratic

Conversation

@qiyuhuating

@qiyuhuating 祈雨 (qiyuhuating) commented Oct 25, 2026 •

Copy link
Copy Markdown

Fixes #64955

Checker.isConstContext recurses up through array/object/parenthesized/property-assignment parents and is invoked once per nesting level, with no caching, so checking a deeply nested array or object literal is quadratic in the depth and can effectively hang on valid input (a depth-8000 array literal, which the Go parser handles fine but the JavaScript parser's stack never allowed, takes >30 seconds on main; parse time stays flat at ~25ms). See #64955 for measurements.

This caches the per-node result in NodeLinks using the same computed-flag pattern as NodeCheckFlagsInitializerIsUndefinedComputed, making the const-context computation linear. With this change:

Input (checked with --noEmit --target esnext) before after
array literal, depth 4000 5.3s 0.15s
array literal, depth 8000 >30s 0.18s
object literal, depth 4000 10.8s 0.35s
array literal, depth 2000 0.8s 0.17s
as const / <const T> inference / overload cases unchanged unchanged

Scoping the cache (correctness)

The const-context of a node is not invariant across contextual-typing and inference attempts: overload resolution re-checks the same argument under different parameter types, and a rejected <const T>-less candidate must not decide the answer for the const type parameter candidate that is selected (previously f({ a: 1 }, "s") with a non-const first overload could infer { a: number } instead of { readonly a: 1 }). The per-node cache is therefore only consulted while no non-cache contextual type is on the stack (nonCacheContextualDepth == 0); within those attempts the answer is recomputed exactly as before, while the deeply nested case that needs the fast path runs with only snapshot-style cache entries on the stack. This was flagged by Copilot's review and is covered by TestConstContextAcrossOverloadCandidates, including the nested-descendant variant.

Validation

  • go test ./internal/checker (including the new TestDeeplyNestedArrayLiterals and TestConstContextAcrossOverloadCandidates).
  • Stock-main vs fixed diagnostic diff over 2000 test files from tsc/testdata/tests/cases (every case compiled separately): zero differences.

Copilot AI balanced review requested due to automatic review settings October 25, 2026 06:35
@github-project-automation github-project-automation Bot moved this to Not started in PR Backlog Oct 25, 2026
@typescript-automation typescript-automation Bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Oct 25, 2026

Copilot AI left a comment

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.

🟡 Changes recommended

Cached results can become stale across overload candidates, changing const-generic inference.

1 open issue
What changed in this PR

Addresses #64955 by caching const-context checks in the native TypeScript compiler to avoid repeated ancestor traversal for deeply nested literals.

Changes:

  • Adds computed/value flags for per-node const-context caching.
  • Updates isConstContext to reuse cached results.
  • Adds a depth-20,000 array-literal regression test.
File Description
tsc/​internal/​checker/​types.go Defines const-context cache flags.
tsc/​internal/​checker/​checker.go Caches const-context results in node links.
tsc/​internal/​checker/​checker_test.go Adds the deeply nested array regression test.

Review effort: Balanced

Comment thread tsc/internal/checker/checker.go Outdated
Comment on lines +13877 to +13878
links := c.nodeLinks.Get(node)
if links.flags&NodeCheckFlagsIsConstContextComputed == 0 {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — confirmed and fixed in the updated commit.

I reproduced the staleness exactly as described: with the two overloads, the rejected non-const candidate's isConstContext result survived into the selected <const T> candidate, so f({ a: 1 }, "s") inferred { a: number } instead of { readonly a: 1 }, and the same staleness propagated to nested descendants.

The fix scopes the cache: the per-node result is now only reused while no non-cache contextual type is on the stack (nonCacheContextualDepth == 0). Non-cache entries are exactly the contextual-typing/inference attempts (individual overload candidates, contextual re-checks), so within those attempts the answer is recomputed per candidate as before; the cached fast path only applies while the stack holds only the snapshot-style cache entries pushed by literal checking, which is the deeply-nested case that needs it. Depth-4000/8000 array literals and depth-4000 object literals still check in ~0.2s (vs 5.3s / >30s / 10.8s uncached).

Also added TestConstContextAcrossOverloadCandidates covering this scenario including the nested-descendant variant, and re-ran the stock-vs-fixed diagnostic diff over 2000 test files from tsc/testdata/tests/cases: zero differences.

@qiyuhuating

Copy link
Copy Markdown
Author

@microsoft-github-policy-service agree

@qiyuhuating
祈雨 (qiyuhuating) force-pushed the fix-is-const-context-quadratic branch from 56e1d7f to 7193def Compare October 25, 2026 06:56

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

Status: Not started

Development

Successfully merging this pull request may close these issues.

Checking deeply nested array/object literals is quadratic and can hang in TS7

2 participants