From afc8a261ccc6b19873032f9f5adb47b2ce874eef Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Thu, 21 Sep 2017 21:31:11 -0700 Subject: [PATCH] Always perform structural comparison when variance check fails --- src/compiler/checker.ts | 117 ++++++++++++---------------------------- src/compiler/types.ts | 4 +- 2 files changed, 35 insertions(+), 86 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 574ce5c6d80..25dc3030556 100644 --- a/src/compiler/checker.ts +++ b/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 = createType(TypeFlags.TypeParameter); + const markerSubType = 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(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((source).target); if (result = typeArgumentsRelatedTo(source, 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(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(type); } @@ -10709,9 +10660,9 @@ namespace ts { const sourceTypes = (source).typeArguments || emptyArray; const targetTypes = (target).typeArguments || emptyArray; const count = sourceTypes.length < targetTypes.length ? sourceTypes.length : targetTypes.length; - const variances = strictFunctionTypes ? getVariances((source).target) : undefined; + const variances = getVariances((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 { diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 3f07716fd1b..8ab999032bc 100644 --- a/src/compiler/types.ts +++ b/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