Always perform structural comparison when variance check fails

Author: ahejlsberg

src/compiler/checker.ts

@@ -282,9 +282,9 @@ namespace ts {
         const noConstraintType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);
         const circularConstraintType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);
 
-        const markerSuperType = createType(TypeFlags.MarkerType);
-        const markerSubType = createType(TypeFlags.MarkerType);
-        const markerOtherType = createType(TypeFlags.MarkerType);
+        const markerSuperType = <TypeParameter>createType(TypeFlags.TypeParameter);
+        const markerSubType = <TypeParameter>createType(TypeFlags.TypeParameter);
+        markerSubType.constraint = markerSuperType;
 
         const anySignature = createSignature(undefined, undefined, undefined, emptyArray, anyType, /*typePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false);
         const unknownSignature = createSignature(undefined, undefined, undefined, emptyArray, unknownType, /*typePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasLiteralTypes*/ false);
@@ -8948,10 +8948,6 @@ namespace ts {
 
                 if (isSimpleTypeRelatedTo(source, target, relation, reportErrors ? reportError : undefined)) return Ternary.True;
 
-                if (source.flags & TypeFlags.MarkerType && target.flags & TypeFlags.MarkerType && !(source.flags & TypeFlags.Object || target.flags & TypeFlags.Object)) {
-                    return source === markerSubType && target === markerSuperType ? Ternary.True : Ternary.False;
-                }
-
                 if (getObjectFlags(source) & ObjectFlags.ObjectLiteral && source.flags & TypeFlags.FreshLiteral) {
                     if (hasExcessProperties(<FreshObjectLiteralType>source, target, reportErrors)) {
                         if (reportErrors) {
@@ -9219,41 +9215,15 @@ namespace ts {
                     // in the process of computing variance information for recursive types and when
                     // comparing 'this' type arguments.
                     const variance = i < variances.length ? variances[i] : Variance.Covariant;
-                    // We simply ignore omnivariant type arguments (because they're never witnessed).
-                    if (variance !== Variance.Omnivariant) {
-                        const s = sources[i];
-                        const t = targets[i];
-                        let related = Ternary.True;
-                        if (variance === Variance.Covariant) {
-                            related = isRelatedTo(s, t, reportErrors);
-                        }
-                        else if (variance === Variance.Contravariant) {
-                            related = isRelatedTo(t, s, reportErrors);
-                        }
-                        else if (variance === Variance.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, /*reportErrors*/ false);
-                            if (!related) {
-                                related = isRelatedTo(s, t, reportErrors);
-                            }
-                        }
-                        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, reportErrors);
-                            if (related) {
-                                related &= isRelatedTo(t, s, reportErrors);
-                            }
-                        }
-                        if (!related) {
-                            return Ternary.False;
-                        }
-                        result &= related;
+                    const s = sources[i];
+                    const t = targets[i];
+                    const related = variance === Variance.Covariant ? isRelatedTo(s, t, reportErrors) :
+                        variance === Variance.Contravariant ? isRelatedTo(t, s, reportErrors) :
+                        Ternary.False;
+                    if (!related) {
+                        return Ternary.False;
                     }
+                    result &= related;
                 }
                 return result;
             }
@@ -9418,21 +9388,14 @@ namespace ts {
                         !(source.flags & TypeFlags.MarkerType || target.flags & TypeFlags.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.
+                        // information for the type parameters and relate the type arguments accordingly. If we do
+                        // not succeed, fall through and do a structural comparison instead (there are instances
+                        // where the variance information isn't accurate, e.g. when type parameters are used only
+                        // in bivariant positions or when a type argument is 'any' or 'void'.)
                         const variances = getVariances((<TypeReference>source).target);
                         if (result = typeArgumentsRelatedTo(<TypeReference>source, <TypeReference>target, variances, reportErrors)) {
                             return result;
                         }
-                        // The type arguments did not relate appropriately, but it may be because getVariances was
-                        // invoked recursively and returned emptyArray (in which case typeArgumentsRelatedTo defaulted
-                        // to covariance for all type arguments). It might also be the case that the target type has a
-                        // 'void' type argument for a covariant type parameter that is only used in return positions
-                        // within the generic type (in which case any type argument is permitted on the source side).
-                        // In those cases we proceed with a structural comparison. Otherwise, we know for certain the
-                        // instantiations aren't related and we can return here.
-                        if (variances !== emptyArray && !hasCovariantVoidArgument(<TypeReference>target, variances)) {
-                            return Ternary.False;
-                        }
                     }
                     // Even if relationship doesn't hold for unions, intersections, or generic type references,
                     // it may hold in a structural comparison.
@@ -9851,13 +9814,18 @@ namespace ts {
             return result;
         }
 
-        // Return an array containing the variance of each type parameter. The variance is effectively
-        // a digest of the type comparisons that occur for each type argument when instantiations of the
-        // generic type are structurally compared. We infer the variance information by comparing
-        // instantiations of the generic type for type arguments with known relations. Note that the
-        // function returns the emptyArray singleton to signal that it has been invoked recursively for
-        // the given generic type.
+        // Return an array containing the variance of each type parameter. The variance information is
+        // computed by comparing instantiations of the generic type for type arguments with known relations.
+        // A type parameter is marked as covariant if a covariant comparison succeeds; otherwise, it is
+        // marked contravariant if a contravarint comparison succeeds; otherwise, it is marked invariant.
+        // One form of variance doesn't exclude another, so this information simply serves to indicate
+        // a "primary" relationship that can be checked as an optimization ahead of a full structural
+        // comparison. The function returns the emptyArray singleton if we're not in strictFunctionTypes
+        // mode or if the function has been invoked recursively for the given generic type.
         function getVariances(type: GenericType): Variance[] {
+            if (!strictFunctionTypes) {
+                return emptyArray;
+            }
             const typeParameters = type.typeParameters || emptyArray;
             let variances = type.variances;
             if (!variances) {
@@ -9870,20 +9838,14 @@ namespace ts {
                     type.variances = emptyArray;
                     variances = [];
                     for (const tp of typeParameters) {
-                        // We first compare instantiations where the type parameter is replaced with
-                        // marker types that have a known subtype relationship. From this we can infer
-                        // invariance, covariance, contravariance or bivariance.
+                        // We compare instantiations where the type parameter is replaced with marker types
+                        // that have a known subtype relationship. From this we infer covariance, contravariance
+                        // or invariance.
                         const typeWithSuper = getMarkerTypeReference(type, tp, markerSuperType);
                         const typeWithSub = getMarkerTypeReference(type, tp, markerSubType);
-                        let variance = (isTypeAssignableTo(typeWithSub, typeWithSuper) ? Variance.Covariant : 0) |
-                            (isTypeAssignableTo(typeWithSuper, typeWithSub) ? Variance.Contravariant : 0);
-                        // If the instantiations appear to be related bivariantly, it may be because the
-                        // type parameter is omnivariant (i.e. it isn't witnessed anywhere in the generic
-                        // type). To determine this we compare instantiations where the type parameter is
-                        // replaced with marker types that are known to be unrelated.
-                        if (variance === Variance.Bivariant && isTypeAssignableTo(getMarkerTypeReference(type, tp, markerOtherType), typeWithSuper)) {
-                            variance = Variance.Omnivariant;
-                        }
+                        const variance = isTypeAssignableTo(typeWithSub, typeWithSuper) ? Variance.Covariant :
+                            isTypeAssignableTo(typeWithSuper, typeWithSub) ? Variance.Contravariant :
+                            Variance.Invariant;
                         variances.push(variance);
                     }
                 }
@@ -9892,17 +9854,6 @@ namespace ts {
             return variances;
         }
 
-        // Return true if the given type reference has a 'void' type argument for a covariant type parameter.
-        // See comment at call in recursiveTypeRelatedTo for when this case matters.
-        function hasCovariantVoidArgument(type: TypeReference, variances: Variance[]): boolean {
-            for (let i = 0; i < variances.length; i++) {
-                if (variances[i] === Variance.Covariant && type.typeArguments[i].flags & TypeFlags.Void) {
-                    return true;
-                }
-            }
-            return false;
-        }
-
         function isUnconstrainedTypeParameter(type: Type) {
             return type.flags & TypeFlags.TypeParameter && !getConstraintFromTypeParameter(<TypeParameter>type);
         }
@@ -10709,9 +10660,9 @@ namespace ts {
                     const sourceTypes = (<TypeReference>source).typeArguments || emptyArray;
                     const targetTypes = (<TypeReference>target).typeArguments || emptyArray;
                     const count = sourceTypes.length < targetTypes.length ? sourceTypes.length : targetTypes.length;
-                    const variances = strictFunctionTypes ? getVariances((<TypeReference>source).target) : undefined;
+                    const variances = getVariances((<TypeReference>source).target);
                     for (let i = 0; i < count; i++) {
-                        if (variances && i < variances.length && variances[i] === Variance.Contravariant) {
+                        if (i < variances.length && variances[i] === Variance.Contravariant) {
                             inferFromContravariantTypes(sourceTypes[i], targetTypes[i]);
                         }
                         else {

src/compiler/types.ts

@@ -3345,11 +3345,9 @@ namespace ts {
     }
 
     export const enum Variance {
-        Invariant     = 0,  // Both covariant and contravariant
+        Invariant     = 0,  // Neither covariant nor contravariant
         Covariant     = 1,  // Covariant
         Contravariant = 2,  // Contravariant
-        Bivariant     = 3,  // Either covariant or contravariant
-        Omnivariant   = 4   // Unwitnessed type parameter
     }
 
     // Generic class and interface types