[dart2js] Added variance support for static subtype checking.

kallentu · commit-bot@chromium.org · commit 53bbe6c88c3f · 2019-12-04T20:54:16.000Z
Change-Id: I0731884aad68e3dd7f84658d75e946ca5db62bbe Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/127063 Reviewed-by: Sigmund Cherem <sigmund@google.com> Commit-Queue: Kallen Tu <kallentu@google.com>
diff --git a/pkg/compiler/lib/src/elements/entities.dart b/pkg/compiler/lib/src/elements/entities.dart
@@ -218,6 +218,9 @@ class AsyncMarker {
   int get index => values.indexOf(this);
 }
 
+/// Values for variance annotations.
+enum Variance { legacyCovariant, covariant, contravariant, invariant }
+
 /// Stripped down super interface for constructor like entities.
 ///
 /// Currently only [ConstructorElement] but later also kernel based Dart
diff --git a/pkg/compiler/lib/src/elements/types.dart b/pkg/compiler/lib/src/elements/types.dart
@@ -1501,6 +1501,9 @@ abstract class AbstractTypeRelation<T extends DartType>
   /// Returns the declared bound of [element].
   DartType getTypeVariableBound(TypeVariableEntity element);
 
+  /// Returns the variances for each type parameter in [cls].
+  List<Variance> getTypeVariableVariances(ClassEntity cls);
+
   @override
   bool visitType(T t, T s) {
     throw 'internal error: unknown type ${t}';
@@ -1529,10 +1532,25 @@ abstract class AbstractTypeRelation<T extends DartType>
     bool checkTypeArguments(InterfaceType instance, InterfaceType other) {
       List<T> tTypeArgs = instance.typeArguments;
       List<T> sTypeArgs = other.typeArguments;
+      List<Variance> tVariances = getTypeVariableVariances(instance.element);
       assert(tTypeArgs.length == sTypeArgs.length);
+      assert(tTypeArgs.length == tVariances.length);
       for (int i = 0; i < tTypeArgs.length; i++) {
-        if (invalidTypeArguments(tTypeArgs[i], sTypeArgs[i])) {
-          return false;
+        switch (tVariances[i]) {
+          case Variance.legacyCovariant:
+          case Variance.covariant:
+            if (invalidTypeArguments(tTypeArgs[i], sTypeArgs[i])) return false;
+            break;
+          case Variance.contravariant:
+            if (invalidTypeArguments(sTypeArgs[i], tTypeArgs[i])) return false;
+            break;
+          case Variance.invariant:
+            if (invalidTypeArguments(tTypeArgs[i], sTypeArgs[i]) ||
+                invalidTypeArguments(sTypeArgs[i], tTypeArgs[i])) return false;
+            break;
+          default:
+            throw StateError(
+                "Invalid variance ${tVariances[i]} used for subtype check.");
         }
       }
       return true;
diff --git a/pkg/compiler/lib/src/ir/element_map.dart b/pkg/compiler/lib/src/ir/element_map.dart
@@ -69,4 +69,5 @@ abstract class IrToElementMap {
   DartType getCallType(InterfaceType type);
   int getHierarchyDepth(IndexedClass cls);
   DartType getTypeVariableBound(IndexedTypeVariable typeVariable);
+  List<Variance> getTypeVariableVariances(IndexedClass cls);
 }
diff --git a/pkg/compiler/lib/src/ir/types.dart b/pkg/compiler/lib/src/ir/types.dart
@@ -132,6 +132,11 @@ abstract class AbstractTypeRelationMixin
     return elementMap.getTypeVariableBound(element);
   }
 
+  @override
+  List<Variance> getTypeVariableVariances(ClassEntity cls) {
+    return elementMap.getTypeVariableVariances(cls);
+  }
+
   @override
   FunctionType getCallType(InterfaceType type) {
     return elementMap.getCallType(type);
diff --git a/pkg/compiler/lib/src/ir/util.dart b/pkg/compiler/lib/src/ir/util.dart
@@ -68,6 +68,21 @@ AsyncMarker getAsyncMarker(ir.FunctionNode node) {
   }
 }
 
+/// Returns the `Variance` corresponding to `node.variance`.
+Variance convertVariance(ir.TypeParameter node) {
+  if (node.isLegacyCovariant) return Variance.legacyCovariant;
+  switch (node.variance) {
+    case ir.Variance.covariant:
+      return Variance.covariant;
+    case ir.Variance.contravariant:
+      return Variance.contravariant;
+    case ir.Variance.invariant:
+      return Variance.invariant;
+    default:
+      throw new UnsupportedError("Variance ${node.variance} is not supported.");
+  }
+}
+
 /// Kernel encodes a null-aware expression `a?.b` as
 ///
 ///     let final #1 = a in #1 == null ? null : #1.b
diff --git a/pkg/compiler/lib/src/js_model/closure.dart b/pkg/compiler/lib/src/js_model/closure.dart
@@ -895,6 +895,9 @@ class RecordClassData implements JClassData {
 
   @override
   InterfaceType get instantiationToBounds => thisType;
+
+  @override
+  List<Variance> getVariances() => null;
 }
 
 /// A container for variables declared in a particular scope that are accessed
diff --git a/pkg/compiler/lib/src/js_model/element_map_impl.dart b/pkg/compiler/lib/src/js_model/element_map_impl.dart
@@ -1070,6 +1070,13 @@ class JsKernelToElementMap implements JsToElementMap, IrToElementMap {
     return data.getBound(this);
   }
 
+  @override
+  List<Variance> getTypeVariableVariances(IndexedClass cls) {
+    assert(checkFamily(cls));
+    JClassData data = classes.getData(cls);
+    return data.getVariances();
+  }
+
   DartType _getTypeVariableDefaultType(IndexedTypeVariable typeVariable) {
     assert(checkFamily(typeVariable));
     JTypeVariableData data = typeVariables.getData(typeVariable);
diff --git a/pkg/compiler/lib/src/js_model/env.dart b/pkg/compiler/lib/src/js_model/env.dart
@@ -16,6 +16,7 @@ import '../elements/types.dart';
 import '../ir/element_map.dart';
 import '../ir/static_type_cache.dart';
 import '../ir/visitors.dart';
+import '../ir/util.dart';
 import '../js_model/element_map.dart';
 import '../ordered_typeset.dart';
 import '../serialization/serialization.dart';
@@ -451,6 +452,8 @@ abstract class JClassData {
 
   bool get isEnumClass;
   bool get isMixinApplication;
+
+  List<Variance> getVariances();
 }
 
 class JClassDataImpl implements JClassData {
@@ -482,6 +485,8 @@ class JClassDataImpl implements JClassData {
   @override
   OrderedTypeSet orderedTypeSet;
 
+  List<Variance> _variances;
+
   JClassDataImpl(this.cls, this.definition);
 
   factory JClassDataImpl.readFromDataSource(DataSource source) {
@@ -506,6 +511,10 @@ class JClassDataImpl implements JClassData {
 
   @override
   DartType get callType => null;
+
+  @override
+  List<Variance> getVariances() =>
+      _variances ??= cls.typeParameters.map(convertVariance).toList();
 }
 
 /// Enum used for identifying [JMemberData] subclasses in serialization.
diff --git a/pkg/compiler/lib/src/kernel/element_map_impl.dart b/pkg/compiler/lib/src/kernel/element_map_impl.dart
@@ -643,6 +643,13 @@ class KernelToElementMapImpl implements KernelToElementMap, IrToElementMap {
     return data.getBound(this);
   }
 
+  @override
+  List<Variance> getTypeVariableVariances(IndexedClass cls) {
+    assert(checkFamily(cls));
+    KClassData data = classes.getData(cls);
+    return data.getVariances();
+  }
+
   ClassEntity getAppliedMixin(IndexedClass cls) {
     assert(checkFamily(cls));
     KClassData data = classes.getData(cls);
diff --git a/pkg/compiler/lib/src/kernel/env.dart b/pkg/compiler/lib/src/kernel/env.dart
@@ -629,6 +629,7 @@ abstract class KClassData {
   bool get isMixinApplication;
 
   Iterable<ConstantValue> getMetadata(IrToElementMap elementMap);
+  List<Variance> getVariances();
 
   /// Convert this [KClassData] to the corresponding [JClassData].
   JClassData convert();
@@ -659,6 +660,7 @@ class KClassDataImpl implements KClassData {
   OrderedTypeSet orderedTypeSet;
 
   Iterable<ConstantValue> _metadata;
+  List<Variance> _variances;
 
   KClassDataImpl(this.node);
 
@@ -677,6 +679,10 @@ class KClassDataImpl implements KClassData {
         node.annotations);
   }
 
+  @override
+  List<Variance> getVariances() =>
+      _variances ??= node.typeParameters.map(convertVariance).toList();
+
   @override
   JClassData convert() {
     return new JClassDataImpl(node, new RegularClassDefinition(node));
diff --git a/tests/compiler/dart2js/codegen/variance_subtype_cast_test.dart b/tests/compiler/dart2js/codegen/variance_subtype_cast_test.dart
@@ -0,0 +1,132 @@
+// Copyright (c) 2019, 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 'package:async_helper/async_helper.dart';
+import '../helpers/compiler_helper.dart';
+
+const String LEGACY_COV_CAST = r"""
+class LegacyCovariant<T> {
+  void method() {}
+}
+foo(param) {
+  LegacyCovariant<num> c = LegacyCovariant<num>();
+  (c as LegacyCovariant<int>).method();
+  // present: 'LegacyCovariant_int._as'
+}
+""";
+
+const String LEGACY_COV_NO_CAST = r"""
+class LegacyCovariant<T> {
+  void method() {}
+}
+foo(param) {
+  LegacyCovariant<num> c = LegacyCovariant<num>();
+  (c as LegacyCovariant<Object>).method();
+  // absent: 'LegacyCovariant_Object._as'
+}
+""";
+
+const String COV_CAST = r"""
+class Covariant<out T> {
+  void method() {}
+}
+foo(param) {
+  Covariant<num> c = Covariant<num>();
+  (c as Covariant<int>).method();
+  // present: 'Covariant_int._as'
+}
+""";
+
+const String COV_NO_CAST = r"""
+class Covariant<out T> {
+  void method() {}
+}
+foo(param) {
+  Covariant<num> c = Covariant<num>();
+  (c as Covariant<Object>).method();
+  // absent: 'Covariant_Object._as'
+}
+""";
+
+const String CONTRA_CAST = r"""
+class Contravariant<in T> {
+  void method() {}
+}
+foo(param) {
+  Contravariant<num> c = Contravariant<num>();
+  (c as Contravariant<Object>).method();
+  // present: 'Contravariant_Object._as'
+}
+""";
+
+const String CONTRA_NO_CAST = r"""
+class Contravariant<in T> {
+  void method() {}
+}
+foo(param) {
+  Contravariant<num> c = Contravariant<num>();
+  (c as Contravariant<int>).method();
+  // absent: 'Contravariant_int._as'
+}
+""";
+
+const String INV_CAST1 = r"""
+class Invariant<inout T> {
+  void method() {}
+}
+foo(param) {
+  Invariant<num> i = Invariant<num>();
+  (i as Invariant<Object>).method();
+  // present: 'Invariant_Object._as'
+}
+""";
+
+const String INV_CAST2 = r"""
+class Invariant<inout T> {
+  void method() {}
+}
+foo(param) {
+  Invariant<num> i = Invariant<num>();
+  (i as Invariant<int>).method();
+  // present: 'Invariant_int._as'
+}
+""";
+
+const String INV_NO_CAST = r"""
+class Invariant<inout T> {
+  void method() {}
+}
+foo(param) {
+  Invariant<num> i = Invariant<num>();
+  (i as Invariant<num>).method();
+  // absent: 'Invariant_num._as'
+}
+""";
+
+main() {
+  runTests() async {
+    Future check(String test) {
+      return compile(test,
+          entry: 'foo',
+          check: checkerForAbsentPresent(test),
+          newRti: true,
+          enableVariance: true);
+    }
+
+    await check(LEGACY_COV_CAST);
+    await check(LEGACY_COV_NO_CAST);
+    await check(COV_CAST);
+    await check(COV_NO_CAST);
+    await check(CONTRA_CAST);
+    await check(CONTRA_NO_CAST);
+    await check(INV_CAST1);
+    await check(INV_CAST2);
+    await check(INV_NO_CAST);
+  }
+
+  asyncTest(() async {
+    print('--test from kernel------------------------------------------------');
+    await runTests();
+  });
+}
diff --git a/tests/compiler/dart2js/helpers/compiler_helper.dart b/tests/compiler/dart2js/helpers/compiler_helper.dart
@@ -38,6 +38,7 @@ Future<String> compile(String code,
     bool disableTypeInference: true,
     bool omitImplicitChecks: true,
     bool newRti: false,
+    bool enableVariance: false,
     void check(String generatedEntry),
     bool returnAll: false}) async {
   OutputCollector outputCollector = returnAll ? new OutputCollector() : null;
@@ -63,6 +64,9 @@ Future<String> compile(String code,
   if (newRti) {
     options.add(Flags.experimentNewRti);
   }
+  if (enableVariance) {
+    options.add('${Flags.enableLanguageExperiments}=variance');
+  }
 
   // Pretend this is a dart2js_native test to allow use of 'native' keyword
   // and import of private libraries.

Original file line number	Diff line number	Diff line change
`@@ -218,6 +218,9 @@ class AsyncMarker {`
`218`	`218`	`int get index => values.indexOf(this);`
`219`	`219`	`}`
`220`	`220`
	`221`	`+/// Values for variance annotations.`
	`222`	`+enum Variance { legacyCovariant, covariant, contravariant, invariant }`
	`223`	`+`
`221`	`224`	`/// Stripped down super interface for constructor like entities.`
`222`	`225`	`///`
`223`	`226`	`/// Currently only [ConstructorElement] but later also kernel based Dart`
Original file line number	Diff line number	Diff line change
`@@ -69,4 +69,5 @@ abstract class IrToElementMap {`
`69`	`69`	`DartType getCallType(InterfaceType type);`
`70`	`70`	`int getHierarchyDepth(IndexedClass cls);`
`71`	`71`	`DartType getTypeVariableBound(IndexedTypeVariable typeVariable);`
	`72`	`+ List<Variance> getTypeVariableVariances(IndexedClass cls);`
`72`	`73`	`}`
Original file line number	Diff line number	Diff line change
`@@ -895,6 +895,9 @@ class RecordClassData implements JClassData {`
`895`	`895`
`896`	`896`	`@override`
`897`	`897`	`InterfaceType get instantiationToBounds => thisType;`
	`898`	`+`
	`899`	`+ @override`
	`900`	`+ List<Variance> getVariances() => null;`
`898`	`901`	`}`
`899`	`902`
`900`	`903`	`/// A container for variables declared in a particular scope that are accessed`