Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

investigate possible autofixes when a *_DepthLimit trace event is encountered #49

Open
typeholes opened this issue Jul 23, 2024 · 1 comment

Comments

@typeholes
Copy link
Collaborator

Just highlighting depth limits in the trace could be helpful to point out potential performance issues.

Automatic suggestions may also be possible. For example, LukeAbby on discord mentioned that adding a variance annotation helps when the depth limit is encountered in a variance check as it bails out and trusts the annotation when a cycle is detected.

@LukeAbby
Copy link

LukeAbby commented Jul 23, 2024

The point of interest here would likely be centered around:

function typeArgumentsRelatedTo(sources: readonly Type[] = emptyArray, targets: readonly Type[] = emptyArray, variances: readonly VarianceFlags[] = emptyArray, reportErrors: boolean, intersectionState: IntersectionState): Ternary {
            if (sources.length !== targets.length && relation === identityRelation) {
                return Ternary.False;
            }
            const length = sources.length <= targets.length ? sources.length : targets.length;
            let result = Ternary.True;
            for (let i = 0; i < length; i++) {
                // When variance information isn't available we default to covariance. This happens
                // in the process of computing variance information for recursive types and when
                // comparing 'this' type arguments.
                const varianceFlags = i < variances.length ? variances[i] : VarianceFlags.Covariant;
                const variance = varianceFlags & VarianceFlags.VarianceMask;
                // We ignore arguments for independent type parameters (because they're never witnessed).
                if (variance !== VarianceFlags.Independent) {
                    const s = sources[i];
                    const t = targets[i];
                    let related = Ternary.True;
                    if (varianceFlags & VarianceFlags.Unmeasurable) {
                        // Even an `Unmeasurable` variance works out without a structural check if the source and target are _identical_.
                        // We can't simply assume invariance, because `Unmeasurable` marks nonlinear relations, for example, a relation tained by
                        // the `-?` modifier in a mapped type (where, no matter how the inputs are related, the outputs still might not be)
                        related = relation === identityRelation ? isRelatedTo(s, t, RecursionFlags.Both, /*reportErrors*/ false) : compareTypesIdentical(s, t);
                    }
                    else if (variance === VarianceFlags.Covariant) {
                        related = isRelatedTo(s, t, RecursionFlags.Both, reportErrors, /*headMessage*/ undefined, intersectionState);
                    }
                    else if (variance === VarianceFlags.Contravariant) {
                        related = isRelatedTo(t, s, RecursionFlags.Both, reportErrors, /*headMessage*/ undefined, intersectionState);
                    }
                    else if (variance === VarianceFlags.Bivariant) {
                        // In the bivariant case we first compare contravariantly without reporting
                        // errors. Then, if that doesn't succeed, we compare covariantly with error
                        // reporting. Thus, error elaboration will be based on the the covariant check,
                        // which is generally easier to reason about.
                        related = isRelatedTo(t, s, RecursionFlags.Both, /*reportErrors*/ false);
                        if (!related) {
                            related = isRelatedTo(s, t, RecursionFlags.Both, reportErrors, /*headMessage*/ undefined, intersectionState);
                        }
                    }
                    else {
                        // In the invariant case we first compare covariantly, and only when that
                        // succeeds do we proceed to compare contravariantly. Thus, error elaboration
                        // will typically be based on the covariant check.
                        related = isRelatedTo(s, t, RecursionFlags.Both, reportErrors, /*headMessage*/ undefined, intersectionState);
                        if (related) {
                            related &= isRelatedTo(t, s, RecursionFlags.Both, reportErrors, /*headMessage*/ undefined, intersectionState);
                        }
                    }
                    if (!related) {
                        return Ternary.False;
                    }
                    result &= related;
                }
            }
            return result;
        }

This is currently at line 22,575 in src/compiler/checker.ts which I can't permalink because GitHub can't display files that large.

It's been a few months since I last worked on a problem related to this but if I had to approximately sum up the apparent explanation for the issue:

  1. A user provides a bound that (whether obviously or not) trips a pathological bound. To the user this may seem as obvious as something like proving that for class SubClass<T> extends BaseClass<T> {} that SubClass<T> extends BaseClass<any>.
  2. There's some fundamental circularity (e.g. this or other self references in a type) that causes the variance check for BaseClass to recurse indefinitely. I have reported several of these as bugs though some were refused as not a defect though I have a few I'm still trying to distill.
  3. As you can see in that code, TypeScript then simply assumes BaseClass is covariant and says the constraint SubClass<T> extends BaseClass<any> is true. This may theoretically lead to unsoundness if BaseClass is in truth contravariant or invariant but isn't too bad of a performance cost and seems to rarely cause unsoundness in practice.
  4. Something seemingly completely unrelated figuratively sparks the flames. This is the hardest part for me to focus on reproducing but it's usually been rather innocuous things, like a Readonly modifier on a parameter in a subclass that sparks the flame.

I'd have to dig up some examples that are definitely variance related. The one I easily have on hand is this but since I haven't run it through a debugger or anything I can only say that it smells like a variance detection issue based upon the particulars of the circularity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants