[dart2js] Fix interface subtyping bug.

Previously, when checking S <: T, we only checked if S and T had the
same interface name once. However, it's possible that redirections
eventually cause them to become equal. For example, if S is an interop
type and T is JavaScriptObject, after following the redirection from S
to JavaScriptObject, we end up checking JavaScriptObject <:
JavaScriptObject. Therefore, we need to recheck after following each
redirection.

Change-Id: Ie3eb9530627a0e48a5ea704fd4078ae238a65c78
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/204744
Commit-Queue: Mayank Patke <[email protected]>
Reviewed-by: Stephen Adams <[email protected]>

Author: fishythefish

sdk/lib/_internal/js_runtime/lib/rti.dart

@@ -334,7 +334,7 @@ class Rti {
   }
 
   static Rti allocate() {
-    return new Rti();
+    return Rti();
   }
 
   Object? _canonicalRecipe;
@@ -2974,86 +2974,94 @@ bool _isInterfaceSubtype(
   String sName = Rti._getInterfaceName(s);
   String tName = Rti._getInterfaceName(t);
 
-  // Interface Compositionality:
-  if (sName == tName) {
-    var sArgs = Rti._getInterfaceTypeArguments(s);
-    var tArgs = Rti._getInterfaceTypeArguments(t);
-    int length = _Utils.arrayLength(sArgs);
-    assert(length == _Utils.arrayLength(tArgs));
-
-    var sVariances;
-    bool? hasVariances;
-    if (JS_GET_FLAG("VARIANCE")) {
-      sVariances = _Universe.findTypeParameterVariances(universe, sName);
-      hasVariances = sVariances != null;
-      assert(!hasVariances || length == _Utils.arrayLength(sVariances));
-    }
-
+  while (sName != tName) {
+    // The Super-Interface rule says that if [s] has superinterfaces C0,...,Cn,
+    // then we need to check if for some i, Ci <: [t]. However, this requires us
+    // to iterate over the superinterfaces. Instead, we can perform case
+    // analysis on [t]. By this point, [t] can only be Never, a type variable,
+    // or an interface type. (Bindings do not participate in subtype checks and
+    // all other cases have been eliminated.) If [t] is not an interface, then
+    // [s] </: [t]. Therefore, the only remaining case is that [t] is an
+    // interface, so rather than iterating over the Ci, we can instead look up
+    // [t] in our ruleset.
+    // TODO(fishythefish): Handle variance correctly.
+
+    var rule = _Universe._findRule(universe, sName);
+    if (rule == null) return false;
+    if (_Utils.isString(rule)) {
+      sName = _Utils.asString(rule);
+      continue;
+    }
+
+    var recipes = TypeRule.lookupSupertype(rule, tName);
+    if (recipes == null) return false;
+    int length = _Utils.arrayLength(recipes);
+    Object? supertypeArgs = length > 0 ? JS('', 'new Array(#)', length) : null;
     for (int i = 0; i < length; i++) {
-      Rti sArg = _Utils.asRti(_Utils.arrayAt(sArgs, i));
-      Rti tArg = _Utils.asRti(_Utils.arrayAt(tArgs, i));
-      if (JS_GET_FLAG("VARIANCE")) {
-        int sVariance = hasVariances != null
-            ? _Utils.asInt(_Utils.arrayAt(sVariances, i))
-            : Variance.legacyCovariant;
-        switch (sVariance) {
-          case Variance.legacyCovariant:
-          case Variance.covariant:
-            if (!_isSubtype(universe, sArg, sEnv, tArg, tEnv)) {
-              return false;
-            }
-            break;
-          case Variance.contravariant:
-            if (!_isSubtype(universe, tArg, tEnv, sArg, sEnv)) {
-              return false;
-            }
-            break;
-          case Variance.invariant:
-            if (!_isSubtype(universe, sArg, sEnv, tArg, tEnv) ||
-                !_isSubtype(universe, tArg, tEnv, sArg, sEnv)) {
-              return false;
-            }
-            break;
-          default:
-            throw StateError(
-                "Unknown variance given for subtype check: $sVariance");
-        }
-      } else {
-        if (!_isSubtype(universe, sArg, sEnv, tArg, tEnv)) {
-          return false;
-        }
-      }
+      String recipe = _Utils.asString(_Utils.arrayAt(recipes, i));
+      Rti supertypeArg = _Universe.evalInEnvironment(universe, s, recipe);
+      _Utils.arraySetAt(supertypeArgs, i, supertypeArg);
     }
-    return true;
+    var tArgs = Rti._getInterfaceTypeArguments(t);
+    return _areArgumentsSubtypes(
+        universe, supertypeArgs, null, sEnv, tArgs, tEnv);
   }
 
-  // The Super-Interface rule says that if [s] has superinterfaces C0,...,Cn,
-  // then we need to check if for some i, Ci <: [t]. However, this requires us
-  // to iterate over the superinterfaces. Instead, we can perform case
-  // analysis on [t]. By this point, [t] can only be Never, a type variable,
-  // or an interface type. (Bindings do not participate in subtype checks and
-  // all other cases have been eliminated.) If [t] is not an interface, then
-  // [s] </: [t]. Therefore, the only remaining case is that [t] is an
-  // interface, so rather than iterating over the Ci, we can instead look up
-  // [t] in our ruleset.
-  // TODO(fishythefish): Handle variance correctly.
-
-  // We don't list Object explicitly as a supertype of each interface, so check
-  // this trivial case first.
-  if (isObjectType(t)) return true;
-  var rule = _Universe.findRule(universe, sName);
-  if (rule == null) return false;
-  var supertypeArgs = TypeRule.lookupSupertype(rule, tName);
-  if (supertypeArgs == null) return false;
-  int length = _Utils.arrayLength(supertypeArgs);
+  // Interface Compositionality:
+  assert(sName == tName);
+  var sArgs = Rti._getInterfaceTypeArguments(s);
   var tArgs = Rti._getInterfaceTypeArguments(t);
+  var sVariances;
+  if (JS_GET_FLAG("VARIANCE")) {
+    sVariances = _Universe.findTypeParameterVariances(universe, sName);
+  }
+  return _areArgumentsSubtypes(universe, sArgs, sVariances, sEnv, tArgs, tEnv);
+}
+
+bool _areArgumentsSubtypes(Object? universe, Object? sArgs, Object? sVariances,
+    Object? sEnv, Object? tArgs, Object? tEnv) {
+  int length = sArgs != null ? _Utils.arrayLength(sArgs) : 0;
   assert(length == _Utils.arrayLength(tArgs));
+  bool hasVariances = sVariances != null;
+  if (JS_GET_FLAG("VARIANCE")) {
+    assert(!hasVariances || length == _Utils.arrayLength(sVariances));
+  } else {
+    assert(!hasVariances);
+  }
+
   for (int i = 0; i < length; i++) {
-    String recipe = _Utils.asString(_Utils.arrayAt(supertypeArgs, i));
-    Rti supertypeArg = _Universe.evalInEnvironment(universe, s, recipe);
+    Rti sArg = _Utils.asRti(_Utils.arrayAt(sArgs, i));
     Rti tArg = _Utils.asRti(_Utils.arrayAt(tArgs, i));
-    if (!_isSubtype(universe, supertypeArg, sEnv, tArg, tEnv)) {
-      return false;
+    if (JS_GET_FLAG("VARIANCE")) {
+      int sVariance = hasVariances
+          ? _Utils.asInt(_Utils.arrayAt(sVariances, i))
+          : Variance.legacyCovariant;
+      switch (sVariance) {
+        case Variance.legacyCovariant:
+        case Variance.covariant:
+          if (!_isSubtype(universe, sArg, sEnv, tArg, tEnv)) {
+            return false;
+          }
+          break;
+        case Variance.contravariant:
+          if (!_isSubtype(universe, tArg, tEnv, sArg, sEnv)) {
+            return false;
+          }
+          break;
+        case Variance.invariant:
+          if (!_isSubtype(universe, sArg, sEnv, tArg, tEnv) ||
+              !_isSubtype(universe, tArg, tEnv, sArg, sEnv)) {
+            return false;
+          }
+          break;
+        default:
+          throw StateError(
+              "Unknown variance given for subtype check: $sVariance");
+      }
+    } else {
+      if (!_isSubtype(universe, sArg, sEnv, tArg, tEnv)) {
+        return false;
+      }
     }
   }
   return true;

tests/web/internal/rti/js_interop_subtype_test.dart

@@ -0,0 +1,22 @@
+// Copyright (c) 2021, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:_interceptors';
+
+import 'package:expect/expect.dart';
+import 'package:js/js.dart';
+
+@JS()
+class JSClass {}
+
+@JS()
+external void eval(String code);
+
+void main() {
+  eval(r'''
+      function JSClass() {}
+      ''');
+  Expect.type<JavaScriptObject>(JSClass());
+  Expect.type<List<JavaScriptObject>>(<JSClass>[]);
+}

tests/web_2/internal/rti/js_interop_subtype_test.dart

@@ -0,0 +1,22 @@
+// Copyright (c) 2021, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:_interceptors';
+
+import 'package:expect/expect.dart';
+import 'package:js/js.dart';
+
+@JS()
+class JSClass {}
+
+@JS()
+external void eval(String code);
+
+void main() {
+  eval(r'''
+      function JSClass() {}
+      ''');
+  Expect.type<JavaScriptObject>(JSClass());
+  Expect.type<List<JavaScriptObject>>(<JSClass>[]);
+}