Repository navigation
[experiment] fully order independent type checking - #65064
Jake Bailey (jakebailey) wants to merge 189 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.
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)
When a computation re-entered one in progress, memoCycle made every computation above it depend on it, stopping only at one that already depended on that same head. With cycles nested in cycles, re-entries reach many different heads, so each walked most of the stack and every computation gained a dependency per head. A computation in progress above a cycle head becomes part of the head's effects, so depending on the lowest head covers every head above it. Record the head's index with each dependency, add one only for a lower head than the computation already depends on, and stop the walk at the first computation that depends on this head or one below it. (seen in nested variance stress)
Relating type references and aliases now asks whether the target's variances are being measured, which scanned the whole variance stack on every comparison made during a measurement. Index the stack by symbol, restoring a symbol's previous index when its measurement ends, so that a measurement hidden with the rest of the stack is still not found. (seen in nested variance stress)
isTypeRelatedTo reads the cached result for the relation key before it starts a comparison, and beginTop read it again in case a comparison made in between cached one. Most comparisons cache nothing in between, so memoMap now counts its writes, and beginTop reads the result again only if the relation was written since the caller read it. This avoids a map lookup per comparison that reaches recursiveTypeRelatedTo. (seen in VS Code check time)
|
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: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Every statement of a run of unreachable statements reports the whole run, so each one found the run by scanning its neighbors. Checking a run of n statements thus took quadratic time: 20000 unreachable statements took 2.85s to check, against 0.22s when the runs of a block are found together and cached. (seen in review)
A result cached while a cycle is in progress may be computed from an incomplete result of the cycle, so it must be discarded when the cycle ends. The exports of a module, the module of the implicit JSX import, and the type of an instantiation expression were kept in plain fields whose writes were not logged for discard, so they kept whatever the first order of computing them produced. Entries interned by pointer own cached results like links do, but the check that cached state is memoized did not look at them, which is how the instantiation expression slipped through. It now does. The loop fixed point it then finds is kept on purpose: analyzing the loop again after a cycle starts from another reference and can reach another fixed point, which made controlFlowIterationErrors depend on order. (seen in review)
The reachability of the last flow node asked about is kept outside any memo, to make walking back through a run of statements linear. A result kept while a cycle is in progress may come from an incomplete result that the cycle later discards, and reusing it while effects are being recorded leaves the reuse out of them. It is now kept only when nothing can discard it, and reused only when reading need not be accounted for. (seen in review)
|
Jake Bailey (@jakebailey) Here are the results of running the top 400 repos with tsc comparing Something interesting changed - please have a look. Details
|
A result cached before its computation had effects is attached to them later as if it had been attached when cached, so the link is not logged for discard. Only an ordinary cycle was set aside for that, so a variance measurement re-entered since then still logged it, and ending the measurement dropped a dependency of a result that it never discards, losing effects that reusing the result should replay. (seen in review)
The documentation of beginMemo sat above readsWithoutMemo, reading as part of it. Type.generation was described as an allocation counter, but it is derived from the types a type is built from, all fixed when it is created, which is why it is order-independent. (seen in review)
getIntrinsicTagSymbol records whether an intrinsic tag resolved by name or by index signature in jsxFlags, but wrote the bits directly. When the tag's resolved symbol was discarded at the end of a cycle, the flags stayed set, so a later resolution could see bits from a discarded attempt. Write them as a flag memo of the resolved symbol so both are discarded together. (seen in review)
Editors ask the checker for contextual types and resolved signatures as well as types and symbols, and both reach parts of the checker that types and symbols alone do not, such as contextual typing of arguments whose call has not been resolved. The check order fuzzer and the typesfirst order now ask for them too, so that results which depend on those queries being answered first are found. (seen in review)
When the type of an expression inside the argument is asked for first, the inferred tuple becomes readonly, though the binding pattern the call result is destructured into makes it mutable when checked in program order. (seen in check order fuzzing)
Contextually checking a function sets a flag first, then infers the return type. If a cycle is detected in between (for example a call inside the body re-entering the outer resolution), the flag is not logged but the return type is, so discarding the cycle drops the return type and keeps the flag. A later request then infers the return type without the contextual types, and a const assertion in the body becomes readonly when queried before its binding pattern. Clearing the flag on discard is not an option: it is TypeScript's guard against re-checking the function from within its own check. Instead, store the return type unlogged whenever the flag was, so both are kept or neither. (seen in check order fuzzing)
Unique symbol properties are named with the ID of their symbol, which depends on the order symbols were created in, so the same type printed differently after querying in another order. (seen in check order fuzzing)
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 what was cached from it is discarded.
(seen in check order fuzzing)
A computation started at no node, such as one a query outside of checking starts, reports errors at the current node wherever its result is reused. When a computation started at another node reused it, its effects joined that computation's without the node, so replaying them located the errors wherever that computation was later reused instead. Asking for the symbol of t.x and then the type of tx moved an excessive depth error from t.x to tx. Wrap such effects with the node they were observed at, when that is not where the enclosing computation started. (seen in check order fuzzing)
Editors ask for the types of names in type annotations too, which can compute results at different nodes than checking does. Both the typesfirst check order and check order fuzzing now ask for them. (seen in check order fuzzing)
Builds and editors emit files before checking them, and declaration emit asks the checker for results at nodes of its own. Fuzzing now emits single files among its queries and compares the emitted output too. (seen in check order fuzzing)
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.