[dartdevc] Remove variance from type bounds in generic function subtypes

As I understand in early versions of strong mode generic function subtypes could
have contra-variance in the bounds of the type parameters. Now the spec states
they must be equal.

Fixes #36501

Tested and passing with the fixed language_2/generic_function_bounds_test
https://dart-review.googlesource.com/c/sdk/+/109726

Change-Id: Ie11d6318a542867e0541d81e2a18e5e25d3f0d9d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/110361
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Leaf Petersen <leafp@google.com>

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/types.dart

@@ -342,16 +342,16 @@ class FunctionType extends AbstractFunctionType {
   List metadata = [];
   String _stringValue;
 
-  /**
-   * Construct a function type.
-   *
-   * We eagerly normalize the argument types to avoid having to deal with
-   * this logic in multiple places.
-   *
-   * This code does best effort canonicalization.  It does not guarantee
-   * that all instances will share.
-   *
-   */
+  /// Construct a function type.
+  ///
+  /// We eagerly normalize the argument types to avoid having to deal with this
+  /// logic in multiple places.
+  ///
+  /// This code does best effort canonicalization.  It does not guarantee that
+  /// all instances will share.
+  ///
+  /// Note: Generic function subtype checks assume types have been canonicalized
+  /// when testing if type bounds are equal.
   static FunctionType create(returnType, List args, extra) {
     // Note that if extra is ever passed as an empty array
     // or an empty map, we can end up with semantically
@@ -516,12 +516,15 @@ class GenericFunctionType extends AbstractFunctionType {
     return _typeFormals = _typeFormalsFromFunction(_instantiateTypeParts);
   }
 
+  /// `true` if there are bounds on any of the generic type parameters.
+  get hasTypeBounds => _instantiateTypeBounds != null;
+
   /// Checks that [typeArgs] satisfies the upper bounds of the [typeFormals],
   /// and throws a [TypeError] if they do not.
   void checkBounds(List typeArgs) {
     // If we don't have explicit type parameter bounds, the bounds default to
     // a top type, so there's nothing to check here.
-    if (_instantiateTypeBounds == null) return;
+    if (!hasTypeBounds) return;
 
     var bounds = instantiateTypeBounds(typeArgs);
     var typeFormals = this.typeFormals;
@@ -537,8 +540,7 @@ class GenericFunctionType extends AbstractFunctionType {
   }
 
   List instantiateTypeBounds(List typeArgs) {
-    var boundsFn = _instantiateTypeBounds;
-    if (boundsFn == null) {
+    if (!hasTypeBounds) {
       // The Dart 1 spec says omitted type parameters have an upper bound of
       // Object. However Dart 2 uses `dynamic` for the purpose of instantiate to
       // bounds, so we use that here.
@@ -546,7 +548,7 @@ class GenericFunctionType extends AbstractFunctionType {
     }
     // Bounds can be recursive or depend on other type parameters, so we need to
     // apply type arguments and return the resulting bounds.
-    return JS('List', '#.apply(null, #)', boundsFn, typeArgs);
+    return JS('List', '#.apply(null, #)', _instantiateTypeBounds, typeArgs);
   }
 
   toString() {
@@ -948,24 +950,23 @@ bool _isSubtype(t1, t2) => JS('', '''(() => {
     // rather it uses JS function parameters to ensure correct binding.
     let fresh = $t2.typeFormals;
 
-    // TODO(nshahan) Remove this variance check. The types should be equal
-    // according to the spec and to match other backends.
-
-    // Check the bounds of the type parameters of g1 and g2.
-    // given a type parameter `T1 extends U1` from g1, and a type parameter
-    // `T2 extends U2` from g2, we must ensure that:
-    //
-    //      U2 <: U1
-    //
-    // (Note the reversal of direction -- type formal bounds are contravariant,
-    // similar to the function's formal parameter types).
-    //
-    let t1Bounds = $t1.instantiateTypeBounds(fresh);
-    let t2Bounds = $t2.instantiateTypeBounds(fresh);
-    // TODO(jmesserly): we could optimize for the common case of no bounds.
-    for (let i = 0; i < formalCount; i++) {
-      if (!$_isSubtype(t2Bounds[i], t1Bounds[i])) {
-        return false;
+    // Without type bounds all will instantiate to dynamic. Only need to check
+    // further if at least one of the functions has type bounds.
+    if ($t1.hasTypeBounds || $t2.hasTypeBounds) {
+      // Check the bounds of the type parameters of g1 and g2.
+      // given a type parameter `T1 extends U1` from g1, and a type parameter
+      // `T2 extends U2` from g2, we must ensure that:
+      //
+      //      U2 == U1
+      //
+      // (Note there is no variance in the type bounds of type parameters of
+      // generic functions).
+      let t1Bounds = $t1.instantiateTypeBounds(fresh);
+      let t2Bounds = $t2.instantiateTypeBounds(fresh);
+      for (let i = 0; i < formalCount; i++) {
+        if (t2Bounds[i] != t1Bounds[i]) {
+          return false;
+        }
       }
     }
 

tests/compiler/dartdevc_native/subtype_test.dart

@@ -124,15 +124,69 @@ void main() {
   // A -> B <: A -> A
   checkSubtype(function1(B, A), function1(A, A));
 
-  // <T extends B> void fn() <: <T extends B> void fn()
+  // Generic Function Subtypes.
+  // Bound is a built in type.
+  // <T extends int> void -> void <: <T extends int> void -> void
+  checkSubtype(genericFunction(int), genericFunction(int));
+
+  // <T extends String> A -> T <: <T extends String> B -> T
+  checkProperSubtype(
+      functionGenericReturn(String, A), functionGenericReturn(String, B));
+
+  // <T extends double> T -> B <: <T extends double> T -> A
+  checkProperSubtype(
+      functionGenericArg(double, B), functionGenericArg(double, A));
+
+  // Bound is a function type.
+  // <T extends A -> B> void -> void <: <T extends A -> B> void -> void
+  checkSubtype(
+      genericFunction(function1(B, A)), genericFunction(function1(B, A)));
+
+  // <T extends A -> B> A -> T <: <T extends A -> B> B -> T
+  checkProperSubtype(functionGenericReturn(function1(B, A), A),
+      functionGenericReturn(function1(B, A), B));
+
+  // <T extends A -> B> T -> B <: <T extends A -> B> T -> A
+  checkProperSubtype(functionGenericArg(function1(B, A), B),
+      functionGenericArg(function1(B, A), A));
+
+  // Bound is a user defined class.
+  // <T extends B> void -> void <: <T extends B> void -> void
   checkSubtype(genericFunction(B), genericFunction(B));
 
-  // <T extends B> T fn(A) <: <T extends B> T fn(B)
+  // <T extends B> A -> T <: <T extends B> B -> T
   checkProperSubtype(functionGenericReturn(B, A), functionGenericReturn(B, B));
 
-  // <T extends B> B fn(T) <: <T extends B> A fn(T)
+  // <T extends B> T -> B <: <T extends B> T -> A
   checkProperSubtype(functionGenericArg(B, B), functionGenericArg(B, A));
 
+  // Bound is a Future.
+  // <T extends Future<B>> void -> void <: <T extends Future<B>> void -> void
+  checkSubtype(genericFunction(generic1(Future, B)),
+      genericFunction(generic1(Future, B)));
+
+  // <T extends Future<B>> A -> T <: <T extends Future<B>> B -> T
+  checkProperSubtype(functionGenericReturn(generic1(Future, B), A),
+      functionGenericReturn(generic1(Future, B), B));
+
+  // <T extends Future<B>> T -> B <: <T extends Future<B>> T -> A
+  checkProperSubtype(functionGenericArg(generic1(Future, B), B),
+      functionGenericArg(generic1(Future, B), A));
+
+  // Bound is a FutureOr.
+  // <T extends FutureOr<B>> void -> void <:
+  //    <T extends FutureOr<B>> void -> void
+  checkSubtype(genericFunction(generic1(FutureOr, B)),
+      genericFunction(generic1(FutureOr, B)));
+
+  // <T extends FutureOr<B>> A -> T <: <T extends FutureOr<B>> B -> T
+  checkProperSubtype(functionGenericReturn(generic1(FutureOr, B), A),
+      functionGenericReturn(generic1(FutureOr, B), B));
+
+  // <T extends FutureOr<B>> T -> B <: <T extends FutureOr<B>> T -> A
+  checkProperSubtype(functionGenericArg(generic1(FutureOr, B), B),
+      functionGenericArg(generic1(FutureOr, B), A));
+
   // D <: D<B>
   checkSubtype(D, generic1(D, B));
   // D<B> <: D