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

Regression in variance measurements for aliased recursive types: not falling back to structural checks when measuring #33872

Closed
jack-williams opened this issue Oct 8, 2019 · 4 comments · Fixed by #35266
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@jack-williams
Copy link
Collaborator

jack-williams commented Oct 8, 2019

Version. 3.7-beta

Code

export interface CalcObj<O> {
    read: (origin: O) => CalcValue<O>;
}

export type CalcValue<O> = CalcObj<O>;

function foo<O>() {
    const unk: CalcObj<unknown> = { read: (origin: unknown) => unk }
    const x: CalcObj<O> = unk;
}

Expected behavior:

No error on the assignment to x.

Actual behavior:

Error. unknown is not assignable to O. The parameter O is being measured as invariant.

This works correctly on 3.6.3 and below. The usual tricks to avoid variance-based comparisons let the assignment pass.

Also, removing the type alias works too:

export interface CalcObj<O> {
    read: (origin: O) => CalcObj<O>;
}

function foo<O>() {
    const unk: CalcObj<unknown> = { read: (origin: unknown) => unk }
    const x: CalcObj<O> = unk; // fine!
}

Probably caused by #33050.

Playground Link: link

@jack-williams jack-williams changed the title Regression in variance measurements for recursive types. Regression in variance measurements for aliased recursive types. Oct 8, 2019
@jack-williams
Copy link
Collaborator Author

jack-williams commented Oct 9, 2019

The issue is that the flags for identifying types instantiated with markers are not being propagated fully. The comparison ends up doing a default covariant check on type arguments when it should be falling back to a structural check (because we're in the middle of a variance measurement). In this case:

function getAliasVariances(symbol: Symbol) {
    const links = getSymbolLinks(symbol);
    return getVariancesWorker(links.typeParameters, links, (_links, param, marker) => {
        const type = getTypeAliasInstantiation(symbol, instantiateTypes(links.typeParameters!, makeUnaryTypeMapper(param, marker)));
        type.aliasTypeArgumentsContainsMarker = true;
        return type;
    });
}

Getting the alias variance for CalcValue returns an object type (CalcObj) without the marker object flags set. This means the following check passes when it shouldn't:

 if (getObjectFlags(source) & ObjectFlags.Reference && getObjectFlags(target) & ObjectFlags.Reference && (<TypeReference>source).target === (<TypeReference>target).target &&
    !(getObjectFlags(source) & ObjectFlags.MarkerType || getObjectFlags(target) & ObjectFlags.MarkerType)) {
    // We have type references to the same generic type, and the type references are not marker
    // type references (which are intended by be compared structurally). Obtain the variance
    // information for the type parameters and relate the type arguments accordingly.
    const variances = getVariances((<TypeReference>source).target);
    const varianceResult = relateVariances(getTypeArguments(<TypeReference>source), getTypeArguments(<TypeReference>target), variances, isIntersectionConstituent);
    if (varianceResult !== undefined) {
        return varianceResult;
    }
}

Looking for type.aliasTypeArgumentsContainsMarker isn't sufficient because we can have union types that are later decomposed, with their constituents being lazily instantiated.

@jack-williams
Copy link
Collaborator Author

Smaller repro:

type A<T> = B<T>;

interface B<T> {
    prop: A<T>;
}

declare let a: A<number>;
declare let b: A<3>;
 
b = a; // error

@Jessidhia
Copy link

That looks like a correct error, though ((T=3) = (T=number)); a = b; ((T=number) = (T=3)) is the direction that I'd expect to be a bug if it errors.

@jack-williams
Copy link
Collaborator Author

The error is not correct because the type parameter is never materialised in the type, so the type-parameters of two instantiations of A are independent. The type A<T> is structurally equivalent to C for all T.

interface C {
    prop: C;
}

declare let a: A<number>;
declare let b: A<3>;
const c1: C = a;
const c2: C = b;
a = c1;
b = c2;

@jack-williams jack-williams changed the title Regression in variance measurements for aliased recursive types. Regression in variance measurements for aliased recursive types: not falling back to structural checks when measuring Oct 13, 2019
@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Oct 17, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.7.1 milestone Oct 17, 2019
@ahejlsberg ahejlsberg added Bug A bug in TypeScript and removed Needs Investigation This issue needs a team member to investigate its status. labels Nov 20, 2019
@ahejlsberg ahejlsberg added the Fix Available A PR has been opened for this issue label Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
4 participants