Repository navigation
[experiment] fully order independent type checking - #65064
Jake Bailey (jakebailey) wants to merge 172 commits into
Conversation
|
TypeScript Bot (@typescript-bot) test it |
|
Hey Jake Bailey (@jakebailey), the results of running the DT tests are ready. There were interesting changes: Branch only errors:
|
|
Jake Bailey (@jakebailey) Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Well, that's a little bleak. |
7eda73a to
1846356
Compare
|
Not sure why top400 didn't come back either |
Checker results should not depend on which files were checked first, or on which types an editor happened to ask for before diagnostics were requested. Tests had no way to show such dependence, so fixes for it could only be described, not demonstrated. The harness-only checkOrder option queries the checker in a different order: files in reverse, each file in its own program, or every expression's type and symbol before any diagnostics. Listing several orders produces a baseline for each, so order dependence shows up as differing baselines.
The checkOrder option only tries a few fixed orders over whole files, but results can depend on which single query a checker answers first, as when an editor asks for the type of one node before anything else is checked. Such dependence is hard to find by hand and easy to reintroduce. With TS_CHECK_ORDER_FUZZ=n, each test is checked again n times in fresh programs that first answer a seeded random selection of type, symbol, and diagnostics queries. Any change in diagnostics or in the type or symbol of a node fails the test with the seed and a minimized list of queries that reproduces it. The fuzzer is off by default, so it does not slow the normal suite.
Asking the checker for types and symbols before requesting diagnostics changes the diagnostics and types reported for these tests. Recording them in that order too shows which later changes make the results independent of what was asked first.
These tests exercise checker results that can depend on the order in which declarations, files, and relations are checked: variance measurement cycles, relation caching under recursion and depth limits, instantiation limits, contextual re-checks, and deferred type reference cycles. Recording the current baselines makes later changes to make checking order independent show up as reviewable baseline diffs.
Variance measured through a cycle of recursive references used to depend on which type in the cycle was measured first, so reordering declarations or assignments changed which assignments were errors. These cases cover cycles through interfaces, type aliases, callbacks, and nested compositions, alongside annotated and structurally measured types whose results should not change.
The errors for a getter and setter that disagree on abstractness or accessibility were reported on both names by whichever accessor was checked first, guarded by a flag on the getter. When that happened during a query rather than a full check, the flag was set without the errors being reported. Since the errors only depend on the modifiers, each accessor now reports the error on its own name with no state, which yields the same set of errors in any order.
The type parameters of a type alias are resolved along with its declared type, but some readers took them from the links without resolving the declared type first, relying on it having been resolved earlier. Reading them through an accessor that resolves the declared type makes the dependency explicit, so the result does not depend on what was resolved before.
checkAssertion runs in several check modes, such as when a call is first checked without its context sensitive arguments, and each run overwrote the operand type kept for the deferred comparability check. That check used whichever was stored last, so its result depended on the order in which the assertion was checked. The deferred check now uses the cached normal type of the operand. The assertion itself only needs the operand type for `as const`, and the operand of a non-const assertion is contextually typed by the asserted type alone, so in any check mode the assertion computes the cached type the deferred check will read, with inference for an enclosing call hidden from the operand. This is the result the deferred check would produce on its own, and checks each operand once rather than again with fresh flow analysis in the deferred check.
Nothing reads or writes it.
The type of a destructured parameter's binding elements depended on
whether the parameter's type happened to be cached already. For
example, in non-strict mode
(({ [a()?.d]: c = "" }) => {})();
reported TS2537 when 'c' was queried before the IIFE call resolved
the parameter (its declared type is the undefined widening type),
but not otherwise. Conversely, in
f7({ a: {} });
function f7({ a: { x = 0, y = 0 } } = { a: {} }) {}
calling f7 first made 'x' and 'y' errors of type any.
Make the choice depend only on the declaration. Parameters of
context-sensitive functions take their type from contextual
assignment, so ask for the parameter's type. Everything else uses
the type derived from the declaration, as it did when nothing was
cached. Asking for the parameter's type unconditionally instead
introduces circularities in patterns whose initializers reference
sibling elements.
Asking for the type of a context-sensitive function's parameter can
resolve the enclosing call, which assigns the parameter its
contextual type from the inference in progress. The first query
nevertheless returned the type it computed itself from the final
signature, while every later query returned the assigned type. In
declare function fun<T>(g: (x: T) => T, h: (y: T) => T, x: T): T;
fun((Math.random() < 0.5 ? (x => x) : (x => undefined)), x => x, 10);
the last 'x' was number when the call was checked first, but any
when 'x' was queried first and its result was cached.
Return the assigned type whenever one exists, so that the first
query agrees with the rest, as the original comment intended.
When resolving a function expression's members reaches the function again through its enclosing call, the contextual check sees an empty signature list and gives up, yet it left the node marked as context checked. Whichever query got there first therefore decided whether the function's parameters were ever contextually typed, and diagnostics such as implicit any errors could be lost depending on check order. Leave the node unchecked in that case so a later request performs the check, and have the deferred function check request it, so that the check is performed at least once whichever query reached it first.
Generic calls nested in the arguments of other generic calls, or in context sensitive functions passed to them, depend on the enclosing call for their contextual type and parameter types, and the enclosing call infers from them in turn. When the inner call was queried first, it was already marked as resolving by the time its contextual type resolved the enclosing call, so the enclosing inference saw a placeholder and both calls cached results that a check in source order never produces. Computing the contextual type of a call nested in an argument of an enclosing call before resolving it resolves the enclosing call first, as source order does. Other contexts are not resolved ahead of time, since computing them may evaluate expressions that checking does not, such as the target of a destructuring assignment.
Asking for the symbol of a property access that hits an index signature stored the index symbol as the access's resolved symbol. Checking a delete expression reads that link to decide whether the operand must be optional, so a prior symbol query turned `delete a.x` on an index signature into an error that a plain check does not report. The type of the index symbol at such an access, which hovers show, is found by comparing it to the symbol the access resolved to, so that comparison now looks the index symbol up again rather than reading it from links.
A context-sensitive function's signature takes the type parameters of
a generic contextual signature, and its parameters their types, when
the function is contextually checked. Anything reading the signature
before then saw it incomplete and cached what it derived from it. Type
aliases in the body read the type parameters as outer type parameters,
so querying such an alias first resolved its conditional types eagerly.
In
declare function f(fun: <T>(t: T) => void): void
f(function self(t) {
const s = self;
type P = Parameters<typeof s>;
});
P was [t: unknown] when the call was checked first, but [t: T] when P
was asked for first, since s then had the uncontextual signature.
Split out assigning the contextual parameter types, which completes
the signature, from the rest of the contextual check, and have every
way to obtain the signature request it: the type of the function's
symbol, before anything resolves its members, and the signature of its
declaration. The rest of the check, inferring the return type and
checking the signature, remains for checking the function itself.
Contextual typing can reach the enclosing call while the signature is
being created, so keep the signature created there rather than replace
the one that was contextually typed. This also makes it unnecessary to
leave a function unchecked when its members are found being resolved.
A single signature type produced by higher order inference is instantiated with the full mapper combined in, since its signature can reference type parameters other than the outer type parameters of its declaration. The instantiation cache was keyed only by the mapped outer type parameters, so two instantiations differing in another type parameter (here the this type) shared whichever result was computed first. Which property access was checked first therefore decided the type of both, and checking `outer.log` gave it the type of `inner.log` contrary to the test's expectation.
Explaining why a name failed to resolve looks up other symbols, for example to find a spelling suggestion, and that can resolve aliases. When the failed name was the target of an alias, the suggestion lookup re-entered the alias being resolved, or skipped it, depending on which aliases were already in progress. In declarationEmitUnknownImport that decided whether two "Circular definition of import alias" errors were reported: they appeared when the import was checked first, and not when the export specifier was resolved first. Defer the explanation until no resolution is in progress, so it only observes finished results. The circularity errors that were artifacts of the suggestion lookup are no longer reported.
When one reference hit the flow analysis depth limit, flow analysis was disabled for every later reference in the same function or module body. The type of a reference then depended on whether another reference in the body had been analyzed before it. Each reference now gives up only when its own analysis hits the limit, so its type is the one it would get if it were analyzed first.
The linter prefers slices.Backward over an index loop counting down.
|
Jake Bailey (@jakebailey) Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
This is a port of Anders Hejlsberg's "Allow accessing named declared members during a type's resolution", which is expected to land in main first, and replaces this branch's earlier approach to the same problem. Resolving the members of a class or interface reference resolves those of its base types with this bound to the reference, and a this-dependent member of a base may need a named member of the reference itself. Previously this branch installed a layout of only the declared members while inherited ones were resolved, then replaced it with the full layout, so a member read during resolution could be a different symbol from the final one. Instead, as in that PR, the type is pushed on a stack of types whose layout is being built, and a named property lookup on it finds the declared member instantiated by its mapper, or else the member of a base type. Members found that way are written into the property slots of the final layout, so a property is the same symbol however it is read. Nothing partial is ever installed on the type. The lookup also observes the resolution in progress, so results derived from it are discarded with the cycle like any other. Also as in that PR, class and interface types are no longer treated as string index signature only types when simplifying indexed accesses, as whether they are depends on whether their members were resolved yet. keyofGenericExtendingClassDoubleLayer now reports the infinitely circular instantiation error at both places that reach it, where main reports it once and Strada not at all: keyof needs every property, which a named lookup cannot provide. This may be worth revisiting.
The flag marked members whose resolution was in progress, so that the fast path would not read members published before they were complete. That duplicates what the resolution stack already records: the resolution of the members is in Checker.resolving, or shadowed by a cycle yield. The flag also let a type carry a "resolved" bit for members that were not yet final, which is the install-then-replace pattern Anders Hejlsberg objected to in the review of "Allow accessing named declared members during a type's resolution". ObjectFlagsMembersResolved now means only that the outermost resolution of the members completed. Whether partial members are available to a computation re-entering the resolution is recorded on the resolution itself (membersPublished), and a re-entry finds it even when hidden by a cycle yield. Members are marked published before their properties are listed, since listing them may re-enter the resolution, which then sees the members as it did before. Types created with their members, rather than resolved, still set the flag directly through setStructuredTypeMembers; resolvers use setStructuredTypeData or publishStructuredTypeMembers.
c7ee0b9 to
c501b1f
Compare
|
TypeScript Bot (@typescript-bot) test it |
|
Hey Jake Bailey (@jakebailey), the results of running the DT tests are ready. There were interesting changes: Branch only errors:
|
|
Jake Bailey (@jakebailey) Here are the results of running the user tests with tsc comparing Something interesting changed - please have a look. Details
|
|
Jake Bailey (@jakebailey) Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Jake Bailey (@jakebailey) Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
Reduced from the react-relay DefinitelyTyped tests. On main, the variances of these mutually recursive generics are measured while the cycle is still open and then frozen, so every parameter of EntryPointComponent ends up "independent". Main therefore accepts the assignment of `e` to `EntryPoint<any, any>` while rejecting the assignment of `e.root` to that same type's `root` property. The checker now reports both assignments, which is the consistent answer. Recording the case keeps that behavior covered.
A function with an inferred return type that calls itself inside a callback passed to a non-generic function forms a return type cycle through the callback. TypeScript reports TS7023 on the function and TS7024 on the callback, and without noImplicitAny the cycle resolves both to any. The baselines record this branch's current output, which misses the cycle: there are no errors under strict, and the non-strict case reports a spurious TS2322 (seen in OmniRoute).
Contextually checking a function expression infers its return type from its body. Since "Discard results derived from computations in progress", 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 relates the function expression's signature, which requests its return type. Seeing it in progress, that request returned any without detecting a cycle, so the enclosing function's return type was inferred from a provisional any instead of reporting the circularity. 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 as before. The result is cached only if no request has already cached one, so whichever request finishes first decides it, the same as when the return type is requested directly.
Inferring a method's return type as a union of the class's this type and an interface relates the class to the interface, which needs the return types of the other methods. When one of those calls back into the first method, the return types form a cycle, and which members report it depends on which return type is requested first. TypeScript reports the cycle on both methods when the class declaration is checked before its use. When the class is first related to the interface, the method calling back is computed first, sees the other in progress, and both are inferred without error. This branch discards results that observed a computation in progress, so the latter recomputes from the other entry and reports the cycle (seen in effect's SchemaAST).
In JavaScript, a @type tag on a function expression gives the types of its parameters, even where the function also has a contextual type. The baselines record this branch's current output, which types the parameters from the contextual type instead (seen in webpack).
Completing contextual signatures eagerly 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. Prefer the tag's signature, as before, and fall back to the declaration's signature otherwise.
Inferring from string[][] to Iterable<Iterable<T>> passes through the [Symbol.iterator] method types of the outer and inner arrays. Ordering those by type id calls the inner method type newer, and so the inference expanding recursion, only when it is first created during the inference; iterating string[] beforehand avoids that. Ordering by generation infers the element type in both cases (seen in webpack).
This reverts commit f871455. Computing the contextual type of a call nested in an argument of an enclosing call does more than resolve the enclosing call: it walks the contextual types between them, such as the contextual return type of an arrow function the nested call is returned from. When the enclosing call is already resolving, that walk still decides where a cycle through the two calls is entered. With it skipped, a cycle through a scope object whose members are thunks referring to the scope was entered at the nested call rather than at the thunk's return type, as in source order, and parameters of callbacks passed to the nested call became implicitly any (seen in arktype).
A scope object built by a generic call whose parameter is a mapped type over the object, with a member that is a thunk calling back into the scope, checks without error in the reference implementation under every order tried. There, the thunk's return type is computed once, while the scope's reverse mapping of the thunk is in progress, which cuts off the nested resolution of the scope call, and is cached. Here it is computed again after the scope call's nested resolution is discarded, outside that reverse mapping, and fails as a cycle through the scope. The baseline records the current, incorrect, circularity errors. The case is reduced by hand from a pattern in arktype, which itself checks without error.
The variable is narrowed to never inside the if. With && in the loop condition, main reports only the assignment of false to never: loop analysis creates an incomplete never placeholder (silentNever), and that placeholder leaks into the type of the reference, so the property access is silently allowed. With a plain loop condition, main reports the property access instead, which is what we report in both cases. Reduced with creduce (seen in uglify-js).
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 what that one 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).
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 what was 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 what that computation instantiated, which does not depend on where it was entered, so its messages keep the order in which the types recur.
Every computation with effects keeps a memoEffects until a check replays it, and when nothing checks again, such as when emitting after checking, those of the whole program stay live. Few have reference marks or cycle heads, so moving them to a side struct shrinks every node from 128 to 80 bytes (seen in angular, about 60MB less).
linkState wrapped the effects of the computation that cached a result in a set of deps even when it was the only one, though replaying a set holding only f replays f the same way. The state now points at f until a second computation caches the result, which then gets a set rather than adding to the effects of f, shared with everything else depending on it (seen in angular, about 30MB less).
Most results with a memo state only have pending effects, and every state stays live until a check reads it, so storing the rare errors list behind a pointer halves the state (seen in angular, about 20MB less).
A computation with no effects of its own that observed only one result computed at the same node replays exactly what that result does, so the result keeps those effects as its own rather than a node holding one edge to them (seen in angular, about 10MB less).
resolvingIndex scanned every in-progress entry of a memo map to find a key, which is quadratic when many entries of one map are in progress at once, as in deeply nested variance measurements. Past a small number of in-progress entries, count them by key so that a lookup for a key that is not in progress is constant time. (seen in nested variance stress)
When a variance measurement re-entered one already in progress, the inner measurement restarted from the canonical (smallest) type of the cycle with the stack hidden. With cycles nested in cycles, each restart re-measured every type still in progress, so the work multiplied with the nesting depth. Now re-entry instead records the lowest stack index it reached, as in Tarjan's algorithm, and returns provisional variances. When a measurement finishes as the root of its strongly connected component and the component's canonical type is not the root, what was cached since the root began is discarded and the canonical type is measured once with the stack hidden; the root then adopts its result. The result is what main computes when the canonical type is checked first, and each component is measured at most about twice. The elaboration in varianceCircularConsistency now goes through the 'queries' property again, matching main and TypeScript 6.0. (seen in nested variance stress)
There's too much here to explain, but at a high level, this is a massive change which aims to make the checker entirely order independent, that is, it doesn't matter when you call the checker, which nodes you walk first, if you do or do not pull something, check some other file, helicopter into the wrong part of a cycle, etc, the behavior, diagnostics, everything, will be the same.
Looking at individual commits is my suggested way to look inside, not the entire diff, which will surely crash your browser.
This is all AI driven, so it's a bit hard to unbox (I plan to break this apart since some stuff can be done early), but some interesting tidbits (handwritten by me!)
There's a new
checkOrdertest option, which makes certain tests variant on interesting orderings, like "what if we visit every single node before checking" or "what if we reverse the order of the files" or even "what if we use PRNG to randomly walk nodes and then check". This up front found on the order of 500 failing tests we already had that were secretly order dependent. These were all fuzzed out and kept.There are a significant number of new tests which all show various ordering bugs never before discovered.
The first chunk of the PR are fixing places where code peeked at caches that "happened to be filled" instead of calling the actual function to pull on them.
Then, all of the ID-using code gets removed. At the end everything now has an ID independent ordering based on content, flags, the works. This caught a bunch of places where we were not caching things and ending up with types that were by all rights identical but had different IDs and then were unstable.
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.
Also, this is really meant to be rebased, so expect this all to be nasty nasty force pushes all the time as this gets tweaked. Unfortunately right now that's like 125+ commits, so that sucks.
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.