[dart2js] Pass type arguments to callable properties even with

fishythefish · commit-bot@chromium.org · commit 5671730c5ae2 · 2020-07-24T11:09:15.000Z
--omit-implicit-checks. When the default check policy is "trusted" instead of "emitted", we skip checking every function during the RTI need computation. This works because if the type arguments are needed for some other reason, like the literal being used in the body, we see the type use and go back and update the RTI need of the function. We can't do this for callable properties because we don't know which function will be assigned at runtime, so we need to conservatively provide type arguments no matter what. We could optimize this slightly by only providing type arguments if we know a type use occurs in a function which is assignable to the type of the property, but I expect this to save little for the amount of overhead during compilation. We also update the dependency computation to ensure that if a callable property needs type arguments, then they're available for the enclosing context to pass along. Bug: #41449 Fixes: #41449 Change-Id: I98d9024dfa64cbfe33bd43172ffa905a8537649e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/154284 Reviewed-by: Stephen Adams <sra@google.com> Commit-Queue: Mayank Patke <fishythefish@google.com>
diff --git a/pkg/compiler/lib/src/js_backend/runtime_types_resolution.dart b/pkg/compiler/lib/src/js_backend/runtime_types_resolution.dart
@@ -167,11 +167,15 @@ class MethodNode extends CallableNode {
   }
 }
 
+bool _isProperty(Entity entity) =>
+    entity is MemberEntity && (entity.isField || entity.isGetter);
+
 class CallablePropertyNode extends CallableNode {
   final MemberEntity property;
   final DartType type;
 
-  CallablePropertyNode(this.property, this.type);
+  CallablePropertyNode(this.property, this.type)
+      : assert(_isProperty(property));
 
   @override
   Entity get entity => property;
@@ -181,8 +185,6 @@ class CallablePropertyNode extends CallableNode {
 
   @override
   bool selectorApplies(Selector selector, BuiltWorld world) {
-    if (world.annotationsData.getParameterCheckPolicy(property).isTrusted)
-      return false;
     if (property.memberName != selector.memberName) return false;
     if (type is FunctionType &&
         !selector.callStructure
@@ -305,6 +307,8 @@ class TypeVariableTests {
     Iterable<RtiNode> dependencies;
     if (entity is ClassEntity) {
       dependencies = _classes[entity]?.dependencies;
+    } else if (_isProperty(entity)) {
+      dependencies = _callableProperties[entity]?.dependencies;
     } else {
       dependencies = _methods[entity]?.dependencies;
     }
@@ -1031,6 +1035,8 @@ class RuntimeTypesNeedBuilderImpl implements RuntimeTypesNeedBuilder {
         });
       } else if (entity is FunctionEntity) {
         methodsNeedingTypeArguments.add(entity);
+      } else if (_isProperty(entity)) {
+        // Do nothing. We just need to visit the dependencies.
       } else {
         localFunctionsNeedingTypeArguments.add(entity);
       }
@@ -1139,6 +1145,9 @@ class RuntimeTypesNeedBuilderImpl implements RuntimeTypesNeedBuilder {
     localFunctionsUsingTypeVariableLiterals
         .forEach(potentiallyNeedTypeArguments);
 
+    typeVariableTests._callableProperties.keys
+        .forEach(potentiallyNeedTypeArguments);
+
     if (closedWorld.isMemberUsed(
         closedWorld.commonElements.invocationTypeArgumentGetter)) {
       // If `Invocation.typeArguments` is live, mark all user-defined
@@ -1317,7 +1326,7 @@ class RuntimeTypesNeedBuilderImpl implements RuntimeTypesNeedBuilder {
     typeVariableTests
         .forEachAppliedSelector((Selector selector, Set<Entity> targets) {
       for (Entity target in targets) {
-        if (target is MemberEntity && (target.isField || target.isGetter) ||
+        if (_isProperty(target) ||
             methodsNeedingTypeArguments.contains(target) ||
             localFunctionsNeedingTypeArguments.contains(target)) {
           selectorsNeedingTypeArguments.add(selector);
diff --git a/tests/dart2js/41449c_test.dart b/tests/dart2js/41449c_test.dart
@@ -0,0 +1,57 @@
+// Copyright (c) 2020, 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.
+//
+// dart2jsOptions=-O4
+
+// Regression test for passing type parameters through call-through stub.
+//
+// We use an abstract class with two implementations to avoid the optimizer
+// 'inlining' the call-through stub, so we are testing that the stub itself
+// passes through the type parameters.
+
+import 'package:expect/expect.dart';
+
+abstract class AAA {
+  dynamic get foo;
+}
+
+class B1 implements AAA {
+  final dynamic foo;
+  B1(this.foo);
+}
+
+class B2 implements AAA {
+  final dynamic _arr;
+  B2(foo) : _arr = [foo];
+  dynamic get foo => _arr.first;
+}
+
+class B3 implements AAA {
+  final dynamic __foo;
+  B3(this.__foo);
+  dynamic get _foo => __foo;
+  dynamic get foo => _foo;
+}
+
+@pragma('dart2js:noInline')
+test1<T>(AAA a, String expected) {
+  // call-through getter 'foo' with one type argument.
+  Expect.equals(expected, a.foo<T>());
+}
+
+@pragma('dart2js:noInline')
+test2<U, V>(AAA a, String expected) {
+  // call-through getter 'foo' with two type arguments.
+  Expect.equals(expected, a.foo<U, V>());
+}
+
+main() {
+  test1<int>(B1(<P>() => '$P'), 'int');
+  test1<num>(B2(<Q>() => '$Q'), 'num');
+  test1<double>(B3(<R>() => '$R'), 'double');
+
+  test2<int, num>(B1(<A, B>() => '$A $B'), 'int num');
+  test2<num, int>(B2(<X, Y>() => '$X $Y'), 'num int');
+  test2<double, String>(B3(<C, D>() => '$C $D'), 'double String');
+}
diff --git a/tests/dart2js_2/41449c_test.dart b/tests/dart2js_2/41449c_test.dart
@@ -0,0 +1,59 @@
+// Copyright (c) 2020, 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.
+//
+// dart2jsOptions=-O4
+
+// @dart = 2.7
+
+// Regression test for passing type parameters through call-through stub.
+//
+// We use an abstract class with two implementations to avoid the optimizer
+// 'inlining' the call-through stub, so we are testing that the stub itself
+// passes through the type parameters.
+
+import 'package:expect/expect.dart';
+
+abstract class AAA {
+  dynamic get foo;
+}
+
+class B1 implements AAA {
+  final dynamic foo;
+  B1(this.foo);
+}
+
+class B2 implements AAA {
+  final dynamic _arr;
+  B2(foo) : _arr = [foo];
+  dynamic get foo => _arr.first;
+}
+
+class B3 implements AAA {
+  final dynamic __foo;
+  B3(this.__foo);
+  dynamic get _foo => __foo;
+  dynamic get foo => _foo;
+}
+
+@pragma('dart2js:noInline')
+test1<T>(AAA a, String expected) {
+  // call-through getter 'foo' with one type argument.
+  Expect.equals(expected, a.foo<T>());
+}
+
+@pragma('dart2js:noInline')
+test2<U, V>(AAA a, String expected) {
+  // call-through getter 'foo' with two type arguments.
+  Expect.equals(expected, a.foo<U, V>());
+}
+
+main() {
+  test1<int>(B1(<P>() => '$P'), 'int');
+  test1<num>(B2(<Q>() => '$Q'), 'num');
+  test1<double>(B3(<R>() => '$R'), 'double');
+
+  test2<int, num>(B1(<A, B>() => '$A $B'), 'int num');
+  test2<num, int>(B2(<X, Y>() => '$X $Y'), 'num int');
+  test2<double, String>(B3(<C, D>() => '$C $D'), 'double String');
+}
