Repository navigation
Make checker behavior order independent - #65293
Jake Bailey (jakebailey) wants to merge 27 commits into
Conversation
e8dc435 to
7afb732
Compare
7afb732 to
a7534e3
Compare
a7534e3 to
d31d179
Compare
|
One or more custom setup steps configured for this repository failed during this Copilot code review run: Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review. Note You can configure setup steps for Copilot code review separately from Copilot cloud agent with a |
There was a problem hiding this comment.
🟡 Changes recommended
Critical type-checking regressions retain a false missing-property diagnostic, accept structurally incompatible assignments, and select an inapplicable overload that hides an invalid assignment.
2 open issues
1 file not reviewed
tsc/internal/api/enum_values_generated.go: Generated file
What changed in this PR
These changes support the order-independent checker work in #65064 by adapting checker integration points to memo-backed state and updating regression coverage.
Changes:
- Makes checker consumers, serialization, and emit-reference tracking memo-aware.
- Adds regressions for contextual typing, cyclic member resolution, and JavaScript assignments.
- Updates diagnostic, type, symbol, declaration, and incremental baselines; removes an obsolete object flag.
Validation: Static review only; builds and tests were not run.
| File | Description |
|---|---|
| tsc/testdata/tests/cases/compiler/lateBoundMutualMemberTables.ts | Updated as part of this pull request. |
| tsc/testdata/tests/cases/compiler/jsExpandoIndexedAssignmentDiagnostic.ts | Updated as part of this pull request. |
| tsc/testdata/tests/cases/compiler/contextualReturnTypeInSpreadArgument.ts | Updated as part of this pull request. |
| tsc/testdata/tests/cases/compiler/contextualCheckReentryAfterParameters.ts | Updated as part of this pull request. |
| tsc/testdata/tests/cases/compiler/contextualCheckInDiscardedCycle.ts | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/tsc/incremental/global-diagnostics-produced-during-semantic-checking.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/conformance/templateLiteralTypes1.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/conformance/recursiveMappedTypes.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/conformance/privateNameNestedClassFieldShadowing(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/conformance/importTag13(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/conformance/dependentDestructuredVariablesReferenceInDeclaration1(checkorder=typesfirst).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/conformance/dependentDestructuredVariablesReferenceInDeclaration1(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/varianceCycleDiscardedDiagnostics.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/varianceCycleDiscardedDiagnostics.symbols | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/varianceCycleDiscardedDiagnostics.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/varianceCircularConsistency.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/unusedLocalsOnFunctionDeclarationWithinFunctionDeclaration1(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/typeParameterWithInvalidConstraintType.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/typeParameterHasSelfAsConstraint(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/templateLiteralTypeExcessiveLength(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/templateLiteralTypeExcessiveLength(checkorder=default).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/relationDeeplyNestedCheckOrder.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/relationComplexityError(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/relationComplexityError(checkorder=default).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyExportEqualsWithOtherExportsImportOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyExportEqualsWithOtherExportsImportOrder.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyExportEqualsWithOtherExportsCheckOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyExportEqualsWithOtherExportsCheckOrder.symbols | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyExportEqualsWithOtherExportsCheckOrder.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsNonExportedMemberOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsNonExportedMemberOrder.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsNamespaceMembersResolutionOrderReexportedTypes.symbols | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsNamespaceMembersResolutionOrderReexportedTypes.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsNamespaceMembersResolutionOrderReexportedTypes.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsNamespaceMembersResolutionOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsNamespaceMembersResolutionOrder.symbols | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsNamespaceMembersResolutionOrder.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsNamespaceMembersResolutionOrder.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsExportStarDeclarationCollisionOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsExportStarDeclarationCollisionOrder.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsExportStarCollisionOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDiagnosticsExportStarCollisionOrder.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitVariableMatchesInScopeOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitVariableMatchesInScopeOrder.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitUmdGlobalAliasesOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitUmdGlobalAliasesOrder.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitSharedCheckerOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitRenamedExportOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitRenamedExportOrder.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitLateBoundIndexSignatureSiblingsOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitLateBoundIndexSignatureSiblingsOrder.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitExternalModuleContainerIndexOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitExternalModuleContainerIndexOrder.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitExportEqualsQualifiedNameOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/noFlakyDeclarationEmitExportEqualsQualifiedNameOrder.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/namespaceMembersResolutionOrderExportStar.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/namespaceMembersResolutionOrderExportStar.symbols | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/namespaceMembersResolutionOrderExportStar.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/namespaceMembersResolutionOrderExportStar.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/limitDeepInstantiations(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/limitDeepInstantiations(checkorder=default).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/lateBoundMutualMemberTables.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/lateBoundMutualMemberTables.symbols | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/lateBoundMutualMemberTables.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/lateBoundMutualMemberTables.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/keyofGenericExtendingClassDoubleLayer.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/jsxIntrinsicElementsExtendsRecord(checkorder=typesfirst).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/jsxIntrinsicElementsExtendsRecord(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/jsxImportSourceNonPragmaComment(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/jsExpandoIndexedAssignmentDiagnostic.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/jsExpandoIndexedAssignmentDiagnostic.symbols | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/jsExpandoIndexedAssignmentDiagnostic.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/jsExpandoIndexedAssignmentDiagnostic.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/jsdocTypeTagOnContextuallyTypedFunction.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/jsdocTypeTagOnContextuallyTypedFunction.symbols | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/jsdocTypeTagOnContextuallyTypedFunction.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/incorrectRecursiveMappedTypeConstraint.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/excessivelyLargeTupleSpread(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/dependentDestructuringBindingElementInitializer(checkorder=typesfirst).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/declarationEmitExternalModuleContainerIndexDeclarationOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/declarationEmitExternalModuleContainerIndexDeclarationOrder.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/contextualReturnTypeInSpreadArgument.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/contextualReturnTypeInSpreadArgument.symbols | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/contextualReturnTypeInSpreadArgument.js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/contextualCheckInDiscardedCycle.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/contextualCheckInDiscardedCycle.symbols | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/classVarianceResolveCircularity2.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/classVarianceResolveCircularity2.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/circularScopeThunkReverseMapped(checkorder=typesfirst).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/circularScopeThunkReverseMapped(checkorder=default).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/circularReturnTypeSubtypeReductionOrder.types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/circularReturnTypeSubtypeReductionOrder.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/circularInlineMappedGenericTupleTypeNoCrash(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/circularInlineMappedGenericTupleTypeNoCrash(checkorder=default).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/circularIndexedPropertyInstantiation.errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/circularAccessorAnnotations(checkorder=typesfirst).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderStableInstantiationErrors(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderStableInstantiationErrors(checkorder=reverse).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderStableInstantiationErrors(checkorder=default).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderSpreadCycle(checkorder=typesfirst).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderSpreadCycle(checkorder=typesfirst).js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderSpreadCycle(checkorder=reverse).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderSpreadCycle(checkorder=reverse).js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderSpreadCycle(checkorder=isolated).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderSpreadCycle(checkorder=isolated).js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderSpreadCycle(checkorder=default).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderSpreadCycle(checkorder=default).js | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderEmitBeforeCheck(checkorder=isolated).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderEmitBeforeCheck(checkorder=default).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderCircularDefaults(checkorder=typesfirst).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderCircularDefaults(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderCircularDefaults(checkorder=isolated).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderCircularDefaults(checkorder=isolated).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderCircularDefaults(checkorder=default).types | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderCircularDefaults(checkorder=default).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderCircularConstraint(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderCircularConstraint(checkorder=reverse).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderCircularConstraint(checkorder=default).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderAwaitedCycle(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderAwaitedCycle(checkorder=reverse).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/checkOrderAwaitedCycle(checkorder=default).errors.txt | Updated as part of this pull request. |
| tsc/testdata/baselines/reference/compiler/bigintArbirtraryIdentifier(checkorder=typesfirst).errors.txt | Updated as part of this pull request. |
| tsc/internal/execute/tsctests/tsc_test.go | Updated as part of this pull request. |
| tsc/internal/checker/utilities.go | Updated as part of this pull request. |
| tsc/internal/checker/tracer.go | Updated as part of this pull request. |
| tsc/internal/checker/symbolaccessibility.go | Updated as part of this pull request. |
| tsc/internal/checker/services.go | Updated as part of this pull request. |
| tsc/internal/checker/printer.go | Updated as part of this pull request. |
| tsc/internal/checker/nodebuilderimpl.go | Updated as part of this pull request. |
| tsc/internal/checker/nodebuilder_hover.go | Updated as part of this pull request. |
| tsc/internal/checker/links.go | Updated as part of this pull request. |
| tsc/internal/checker/inference.go | Updated as part of this pull request. |
| tsc/internal/checker/grammarchecks.go | Updated as part of this pull request. |
| tsc/internal/checker/emitresolver.go | Updated as part of this pull request. |
| tsc/internal/ast/utilities.go | Updated as part of this pull request. |
| tsc/internal/ast/symbol.go | Updated as part of this pull request. |
| tsc/internal/api/enum_values_generated.go | Updated as part of this pull request. |
| packages/typescript/enums/objectFlags.js | Updated as part of this pull request. |
| packages/typescript/enums/objectFlags.d.ts | Updated as part of this pull request. |
Review effort: Balanced
| >pick(new Foo(this)).foo : string | ||
| >pick(new Foo(this)) : B | ||
| >pick(new Foo(this)).foo : any | ||
| >pick(new Foo(this)) : A |
d31d179 to
77f463c
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
Critical type-checking regressions accept unsound assignments and select an incompatible overload, allowing invalid values through while hiding expected diagnostics.
1 resolved since last review
1 file not reviewed
tsc/internal/api/enum_values_generated.go: Generated file
Review effort: Balanced
0b221ed to
9599837
Compare
b143a9a to
2acf67a
Compare
2acf67a to
9b506d7
Compare
Building the error for a nonexistent property prints types. In code that is not checked, such as a generated "// @ts-nocheck" validator, printing a function type infers its return type from its body, which can report more nonexistent properties, whose errors are built in turn. Since the errors are built once the check finishes, nothing in progress cuts this short: on webpack, it builds 1400 errors in one schema file, inferring the return types of most of its validators, though the program never reports any error in that file. So the checker asks the program whether it would report errors in the file before reporting one there. webpack and vscode, default GC, default checkers: │ before.txt │ after.txt │ │ sec/op │ sec/op vs base │ webpack 2.980 ± 24% 2.685 ± 36% -9.90% (p=0.007 n=8) vscode 14.32 ± 3% 14.57 ± 8% ~ (p=0.878 n=8) geomean 6.531 6.254 -4.25% │ before.txt │ after.txt │ │ peak-RSS-bytes │ peak-RSS-bytes vs base │ webpack 983.3Mi ± 3% 919.1Mi ± 6% -6.53% (p=0.000 n=8) vscode 5.034Gi ± 3% 4.943Gi ± 2% ~ (p=0.195 n=8) geomean 2.199Gi 2.106Gi -4.20% │ before.txt │ after.txt │ │ instructions/op │ instructions/op vs base │ webpack 50.95G ± 3% 46.17G ± 4% -9.37% (p=0.000 n=8) vscode 224.3G ± 2% 228.0G ± 3% ~ (p=0.382 n=8) geomean 106.9G 102.6G -4.02%
A declared type parameter with a circular constraint is circular no matter how it is used, and checking its declaration reports it. Which use happened to resolve the constraint before that is an accident of check order, so pointing at it as the origin of the cycle made the diagnostic depend on that order. Only an instantiated type parameter has a cycle that originates at a use.
A mapped type resolves its constraint eagerly so that errors in the constraint are reported at the mapped type. It did so only when the type was created, so when the type was first created while checking another node, the mapped type itself never reported those errors.
A cycle through return types gave different results depending on where it was entered. Entered at rebuild's return type, re-entering it fails rebuild, so both members report an implicit any. Entered at b's return type, subtype reduction of rebuild's return type finds b in progress and compares as if it returned any, without failing anything, so rebuild returns B and was kept as a participant of the cycle, though computing it from scratch fails. Cycles through the types of symbols are already resolved from the member declared first (see yieldCycle). The same now applies to the return types of declared signatures, both when a cycle re-enters a return type and when it only observes one in progress, so that each order gives the result of resolving the first declared member first (seen in effect's SchemaAST). Contextually checking a function expression infers its return type from its body, and that inference was pushed as a return type resolution of the function's signature. When the body calls a function whose return type depends on the call enclosing the function expression, resolving that call requests the function expression's return type, which, seen in progress, returned any without detecting a cycle, so the enclosing function's return type was inferred from a provisional any instead of reporting the circularity (seen in OmniRoute). The inference now runs without marking the signature in progress, as in TypeScript: the nested request computes the return type itself, reaches the enclosing function, and the cycle is reported on both. The result is cached only if no request has already cached one, the same as when the return type is requested directly. The flag marking a function as contextually checked is set before its return type is inferred, and is TypeScript's guard against re-checking the function from within its own check, so it is never discarded. The inferred return type is now kept whenever the flag is, so that a discarded cycle in between cannot leave the flag without the return type, which made a const assertion in the body readonly when queried before its binding pattern.
Completing contextual signatures before anyone reads them switched parameter assignment from the signature of the function's symbol to the signature built from its declaration. In JavaScript the symbol's signature comes from a @type tag when one is present, and the function's parameters take their types from that tag. Using the declaration's signature wrote the outer contextual types onto those parameters instead, producing errors inside the function body that the tag rules out (seen in webpack). Prefer the tag's signature, as before, and fall back to the declaration's signature otherwise.
An instantiation stopped by the limit on the depth of the whole instantiation names every type that recurs in it, in the order in which they recur. Which type of the cycle the instantiation entered first depends on the results cached before, so checkers that had checked different files named the same cycle starting from different types, and reported the same error twice with different messages (seen in zod, with checkers running concurrently). The names now start from the type declared first, so the message is the same wherever the cycle is entered. The limit on the depth of a single computation already names only the types that computation instantiated, which does not depend on where it was entered, so its messages keep the order in which the types recur.
While the type implied by a binding pattern is computed as the
contextual type of its initializer, references to the pattern's
elements from the elements' defaults are any. The calls and types
computed from them were cached anyway, so asking for the contextual
type of the initializer first left `extra` in
const { kind, payload, extra = kind === "a" ? ... : ... } = u;
as any rather than unknown. TypeScript behaves the same way.
Track the patterns as resolutions in progress rather than a separate
stack, so that a reference returning any marks the computation circular
and the results cached from it are discarded.
(seen in check order fuzzing)
A call resolution hides the resolutions below it, so a symbol whose initializer reaches itself through a call argument is computed again, nested in itself, as in main. The nested computation observes things in progress that the outer one entered: here, a mapped property whose contextual type is skipped, and the reverse mapping whose depth cutoff it reaches. Those mark everything since as circular, so the nested result was discarded, and the outer computation, reaching the same types again outside that context, then failed with a circularity. When everything the nested computation observed in progress was entered by the outermost hidden resolution of the same target, the nested computation is the one that resolution always computes first from scratch, whichever entry led to it, so its result is kept as a cycle participant, as main keeps the first result set. Main also keeps the reverse-mapped property type computed under the depth cutoff, so it types the property as unknown. That result still depends on the cutoff and is discarded, so the property is computed again from scratch afterwards, giving its full type: an improvement over main rather than a match (seen in arktype).
When a query enters a cycle somewhere other than its program-order entry, the cycle yields to its first member and recomputes it with the resolutions below hidden. Two things made that recomputation differ from computing the member first: - The reverse mapping stacks, contextual types and inference contexts of the hidden resolutions stayed visible, so the recomputation saw contexts that a computation from scratch never has. A yield now hides them along with the resolutions and restores them when it ends. - Results were kept as cycle participants only by comparing the outermost cycle head in progress against the resolution. A cycle headed below the yield, which the yield cannot observe, caused the yield root and the hidden re-entries of itself to be discarded, so the member was computed again and genuinely re-entered itself. Each type resolution now tracks the lowest cycle head it observed, and a result is kept when the computations it observed were entered from scratch: by the outer resolution of itself, by the yield it is the root of, or by the outermost measurement of variances. (seen in circularScopeThunkReverseMapped)
Which symbols are referenced, which nodes need deferred or unused checks, and which aliases are referenced are facts about a node that only the check of its file consumes. Recording them as effects made nearly every query result carry effects, so emit and other queries without a check retained and walked a large effect graph: --noCheck emit of VS Code used twice the time and memory of main. Each fact now depends only on its node, and the check reaches every node, so the first computation reaching a fact records it. Write-only names are not uses wherever a name is resolved. The check now also checks methods and accessors in destructuring targets and decorators where they are not valid, which only a query computing their types reached before, and follows referenced import = declarations itself once the file's references are marked, so that following them reports errors regardless of which computation cached them first. Marks are written only by checking, or by a computation a check started that entered a cycle. A query records them on its result, and they are replayed even when its other effects are dropped, as whether a computation reaches the nodes they mark does not depend on the results it read. Following a referenced alias to the aliases it refers to computes, so it happens where the alias is referenced, and the mark only records the aliases found. A test finds marks written anywhere else.
When no overload applies, call resolution caches its result before reporting errors, so that a circular request for the call while they are reported finds it. Discarding that result in a cycle restored the marker put in place when resolution began, leaving the call marked as being resolved. Caching it like the final result clears the marker instead.
Signature help resolves a call from the arguments typed so far, without reporting errors, and string literal completions resolve it with inference from the argument being typed blocked. The results those requests cache depend on that state, which is not part of their keys, so checking that reuses them could infer other types or miss errors. Nothing such a request caches is kept: it runs in a cycle headed below every computation, which ends with the request. The state of such a request, the number of arguments typed so far and the nodes inference is blocked from, is held in a request that exists only while it runs, rather than in fields of the checker that any computation could read.
A checker's memos are not its only state: it also holds the state of the computations in progress, such as the contextual types pushed for the nodes being checked, the resolutions on the stack, and the cycles being resolved. The next request reads anything a request leaves there, so none of it may outlast the request that set it. Every field of Checker is now either listed as the state of the computations in progress, which the test harness checks is at rest after every request it makes, or listed with the reason it is not, as the fields of the memo store and links already are. A few fields kept indices or flags after the computation using them finished, which nothing read: the lowest cycle head observed, the cycle flag of errors collected at the current node, the start of a circularity, of a reverse mapping and of a depth limit. They are now left at rest too.
When a variance measurement reaches a type whose measurement is in progress, the results cached until the outermost measurement finishes may be discarded, in case the measurement did not start at the canonical entry of its circularity. Until then, checking switched to recording: storing the effects of each result to apply later, and accounting for them on every read of a cached result. Few measurements are ever discarded, a handful per project, but recording roughly doubles the cost of the work done in the meantime, and each checker measures the shared types again, so with several checkers it accounts for most of the extra time these changes cost on xstate. So checking keeps applying effects during a variance cycle, except those that discarding the measurement would have to take back: errors, suggestions and deferred diagnostics are held until nothing can discard them, and dropped with the results if the measurement is.
Every memoized computation records its key as in progress and removes it when it finishes, so that re-entering it is caught as a cycle. Nearly every lookup finds nothing, yet the hash table made each computation pay for an insertion and a deletion that shifts probe sequences. Counting keys per slot makes the common case one load and one increment; the rare hit searches the resolution stack, which already holds the keys.
Each memo map kept its own record of the entries being computed: their keys, where on the resolution stack each was, and, once there were many, a count of each key. The checker already counts every other computation in progress and finds it on the resolution stack, so map entries are now counted with the rest by a hash of the map and key, and found on the resolution stack when the count says one may be in progress. A map keeps only the keys themselves, which a resolution has no room for.
…ested Finding a computation in progress searched the whole resolution stack whenever its slot of the resolving table was taken, and marking the computations that read an incomplete result walked the stack above it. With thousands of computations in progress, as in an instantiation that runs away, nearly every slot is taken, so both cost time proportional to the depth on every step, and nesting deeply took quadratic time.
The limit on the depth of the whole instantiation stack failed an instantiation at whatever depth the stack reached, which depends on how much of what it needed was cached before, so whether a finite but deep instantiation failed depended on the order types were checked in. Caching only ever shortens the stack, so an instantiation that would stay below a limit computed from scratch stays below it in any order. The limit is now a guard against runaway instantiations, far above any finite one in real code (the deepest measured, in arktype's tests, nests 189 instantiations computed from scratch), while the limit on each computation still reports deep ones. An instantiation reaching the guard fails as a whole wherever it is computed or reused, so an infinite one fails the same way in any order.
A cycle that passes through a member table in progress is computed again as if the table had been entered first, with everything below the table hidden. The table's own memoized computation sits just below its type resolution, so it was hidden too, and reading the table then failed the recomputation instead of reading the table. Which member of such a cycle failed then depended on where the cycle was entered: a type alias read through an index signature was circular when queried first, and a parameter typed through a class's base was circular when its class was checked first. Fixes 64998 For 61721
A cycle yield recomputes a member of the cycle with the computations in progress hidden. Reading a member table being resolved below the yield failed the recomputation, since the table it would see is whatever part of it has been published. A table that has published nothing is instead resolved again from scratch inside the yield, so there is nothing incomplete to read, and failing there reported a circularity for a property only when its class was checked before its type was queried. Fixes 61711
When a qualified name used as a type does not resolve, the error
suggests typeof if the name resolves as a value, which needs the type
of the value its first identifier refers to. In
declare const n: n.T;
declare namespace n {}
that type is the annotation being resolved. Checking the annotation
first made it the head of the cycle, and the missing member it reports
was dropped along with the other results computed from the incomplete
annotation, so it was reported only when the type of n was queried
first. The type of a value used within its own declaration can depend
on the name being resolved, so no suggestion is made there, and the
missing member is reported in every order, without a circularity error.
For 18163
Reaching the limit on the instantiation stack failed every
instantiation in progress. A computation that resolves part of a type
whenever it is first needed, like the element type of a deferred array
type, is reached inside an instantiation or not depending on who needs
it first, so whether the instantiation needing it failed as a whole
depended on order. In
type A = A[];
const r = ([] as A).flat(Infinity);
r was any[] when the element type of A was resolved first, but any when
the call was checked first. Reaching the limit now fails only the
instantiations in progress in the innermost computation other than of a
cached instantiation, which fail the same way wherever the computation
is reached, so r is any[] in every order, as in TypeScript 6. A result
carrying the error no longer fails an instantiation reading it, as
computing it there would fail only within it.
For 49280
A base constraint keeps how deep its own walk went, so that a walk reusing it can tell whether computing it again there would give up. Comparisons are about to keep the same kind of record for their cached failures, so the record becomes a type of its own, a height, with the checks both need. The multiplication spreading the bits of a key over the high bits, which several tables and filters index by, is likewise written once.
A comparison nested too deeply is truncated: 100 levels deep, where both sides are deeply nested, or where a conditional type is deeply nested on one side. All of these count the comparisons in progress around it, so a result cached by one comparison could be reused inside another that would have truncated the comparisons the result came from, and the result then depended on which comparison ran first. A cached failure, and a cached result that reports reliability flags or depends on a failure through a check that truncation can change, now keeps a summary of the comparisons below it that it depends on: how deep they went on each side, and which recursion identities they pushed. It is reused only where the comparisons in progress, counting the comparison itself, could truncate none of them, and is otherwise compared again in place. Checks that reject types that relate, or pick among types by which relate, record the results they read, and results depending on a comparison assumed related or truncated outside them are not cached, or reused only where as much is pushed onto the stacks. A cycle through such a check, such as an inference constraint check that compares the comparison in progress again, then gives the same answer whichever comparison in it starts first. The recursion identities on the stacks are now kept with counts, which also replaces scanning the stack for every comparison to find deeply nested types. inferFromTypeArgumentsNoInfiniteRecursion no longer reports an excessive depth in any check order, as TypeScript 6.0 does not.
0470523 to
eea2f5e
Compare
Whether a comparison is too complex depended on how many of the results it needs were cached by comparisons made before it, so the same assignment could be an error in one check order and not another. Charge each reused result the results that computing it cached, and count a comparison only too complex for those charges again with no result cached, which decides it exactly.
eea2f5e to
bf061ab
Compare

Fixes #30982
Fixes #61711
Fixes #63949
Fixes #64998
Fixes #65440
For #18163
For #49280
For #61721
This is the final part of #65064, adding all of the new infra to make the checker insensitive to orderings
The big and scary change is the introduction of a new state management type dubbed "memo". Every single piece of checker state was moved into memos on a sidecar object on the checker (maybe for future sharing??). This memo system keeps track of the current computation, what depends on what else, eliminating basically all flags/bools that track "computed" or "resolved"-ness. This tracking also records side effects, including diagnostics, "referenced" marks, deferred checks, emit helper checks. The result is that if you query via the API and that calculates some error, those same errors will appear later if the checker runs.
With those data structures in place, now all of the cycles can be handled. Once a cycle is discovered, the checker starts logging what it's doing, and throws all of that info away assuming it's been poisoned.
All of the different recursion stacks get pulled into a single memo stack, which is a cool simplification too.
Variance is now a fixpoint algorithm, proceeding in rounds until stability. In practice, it doesn't iterate much.
Instantiation limits also stop being global numbers (since of course, that is order dependent side effect stuff). Instead, things get limited by the amount of work that's been done, and if you then pull on caches related to work, it tallies that work up to error consistently when needed. (I don't think this is fully done yet, though, since there are depth limits that are really hard to get right. Still thinking about that.)
In some cases, cycles still can't be fixed up, and so they're made consistent by doing what we've done previously for these cases, which is to pick a canonical place to restart to at least get it consistent.
Unfortunately, this all has a cost; the end of the PR tries to claw back perf, but at the end of the day, if we do more work tracking, we spend more CPU and memory tracking. It's still an open question whether this can be cheapened any more. But, my hope is that this idea is minimal enough that we could then optimize TS as a whole assuming this order-independence, e.g. work stealing, checker recycling, shared state, maybe even using LRU caches, or dumping data to disk.
But, it could also well be that this is slop and impossible to manage. But I don't want to sit on it anymore and want to see some real world effects.
However, I think that this will fix countless issues on the tracker. I have no idea how to evaluate all 5k issues to see if this PR fixes them. But it's gotta be a lot.