Optimize signature need computation.

Change-Id: Ied6dfbfe71a9d323e189dbcacdb5ae2985c3fbb8
Reviewed-on: https://dart-review.googlesource.com/52221
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>

pkg/compiler/lib/src/elements/resolution_types.dart

@@ -1239,10 +1239,12 @@ class Types extends DartTypes {
   }
 
   bool isPotentialSubtype(
-      covariant ResolutionDartType t, covariant ResolutionDartType s) {
+      covariant ResolutionDartType t, covariant ResolutionDartType s,
+      {bool assumeInstantiations: true}) {
     // TODO(johnniwinther): Return a set of variable points in the positive
     // cases.
-    return potentialSubtypeVisitor.isSubtype(t, s);
+    return potentialSubtypeVisitor.isPotentialSubtype(t, s,
+        assumeInstantiations: assumeInstantiations);
   }
 
   @override

pkg/compiler/lib/src/elements/types.dart

@@ -1033,13 +1033,14 @@ abstract class AbstractTypeRelation<T extends DartType>
     if (s is! FunctionType) return false;
     FunctionType tf = t;
     FunctionType sf = s;
-    if (tf.typeVariables.length != sf.typeVariables.length) {
+    int typeVariablesCount = getCommonTypeVariablesCount(tf, sf);
+    if (typeVariablesCount == null) {
       return false;
     }
-    for (int i = 0; i < tf.typeVariables.length; i++) {
+    for (int i = 0; i < typeVariablesCount; i++) {
       assumptions.assume(tf.typeVariables[i], sf.typeVariables[i]);
     }
-    for (int i = 0; i < tf.typeVariables.length; i++) {
+    for (int i = 0; i < typeVariablesCount; i++) {
       if (!tf.typeVariables[i].bound
           ._equals(sf.typeVariables[i].bound, assumptions)) {
         return false;
@@ -1050,12 +1051,19 @@ abstract class AbstractTypeRelation<T extends DartType>
     }
 
     bool result = visitFunctionTypeInternal(tf, sf);
-    for (int i = 0; i < tf.typeVariables.length; i++) {
+    for (int i = 0; i < typeVariablesCount; i++) {
       assumptions.forget(tf.typeVariables[i], sf.typeVariables[i]);
     }
     return result;
   }
 
+  int getCommonTypeVariablesCount(FunctionType t, FunctionType s) {
+    if (t.typeVariables.length == s.typeVariables.length) {
+      return t.typeVariables.length;
+    }
+    return null;
+  }
+
   bool visitFunctionTypeInternal(FunctionType tf, FunctionType sf) {
     // TODO(johnniwinther): Rewrite the function subtyping to be more readable
     // but still as efficient.
@@ -1301,12 +1309,34 @@ abstract class SubtypeVisitor<T extends DartType>
 /// `false` only if we are sure no such substitution exists.
 abstract class PotentialSubtypeVisitor<T extends DartType>
     extends SubtypeVisitor<T> {
+  bool _assumeInstantiations = true;
+
   bool isSubtype(DartType t, DartType s) {
     if (t is TypeVariableType || s is TypeVariableType) {
       return true;
     }
+    if ((t is FunctionTypeVariable || s is FunctionTypeVariable) &&
+        _assumeInstantiations) {
+      return true;
+    }
     return super.isSubtype(t, s);
   }
+
+  int getCommonTypeVariablesCount(FunctionType t, FunctionType s) {
+    if (t.typeVariables.length == s.typeVariables.length) {
+      return t.typeVariables.length;
+    }
+    if (_assumeInstantiations && s.typeVariables.length == 0) {
+      return 0;
+    }
+    return null;
+  }
+
+  bool isPotentialSubtype(DartType t, DartType s,
+      {bool assumeInstantiations: true}) {
+    _assumeInstantiations = assumeInstantiations;
+    return isSubtype(t, s);
+  }
 }
 
 /// Basic interface for the Dart type system.
@@ -1322,7 +1352,11 @@ abstract class DartTypes {
 
   /// Returns `true` if [t] might be a subtype of [s] for some values of
   /// type variables in [s] and [t].
-  bool isPotentialSubtype(DartType t, DartType s);
+  ///
+  /// If [assumeInstantiations], generic function types are assumed to be
+  /// potentially instantiated.
+  bool isPotentialSubtype(DartType t, DartType s,
+      {bool assumeInstantiations: true});
 
   static const int IS_SUBTYPE = 1;
   static const int MAYBE_SUBTYPE = 0;

pkg/compiler/lib/src/js_backend/backend_usage.dart

@@ -47,6 +47,9 @@ abstract class BackendUsage {
 
   /// `true` if `noSuchMethod` is used.
   bool get isNoSuchMethodUsed;
+
+  /// `true` if generic instantiation is used.
+  bool get isGenericInstantiationUsed;
 }
 
 abstract class BackendUsageBuilder {
@@ -87,6 +90,9 @@ abstract class BackendUsageBuilder {
   /// `true` if `noSuchMethod` is used.
   bool isNoSuchMethodUsed;
 
+  /// `true` if generic instantiation is used.
+  bool isGenericInstantiationUsed;
+
   BackendUsage close();
 }
 
@@ -126,6 +132,9 @@ class BackendUsageBuilderImpl implements BackendUsageBuilder {
   /// `true` if `noSuchMethod` is used.
   bool isNoSuchMethodUsed = false;
 
+  /// `true` if generic instantiation is used.
+  bool isGenericInstantiationUsed = false;
+
   BackendUsageBuilderImpl(this._commonElements);
 
   @override
@@ -284,7 +293,8 @@ class BackendUsageBuilderImpl implements BackendUsageBuilder {
         isIsolateInUse: isIsolateInUse,
         isFunctionApplyUsed: isFunctionApplyUsed,
         isMirrorsUsed: isMirrorsUsed,
-        isNoSuchMethodUsed: isNoSuchMethodUsed);
+        isNoSuchMethodUsed: isNoSuchMethodUsed,
+        isGenericInstantiationUsed: isGenericInstantiationUsed);
   }
 }
 
@@ -323,6 +333,9 @@ class BackendUsageImpl implements BackendUsage {
   /// `true` if `noSuchMethod` is used.
   final bool isNoSuchMethodUsed;
 
+  /// `true` if generic instantiation is used.
+  final bool isGenericInstantiationUsed;
+
   BackendUsageImpl(
       {Set<FunctionEntity> globalFunctionDependencies,
       Set<ClassEntity> globalClassDependencies,
@@ -336,7 +349,8 @@ class BackendUsageImpl implements BackendUsage {
       this.isIsolateInUse,
       this.isFunctionApplyUsed,
       this.isMirrorsUsed,
-      this.isNoSuchMethodUsed})
+      this.isNoSuchMethodUsed,
+      this.isGenericInstantiationUsed})
       : this._globalFunctionDependencies = globalFunctionDependencies,
         this._globalClassDependencies = globalClassDependencies,
         this._helperFunctionsUsed = helperFunctionsUsed,

pkg/compiler/lib/src/js_backend/impact_transformer.dart

@@ -41,7 +41,7 @@ class JavaScriptImpactTransformer extends ImpactTransformer {
   final BackendImpacts _impacts;
   final NativeBasicData _nativeBasicData;
   final NativeResolutionEnqueuer _nativeResolutionEnqueuer;
-  final BackendUsageBuilder _backendUsageBuider;
+  final BackendUsageBuilder _backendUsageBuilder;
   final MirrorsDataBuilder _mirrorsDataBuilder;
   final CustomElementsResolutionAnalysis _customElementsResolutionAnalysis;
   final RuntimeTypesNeedBuilder _rtiNeedBuilder;
@@ -54,7 +54,7 @@ class JavaScriptImpactTransformer extends ImpactTransformer {
       this._impacts,
       this._nativeBasicData,
       this._nativeResolutionEnqueuer,
-      this._backendUsageBuider,
+      this._backendUsageBuilder,
       this._mirrorsDataBuilder,
       this._customElementsResolutionAnalysis,
       this._rtiNeedBuilder,
@@ -67,7 +67,7 @@ class JavaScriptImpactTransformer extends ImpactTransformer {
 
     void registerImpact(BackendImpact impact) {
       impact.registerImpact(transformed, _elementEnvironment);
-      _backendUsageBuider.processBackendImpact(impact);
+      _backendUsageBuilder.processBackendImpact(impact);
     }
 
     for (Feature feature in worldImpact.features) {
@@ -110,6 +110,7 @@ class JavaScriptImpactTransformer extends ImpactTransformer {
           break;
         case Feature.GENERIC_INSTANTIATION:
           registerImpact(_impacts.genericInstantiation);
+          _backendUsageBuilder.isGenericInstantiationUsed = true;
           break;
         case Feature.LAZY_FIELD:
           registerImpact(_impacts.lazyField);
@@ -251,7 +252,9 @@ class JavaScriptImpactTransformer extends ImpactTransformer {
           Local closure = staticUse.element;
           FunctionType type = _elementEnvironment.getLocalFunctionType(closure);
           if (type.containsTypeVariables ||
-              (_options.strongMode && !optimizeLocalSignaturesForStrongMode)) {
+              // TODO(johnniwinther): Can we avoid the need for signatures in
+              // Dart 2?
+              _options.strongMode) {
             registerImpact(_impacts.computeSignature);
           }
           break;
@@ -300,7 +303,7 @@ class JavaScriptImpactTransformer extends ImpactTransformer {
   void onIsCheck(DartType type, TransformedWorldImpact transformed) {
     void registerImpact(BackendImpact impact) {
       impact.registerImpact(transformed, _elementEnvironment);
-      _backendUsageBuider.processBackendImpact(impact);
+      _backendUsageBuilder.processBackendImpact(impact);
     }
 
     type = _elementEnvironment.getUnaliasedType(type);

pkg/compiler/lib/src/js_backend/runtime_types.dart

@@ -27,10 +27,6 @@ import 'namer.dart';
 
 bool cacheRtiDataForTesting = false;
 
-// TODO(johnniwinther): Remove this when local signatures are optimized for
-// Dart 2.
-const bool optimizeLocalSignaturesForStrongMode = false;
-
 /// For each class, stores the possible class subtype tests that could succeed.
 abstract class TypeChecks {
   /// Get the set of checks required for class [element].
@@ -1412,6 +1408,12 @@ class RuntimeTypesNeedBuilderImpl extends _RuntimeTypesBase
       });
     }
 
+    Set<Local> localFunctions = strongMode
+        ? resolutionWorldBuilder.localFunctions.toSet()
+        : resolutionWorldBuilder.localFunctionsWithFreeTypeVariables.toSet();
+    Set<FunctionEntity> closurizedMembers =
+        resolutionWorldBuilder.closurizedMembersWithFreeTypeVariables.toSet();
+
     // Check local functions and closurized members.
     void checkClosures({DartType potentialSubtypeOf}) {
       bool checkFunctionType(FunctionType functionType) {
@@ -1426,32 +1428,51 @@ class RuntimeTypesNeedBuilderImpl extends _RuntimeTypesBase
         return false;
       }
 
+      Set<Local> localFunctionsToRemove;
+      Set<FunctionEntity> closurizedMembersToRemove;
       if (strongMode) {
-        assert(!optimizeLocalSignaturesForStrongMode);
-        // TODO(johnniwinther): Optimize generation of signatures for Dart 2.
-        for (Local function in resolutionWorldBuilder.localFunctions) {
+        for (Local function in localFunctions) {
           FunctionType functionType =
               _elementEnvironment.getLocalFunctionType(function);
-          functionType.forEachTypeVariable((TypeVariableType typeVariable) {
-            potentiallyNeedTypeArguments(typeVariable.element.typeDeclaration);
-          });
-          localFunctionsNeedingSignature.add(function);
+          if (potentialSubtypeOf == null ||
+              closedWorld.dartTypes.isPotentialSubtype(
+                  functionType, potentialSubtypeOf,
+                  assumeInstantiations:
+                      closedWorld.backendUsage.isGenericInstantiationUsed)) {
+            functionType.forEachTypeVariable((TypeVariableType typeVariable) {
+              Entity typeDeclaration = typeVariable.element.typeDeclaration;
+              if (!processedEntities.contains(typeDeclaration)) {
+                potentiallyNeedTypeArguments(typeDeclaration);
+              }
+            });
+            localFunctionsNeedingSignature.add(function);
+            localFunctionsToRemove ??= new Set<Local>();
+            localFunctionsToRemove.add(function);
+          }
         }
       } else {
-        for (Local function
-            in resolutionWorldBuilder.localFunctionsWithFreeTypeVariables) {
+        for (Local function in localFunctions) {
           if (checkFunctionType(
               _elementEnvironment.getLocalFunctionType(function))) {
             localFunctionsNeedingSignature.add(function);
+            localFunctionsToRemove ??= new Set<Local>();
+            localFunctionsToRemove.add(function);
           }
         }
       }
-      for (FunctionEntity function
-          in resolutionWorldBuilder.closurizedMembersWithFreeTypeVariables) {
+      for (FunctionEntity function in closurizedMembers) {
         if (checkFunctionType(_elementEnvironment.getFunctionType(function))) {
           methodsNeedingSignature.add(function);
+          closurizedMembersToRemove ??= new Set<FunctionEntity>();
+          closurizedMembersToRemove.add(function);
         }
       }
+      if (localFunctionsToRemove != null) {
+        localFunctions.removeAll(localFunctionsToRemove);
+      }
+      if (closurizedMembersToRemove != null) {
+        closurizedMembers.removeAll(closurizedMembersToRemove);
+      }
     }
 
     // Compute the set of all classes and methods that need runtime type

pkg/compiler/lib/src/kernel/types.dart

@@ -16,8 +16,10 @@ class _KernelDartTypes extends DartTypes {
             new _KernelPotentialSubtypeVisitor(elementMap);
 
   @override
-  bool isPotentialSubtype(DartType t, DartType s) {
-    return potentialSubtypeVisitor.isSubtype(t, s);
+  bool isPotentialSubtype(DartType t, DartType s,
+      {bool assumeInstantiations: true}) {
+    return potentialSubtypeVisitor.isPotentialSubtype(t, s,
+        assumeInstantiations: assumeInstantiations);
   }
 
   @override

pkg/compiler/lib/src/ssa/kernel_impact.dart

@@ -593,6 +593,7 @@ class KernelImpactBuilder extends ir.Visitor {
 
   @override
   void visitInstantiation(ir.Instantiation node) {
+    // TODO(johnniwinther): Track which arities are used in instantiation.
     impactBuilder.registerFeature(Feature.GENERIC_INSTANTIATION);
     node.visitChildren(this);
   }

tests/compiler/dart2js/rti/data/closure_generic_unneeded_strong.dart

@@ -4,11 +4,11 @@
 
 import 'package:expect/expect.dart';
 
-/*class: A:needsArgs*/
+/*class: A:*/
 class A<T> {
   @NoInline()
   m() {
-    return /*needsSignature*/ (T t, String s) {};
+    return /**/ (T t, String s) {};
   }
 }
 

tests/compiler/dart2js/rti/data/closure_unneeded.dart

@@ -4,14 +4,11 @@
 
 import 'package:expect/expect.dart';
 
-/*ast.class: A:*/
-/*kernel.class: A:*/
-/*strong.class: A:needsArgs*/
+/*class: A:*/
 class A<T> {
   @NoInline()
   m() {
-    // TODO(johnniwinther): Optimize local function type signature need.
-    return /*ast.*/ /*kernel.*/ /*strong.needsSignature*/ (T t, String s) {};
+    return /**/ (T t, String s) {};
   }
 }
 

tests/compiler/dart2js/rti/data/closure_unneeded_strong.dart

@@ -8,7 +8,7 @@ import 'package:expect/expect.dart';
 class A<T> {
   @NoInline()
   m() {
-    return /*needsSignature*/ (int i, String s) {};
+    return /**/ (int i, String s) {};
   }
 }
 

tests/compiler/dart2js/rti/data/local_function_signature2_strong.dart

@@ -0,0 +1,109 @@
+// Copyright (c) 2018, 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:expect/expect.dart';
+
+class Class1 {
+  method1() {
+    /**/
+    num local<T>(num n) => null;
+    return local;
+  }
+
+  method2() {
+    /**/
+    num local<T>(int n) => null;
+    return local;
+  }
+
+  method3() {
+    /**/
+    int local<T>(num n) => null;
+    return local;
+  }
+}
+
+class Class2 {
+  /*element: Class2.method4:needsArgs,selectors=[Selector(call, method4, arity=0, types=1)]*/
+  method4<T>() {
+    /*needsSignature*/
+    num local(T n) => null;
+    return local;
+  }
+}
+
+class Class3 {
+  /*element: Class3.method5:needsArgs,selectors=[Selector(call, method5, arity=0, types=1)]*/
+  method5<T>() {
+    /*needsSignature*/
+    T local(num n) => null;
+    return local;
+  }
+}
+
+class Class4 {
+  /*element: Class4.method6:*/
+  method6<T>() {
+    /**/
+    num local(num n, T t) => null;
+    return local;
+  }
+}
+
+/*element: method7:needsArgs*/
+method7<T>() {
+  /*needsSignature*/
+  num local(T n) => null;
+  return local;
+}
+
+/*element: method8:needsArgs*/
+method8<T>() {
+  /*needsSignature*/
+  T local(num n) => null;
+  return local;
+}
+
+/*element: method9:*/
+method9<T>() {
+  /**/
+  num local(num n, T t) => null;
+  return local;
+}
+
+method10() {
+  /**/
+  num local<T>(T n) => null;
+  return local;
+}
+
+method11() {
+  /**/
+  T local<T>(num n) => null;
+  return local;
+}
+
+method12() {
+  /**/
+  num local<T>(num n, T t) => null;
+  return local;
+}
+
+@NoInline()
+test(o) => o is num Function(num);
+
+main() {
+  Expect.isFalse(test(new Class1().method1()));
+  Expect.isFalse(test(new Class1().method2()));
+  Expect.isFalse(test(new Class1().method3()));
+  Expect.isTrue(test(new Class2().method4<num>()));
+  Expect.isTrue(test(new Class3().method5<num>()));
+  Expect.isFalse(test(new Class4().method6<num>()));
+  Expect.isTrue(test(method7<num>()));
+  Expect.isTrue(test(method8<num>()));
+  Expect.isFalse(test(method9()));
+  Expect.isFalse(test(method10()));
+  Expect.isFalse(test(method11()));
+  Expect.isFalse(test(method12()));
+}

tests/compiler/dart2js/rti/data/local_function_signatures.dart

@@ -14,17 +14,11 @@ class Class1 {
   }
 
   method2() {
-    /*ast.*/
-    /*kernel.*/
-    /*strong.needsSignature*/
     num local(int n) => null;
     return local;
   }
 
   method3() {
-    /*ast.*/
-    /*kernel.*/
-    /*strong.needsSignature*/
     Object local(num n) => null;
     return local;
   }
@@ -48,14 +42,10 @@ class Class3<T> {
   }
 }
 
-/*ast.class: Class4:*/
-/*kernel.class: Class4:*/
-/*strong.class: Class4:needsArgs*/
+/*class: Class4:*/
 class Class4<T> {
   method6() {
-    /*ast.*/
-    /*kernel.*/
-    /*strong.needsSignature*/
+    /**/
     num local(num n, T t) => null;
     return local;
   }

tests/compiler/dart2js/rti/data/local_function_signatures_strong.dart

@@ -12,7 +12,7 @@ class Class1 {
   }
 
   method2() {
-    /*needsSignature*/
+    /**/
     num local<T>(int n) => null;
     return local;
   }
@@ -43,9 +43,9 @@ class Class3 {
 }
 
 class Class4 {
-  /*element: Class4.method6:needsArgs,selectors=[Selector(call, method6, arity=0, types=1)]*/
+  /*element: Class4.method6:*/
   method6<T>() {
-    /*needsSignature*/
+    /**/
     num local(num n, T t) => null;
     return local;
   }
@@ -65,9 +65,9 @@ method8<T>() {
   return local;
 }
 
-/*element: method9:needsArgs*/
+/*element: method9:*/
 method9<T>() {
-  /*needsSignature*/
+  /**/
   num local(num n, T t) => null;
   return local;
 }
@@ -85,7 +85,7 @@ method11() {
 }
 
 method12() {
-  /*needsSignature*/
+  /**/
   num local<T>(num n, T t) => null;
   return local;
 }

tests/compiler/dart2js/rti/emission/closure_function.dart

@@ -8,6 +8,9 @@ import 'package:meta/dart2js.dart';
 test(o) => o is Function;
 
 main() {
-  test(/*checks=[],functionType,instance*/ () {});
+  test(
+      /*ast.checks=[],functionType,instance*/
+      /*kernel.checks=[],functionType,instance*/
+      /*strong.checks=[],instance*/ () {});
   test(null);
 }

tests/compiler/dart2js/rti/emission/closure_signature_unneeded.dart

@@ -10,7 +10,11 @@ class A<T> {
   m() {
     // TODO(johnniwinther): The signature is not needed since the type isn't a
     // potential subtype of the checked function types.
-    return /*checks=[$signature],instance*/ (T t, String s) {};
+    return
+        /*ast.checks=[$signature],instance*/
+        /*kernel.checks=[$signature],instance*/
+        /*strong.checks=[],instance*/
+        (T t, String s) {};
   }
 }
 

tests/compiler/dart2js/rti/emission/local_function_list_literal_strong.dart

@@ -8,7 +8,7 @@ import 'package:expect/expect.dart';
 
 @NoInline()
 method<T>() {
-  return /*checks=[$signature],instance*/ () => <T>[];
+  return /*checks=[],instance*/ () => <T>[];
 }
 
 @NoInline()

tests/compiler/dart2js/rti/emission/local_function_map_literal_strong.dart

@@ -9,7 +9,7 @@ import 'package:expect/expect.dart';
 @NoInline()
 method<T>() {
   return
-      /*checks=[$signature],instance*/
+      /*checks=[],instance*/
       () => <T, int>{};
 }
 

tests/compiler/dart2js_extra/local_function_signatures_strong_test.dart

@@ -74,17 +74,20 @@ method12() {
   return local;
 }
 
-num Function(num) method13() {
+num Function(num) //# 01: ok
+    method13() {
   num local<T>(num n) => null;
   return local;
 }
 
-num Function(num) method14() {
+num Function(num) //# 01: continued
+    method14() {
   num local<T>(T n) => null;
   return local;
 }
 
-num Function(num) method15() {
+num Function(num) //# 01: continued
+    method15() {
   T local<T>(num n) => null;
   return local;
 }
@@ -105,7 +108,7 @@ main() {
   Expect.isFalse(test(method10()));
   Expect.isFalse(test(method11()));
   Expect.isFalse(test(method12()));
-  Expect.isTrue(test(method13()));
-  Expect.isTrue(test(method14()));
-  Expect.isTrue(test(method15()));
+  Expect.isTrue(test(method13())); //# 01: continued
+  Expect.isTrue(test(method14())); //# 01: continued
+  Expect.isTrue(test(method15())); //# 01: continued
 }