From 023f2867727533d7c597d4a925c59878d4653e99 Mon Sep 17 00:00:00 2001 From: Mayank Patke Date: Fri, 31 Jan 2020 18:33:02 +0000 Subject: [PATCH] [dart2js] Update generic function subtyping. We now test bounds for mutual subtyping rather than structural equality up to renaming of bound type variables and equating all top types. Change-Id: I7dd23a3211a1631e463ea90c3173f3deae46ca23 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/134042 Commit-Queue: Mayank Patke Reviewed-by: Sigmund Cherem --- CHANGELOG.md | 8 +- pkg/compiler/lib/src/elements/types.dart | 123 ++---------------- sdk/lib/_internal/js_runtime/lib/rti.dart | 97 ++------------ .../lib/_internal/js_runtime/lib/rti.dart | 97 ++------------ 4 files changed, 38 insertions(+), 287 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f89f97a1ca3..7b383ccab21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,12 +46,16 @@ additional details see the [announcement]. enforced consistently in `ddc`. It will now be enforced in both. * JS interop classes with an index operator are now static errors. + [announcement]: https://github.com/dart-lang/sdk/issues/38994 + #### Dart2JS * JS interop classes with an index operator are now static errors instead of causing invalid code in dart2js. - - [announcement]: https://github.com/dart-lang/sdk/issues/38994 +* **Breaking Change**: The subtyping rule for generic functions is now more + forgiving. Corresponding type parameter bounds now only need to be mutual + subtypes rather than structurally equal up to renaming of bound type variables + and equating all top types. #### Linter diff --git a/pkg/compiler/lib/src/elements/types.dart b/pkg/compiler/lib/src/elements/types.dart index 1e67aace42b..efc2f70521b 100644 --- a/pkg/compiler/lib/src/elements/types.dart +++ b/pkg/compiler/lib/src/elements/types.dart @@ -82,21 +82,6 @@ abstract class DartType { bool _equals(DartType other, _Assumptions assumptions); - /// Returns `true` if `this` is structurally equal to [other] up to renaming - /// of bound type variables (trivially true) and equating all top types. - /// - /// This method handles the common cases of identity and top types. Types - /// should add any additional logic by implementing - /// [_equalsModuloTopInternal]. - bool _equalsModuloTop(DartType other, bool isLegacy) { - other = other.unaliased; - if (identical(this, other)) return true; - if (_isTop(isLegacy)) return other._isTop(isLegacy); - return _equalsModuloTopInternal(other, isLegacy); - } - - bool _equalsModuloTopInternal(DartType other, bool isLegacy); - @override String toString() => _DartTypeToStringVisitor().run(this); } @@ -194,14 +179,6 @@ class LegacyType extends DartType { bool _equalsInternal(LegacyType other, _Assumptions assumptions) => baseType._equals(other.baseType, assumptions); - - @override - bool _equalsModuloTopInternal(DartType other, bool isLegacy) { - if (other is LegacyType) { - return baseType._equalsModuloTop(other.baseType, isLegacy); - } - return false; - } } class NullableType extends DartType { @@ -243,14 +220,6 @@ class NullableType extends DartType { bool _equalsInternal(NullableType other, _Assumptions assumptions) => baseType._equals(other.baseType, assumptions); - - @override - bool _equalsModuloTopInternal(DartType other, bool isLegacy) { - if (other is NullableType) { - return baseType._equalsModuloTop(other.baseType, isLegacy); - } - return false; - } } class InterfaceType extends DartType { @@ -321,15 +290,6 @@ class InterfaceType extends DartType { return identical(element, other.element) && _equalTypes(typeArguments, other.typeArguments, assumptions); } - - @override - bool _equalsModuloTopInternal(DartType other, bool isLegacy) { - if (other is InterfaceType) { - return identical(element, other.element) && - _equalTypesModuloTop(typeArguments, other.typeArguments, isLegacy); - } - return false; - } } class TypedefType extends DartType { @@ -392,13 +352,6 @@ class TypedefType extends DartType { return identical(element, other.element) && _equalTypes(typeArguments, other.typeArguments, assumptions); } - - @override - bool _equalsModuloTop(DartType other, bool isLegacy) => - unaliased._equalsModuloTop(other, isLegacy); - - @override - bool _equalsModuloTopInternal(DartType other, bool isLegacy) => false; } class TypeVariableType extends DartType { @@ -434,14 +387,6 @@ class TypeVariableType extends DartType { } return false; } - - @override - bool _equalsModuloTopInternal(DartType other, bool legacy) { - if (other is TypeVariableType) { - return identical(other.element, element); - } - return false; - } } /// A type variable declared on a function type. @@ -491,14 +436,6 @@ class FunctionTypeVariable extends DartType { return false; } - @override - bool _equalsModuloTopInternal(DartType other, bool legacy) { - if (other is FunctionTypeVariable) { - return index == other.index; - } - return false; - } - @override R accept(DartTypeVisitor visitor, A argument) => visitor.visitFunctionTypeVariable(this, argument); @@ -519,9 +456,6 @@ class NeverType extends DartType { @override bool _equals(DartType other, _Assumptions assumptions) => identical(this, other); - - @override - bool _equalsModuloTopInternal(DartType other, bool isLegacy) => false; } class VoidType extends DartType { @@ -543,9 +477,6 @@ class VoidType extends DartType { bool _equals(DartType other, _Assumptions assumptions) { return identical(this, other); } - - @override - bool _equalsModuloTopInternal(DartType other, bool isLegacy) => false; } class DynamicType extends DartType { @@ -567,9 +498,6 @@ class DynamicType extends DartType { bool _equals(DartType other, _Assumptions assumptions) { return identical(this, other); } - - @override - bool _equalsModuloTopInternal(DartType other, bool isLegacy) => false; } class ErasedType extends DartType { @@ -590,9 +518,6 @@ class ErasedType extends DartType { @override bool _equals(DartType other, _Assumptions assumptions) => identical(this, other); - - @override - bool _equalsModuloTopInternal(DartType other, bool isLegacy) => false; } /// Represents a type which is simultaneously top and bottom. @@ -624,9 +549,6 @@ class AnyType extends DartType { @override bool _equals(DartType other, _Assumptions assumptions) => identical(this, other); - - @override - bool _equalsModuloTopInternal(DartType other, bool isLegacy) => false; } class FunctionType extends DartType { @@ -753,23 +675,6 @@ class FunctionType extends DartType { } return result; } - - @override - bool _equalsModuloTopInternal(DartType other, bool isLegacy) { - if (other is FunctionType) { - return returnType._equalsModuloTop(other.returnType, isLegacy) && - _equalTypesModuloTop( - parameterTypes, other.parameterTypes, isLegacy) && - _equalTypesModuloTop( - optionalParameterTypes, other.optionalParameterTypes, isLegacy) && - equalElements(namedParameters, other.namedParameters) && - _equalTypesModuloTop( - namedParameterTypes, other.namedParameterTypes, isLegacy) && - _equalTypesModuloTop( - typeVariableBounds, other.typeVariableBounds, isLegacy); - } - return false; - } } class FutureOrType extends DartType { @@ -812,14 +717,6 @@ class FutureOrType extends DartType { bool _equalsInternal(FutureOrType other, _Assumptions assumptions) { return typeArgument._equals(other.typeArgument, assumptions); } - - @override - bool _equalsModuloTopInternal(DartType other, bool isLegacy) { - if (other is FutureOrType) { - return typeArgument._equalsModuloTop(other.typeArgument, isLegacy); - } - return false; - } } bool _equalTypes(List a, List b, _Assumptions assumptions) { @@ -832,14 +729,6 @@ bool _equalTypes(List a, List b, _Assumptions assumptions) { return true; } -bool _equalTypesModuloTop(List a, List b, bool isLegacy) { - if (a.length != b.length) return false; - for (int i = 0; i < a.length; i++) { - if (!a[i]._equalsModuloTop(b[i], isLegacy)) return false; - } - return true; -} - abstract class DartTypeVisitor { const DartTypeVisitor(); @@ -1756,10 +1645,18 @@ abstract class DartTypes { if (s is FunctionType) { if (t.isGeneric) { if (!s.isGeneric) return false; - if (!_equalTypesModuloTop(s.typeVariableBounds, - t.typeVariableBounds, useLegacySubtyping)) { + List sBounds = s.typeVariableBounds; + List tBounds = t.typeVariableBounds; + int length = sBounds.length; + if (length != tBounds.length) { return false; } + for (int i = 0; i < length; i++) { + if (!_isSubtype(sBounds[i], sEnv, tBounds[i], tEnv) || + !_isSubtype(tBounds[i], tEnv, sBounds[i], sEnv)) { + return false; + } + } sEnv = sEnv.toSet()..addAll(s.typeVariables); tEnv = tEnv.toSet()..addAll(t.typeVariables); } diff --git a/sdk/lib/_internal/js_runtime/lib/rti.dart b/sdk/lib/_internal/js_runtime/lib/rti.dart index d1ab30f9347..82cb506a5d3 100644 --- a/sdk/lib/_internal/js_runtime/lib/rti.dart +++ b/sdk/lib/_internal/js_runtime/lib/rti.dart @@ -2505,7 +2505,17 @@ bool _isSubtype(universe, Rti s, sEnv, Rti t, tEnv, bool isLegacy) { var sBounds = Rti._getGenericFunctionBounds(s); var tBounds = Rti._getGenericFunctionBounds(t); - if (!typesEqual(sBounds, tBounds, isLegacy)) return false; + int sLength = _Utils.arrayLength(sBounds); + int tLength = _Utils.arrayLength(tBounds); + if (sLength != tLength) return false; + for (int i = 0; i < sLength; i++) { + var sBound = _Utils.arrayAt(sBounds, i); + var tBound = _Utils.arrayAt(tBounds, i); + if (!_isSubtype(universe, sBound, sEnv, tBound, tEnv, isLegacy) || + !_isSubtype(universe, tBound, tEnv, sBound, sEnv, isLegacy)) { + return false; + } + } sEnv = sEnv == null ? sBounds : _Utils.arrayConcat(sBounds, sEnv); tEnv = tEnv == null ? tBounds : _Utils.arrayConcat(tBounds, tEnv); @@ -2694,91 +2704,6 @@ bool _isInterfaceSubtype(universe, Rti s, sEnv, Rti t, tEnv, bool isLegacy) { return true; } -/// Types are equal if they are structurally equal up to renaming of bound type -/// variables and equating all top types. -/// -/// We ignore renaming of bound type variables because we operate on de Bruijn -/// indices, not names. -bool typeEqual(Rti s, Rti t, bool isLegacy) { - if (_Utils.isIdentical(s, t)) return true; - - if (isTopType(s, isLegacy)) return isTopType(t, isLegacy); - - int sKind = Rti._getKind(s); - int tKind = Rti._getKind(t); - if (sKind != tKind) return false; - - switch (sKind) { - case Rti.kindStar: - case Rti.kindQuestion: - case Rti.kindFutureOr: - return typeEqual(_castToRti(Rti._getPrimary(s)), - _castToRti(Rti._getPrimary(t)), isLegacy); - - case Rti.kindInterface: - if (Rti._getInterfaceName(s) != Rti._getInterfaceName(t)) return false; - return typesEqual(Rti._getInterfaceTypeArguments(s), - Rti._getInterfaceTypeArguments(t), isLegacy); - - case Rti.kindBinding: - return typeEqual( - Rti._getBindingBase(s), Rti._getBindingBase(t), isLegacy) && - typesEqual(Rti._getBindingArguments(s), Rti._getBindingArguments(t), - isLegacy); - - case Rti.kindFunction: - return typeEqual( - Rti._getReturnType(s), Rti._getReturnType(t), isLegacy) && - functionParametersEqual(Rti._getFunctionParameters(s), - Rti._getFunctionParameters(t), isLegacy); - - case Rti.kindGenericFunction: - return typeEqual(Rti._getGenericFunctionBase(s), - Rti._getGenericFunctionBase(t), isLegacy) && - typesEqual(Rti._getGenericFunctionBounds(s), - Rti._getGenericFunctionBounds(t), isLegacy); - - default: - return false; - } -} - -bool typesEqual(Object sArray, Object tArray, isLegacy) { - int sLength = _Utils.arrayLength(sArray); - int tLength = _Utils.arrayLength(tArray); - if (sLength != tLength) return false; - for (int i = 0; i < sLength; i++) { - if (!typeEqual(_castToRti(_Utils.arrayAt(sArray, i)), - _castToRti(_Utils.arrayAt(tArray, i)), isLegacy)) return false; - } - return true; -} - -bool namedTypesEqual(Object sArray, Object tArray, isLegacy) { - int sLength = _Utils.arrayLength(sArray); - int tLength = _Utils.arrayLength(tArray); - assert(sLength.isEven); - assert(tLength.isEven); - if (sLength != tLength) return false; - for (int i = 0; i < sLength; i += 2) { - if (_Utils.asString(_Utils.arrayAt(sArray, i)) != - _Utils.asString(_Utils.arrayAt(tArray, i))) return false; - if (!typeEqual(_castToRti(_Utils.arrayAt(sArray, i + 1)), - _castToRti(_Utils.arrayAt(tArray, i + 1)), isLegacy)) return false; - } - return true; -} - -// TODO(fishythefish): Support required named parameters. -bool functionParametersEqual(_FunctionParameters sParameters, - _FunctionParameters tParameters, isLegacy) => - typesEqual(_FunctionParameters._getRequiredPositional(sParameters), - _FunctionParameters._getRequiredPositional(tParameters), isLegacy) && - typesEqual(_FunctionParameters._getOptionalPositional(sParameters), - _FunctionParameters._getOptionalPositional(tParameters), isLegacy) && - namedTypesEqual(_FunctionParameters._getOptionalNamed(sParameters), - _FunctionParameters._getOptionalNamed(tParameters), isLegacy); - bool isLegacyTopType(Rti t) => isTopType(t, true); bool isNnbdTopType(Rti t) => isTopType(t, false); bool isTopType(Rti t, bool isLegacy) { diff --git a/sdk_nnbd/lib/_internal/js_runtime/lib/rti.dart b/sdk_nnbd/lib/_internal/js_runtime/lib/rti.dart index 8ee75ae6a30..b302eee901a 100644 --- a/sdk_nnbd/lib/_internal/js_runtime/lib/rti.dart +++ b/sdk_nnbd/lib/_internal/js_runtime/lib/rti.dart @@ -2505,7 +2505,17 @@ bool _isSubtype(universe, Rti s, sEnv, Rti t, tEnv, bool isLegacy) { var sBounds = Rti._getGenericFunctionBounds(s); var tBounds = Rti._getGenericFunctionBounds(t); - if (!typesEqual(sBounds, tBounds, isLegacy)) return false; + int sLength = _Utils.arrayLength(sBounds); + int tLength = _Utils.arrayLength(tBounds); + if (sLength != tLength) return false; + for (int i = 0; i < sLength; i++) { + var sBound = _Utils.arrayAt(sBounds, i); + var tBound = _Utils.arrayAt(tBounds, i); + if (!_isSubtype(universe, sBound, sEnv, tBound, tEnv, isLegacy) || + !_isSubtype(universe, tBound, tEnv, sBound, sEnv, isLegacy)) { + return false; + } + } sEnv = sEnv == null ? sBounds : _Utils.arrayConcat(sBounds, sEnv); tEnv = tEnv == null ? tBounds : _Utils.arrayConcat(tBounds, tEnv); @@ -2694,91 +2704,6 @@ bool _isInterfaceSubtype(universe, Rti s, sEnv, Rti t, tEnv, bool isLegacy) { return true; } -/// Types are equal if they are structurally equal up to renaming of bound type -/// variables and equating all top types. -/// -/// We ignore renaming of bound type variables because we operate on de Bruijn -/// indices, not names. -bool typeEqual(Rti s, Rti t, bool isLegacy) { - if (_Utils.isIdentical(s, t)) return true; - - if (isTopType(s, isLegacy)) return isTopType(t, isLegacy); - - int sKind = Rti._getKind(s); - int tKind = Rti._getKind(t); - if (sKind != tKind) return false; - - switch (sKind) { - case Rti.kindStar: - case Rti.kindQuestion: - case Rti.kindFutureOr: - return typeEqual(_castToRti(Rti._getPrimary(s)), - _castToRti(Rti._getPrimary(t)), isLegacy); - - case Rti.kindInterface: - if (Rti._getInterfaceName(s) != Rti._getInterfaceName(t)) return false; - return typesEqual(Rti._getInterfaceTypeArguments(s), - Rti._getInterfaceTypeArguments(t), isLegacy); - - case Rti.kindBinding: - return typeEqual( - Rti._getBindingBase(s), Rti._getBindingBase(t), isLegacy) && - typesEqual(Rti._getBindingArguments(s), Rti._getBindingArguments(t), - isLegacy); - - case Rti.kindFunction: - return typeEqual( - Rti._getReturnType(s), Rti._getReturnType(t), isLegacy) && - functionParametersEqual(Rti._getFunctionParameters(s), - Rti._getFunctionParameters(t), isLegacy); - - case Rti.kindGenericFunction: - return typeEqual(Rti._getGenericFunctionBase(s), - Rti._getGenericFunctionBase(t), isLegacy) && - typesEqual(Rti._getGenericFunctionBounds(s), - Rti._getGenericFunctionBounds(t), isLegacy); - - default: - return false; - } -} - -bool typesEqual(Object sArray, Object tArray, isLegacy) { - int sLength = _Utils.arrayLength(sArray); - int tLength = _Utils.arrayLength(tArray); - if (sLength != tLength) return false; - for (int i = 0; i < sLength; i++) { - if (!typeEqual(_castToRti(_Utils.arrayAt(sArray, i)), - _castToRti(_Utils.arrayAt(tArray, i)), isLegacy)) return false; - } - return true; -} - -bool namedTypesEqual(Object sArray, Object tArray, isLegacy) { - int sLength = _Utils.arrayLength(sArray); - int tLength = _Utils.arrayLength(tArray); - assert(sLength.isEven); - assert(tLength.isEven); - if (sLength != tLength) return false; - for (int i = 0; i < sLength; i += 2) { - if (_Utils.asString(_Utils.arrayAt(sArray, i)) != - _Utils.asString(_Utils.arrayAt(tArray, i))) return false; - if (!typeEqual(_castToRti(_Utils.arrayAt(sArray, i + 1)), - _castToRti(_Utils.arrayAt(tArray, i + 1)), isLegacy)) return false; - } - return true; -} - -// TODO(fishythefish): Support required named parameters. -bool functionParametersEqual(_FunctionParameters sParameters, - _FunctionParameters tParameters, isLegacy) => - typesEqual(_FunctionParameters._getRequiredPositional(sParameters), - _FunctionParameters._getRequiredPositional(tParameters), isLegacy) && - typesEqual(_FunctionParameters._getOptionalPositional(sParameters), - _FunctionParameters._getOptionalPositional(tParameters), isLegacy) && - namedTypesEqual(_FunctionParameters._getOptionalNamed(sParameters), - _FunctionParameters._getOptionalNamed(tParameters), isLegacy); - bool isLegacyTopType(Rti t) => isTopType(t, true); bool isNnbdTopType(Rti t) => isTopType(t, false); bool isTopType(Rti t, bool isLegacy) {