[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 <fishythefish@google.com>
Reviewed-by: Stephen Adams <sra@google.com>
diff --git a/sdk/lib/_internal/js_runtime/lib/rti.dart b/sdk/lib/_internal/js_runtime/lib/rti.dart
index 96fc0a8..694bc1d 100644
--- a/sdk/lib/_internal/js_runtime/lib/rti.dart
+++ b/sdk/lib/_internal/js_runtime/lib/rti.dart
@@ -334,7 +334,7 @@
}
static Rti allocate() {
- return new Rti();
+ return Rti();
}
Object? _canonicalRecipe;
@@ -2974,86 +2974,94 @@
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));
+ 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 sVariances;
- bool? hasVariances;
- if (JS_GET_FLAG("VARIANCE")) {
- sVariances = _Universe.findTypeParameterVariances(universe, sName);
- hasVariances = sVariances != null;
- assert(!hasVariances || length == _Utils.arrayLength(sVariances));
+ 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;
diff --git a/tests/web/internal/rti/js_interop_subtype_test.dart b/tests/web/internal/rti/js_interop_subtype_test.dart
new file mode 100644
index 0000000..66010da
--- /dev/null
+++ b/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>[]);
+}
diff --git a/tests/web_2/internal/rti/js_interop_subtype_test.dart b/tests/web_2/internal/rti/js_interop_subtype_test.dart
new file mode 100644
index 0000000..66010da
--- /dev/null
+++ b/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>[]);
+}
