Cleanup registration of closures

pkg/compiler/lib/src/universe/resolution_world_builder.dart

 // Copyright (c) 2017, 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.
 
 part of world_builder;
 
 abstract class ResolutionWorldBuilder implements WorldBuilder, OpenWorld {
+  /// Set of all local functions in the program. Used by the mirror tracking
+  /// system to find all live closure instances.
+  Iterable<LocalFunctionElement> get localFunctions;
+
   /// Set of (live) local functions (closures) whose signatures reference type

[Siggi Cherem (dart-lang), 2017/03/14 03:02:34]

Just to be sure: this is only for RTI and so it includes functions that mention
a free <T> in the argument types or return type? (and will not include a closure
that has a free type-variable in the body), correct?

For example in:

class A<T> {
  m() {
    void localFunction() { return T; }
  }
}

localFunction will not be added here, right?


The name of the field is a bit ambiguous in terms of this. adding "signature" to
the name will just make it so long that it's not worth adding it there, but it
might be worth thinking of other ideas. I don't have any good ones at the moment
:(

[Johnni Winther, 2017/03/14 16:59:46]

Yes

   /// variables.
   ///
   /// A live function is one whose enclosing member function has been enqueued.
-  Iterable<Element> get closuresWithFreeTypeVariables;
+  Iterable<LocalFunctionElement> get localFunctionsWithFreeTypeVariables;
 
-  /// Set of (live) `call` methods whose signatures reference type variables.
-  ///
-  /// A live `call` method is one whose enclosing class has been instantiated.
-  Iterable<Element> get callMethodsWithFreeTypeVariables;
-
-  /// Set of all closures in the program. Used by the mirror tracking system
-  /// to find all live closure instances.
-  Iterable<LocalFunctionElement> get allClosures;
+  /// Set of live local functions (closures) whose signatures reference type
+  /// variables.
+  /// A local function is considered live if the enclosing member function is

[Siggi Cherem (dart-lang), 2017/03/14 03:02:34]

nit: add extra line to divide the paragraph

[Johnni Winther, 2017/03/14 16:59:46]

Duplicate paragraph, removed.

+  /// live.
 
   /// Set of methods in instantiated classes that are potentially closurized.
-  Iterable<Element> get closurizedMembers;
+  Iterable<MethodElement> get closurizedMembers;
+
+  /// Set of live closurized members whose signatures reference type variables.
+  ///
+  /// A `call` method is considered live if the enclosing class has been

[Siggi Cherem (dart-lang), 2017/03/14 03:02:34]

`call` method => closurized member

[Johnni Winther, 2017/03/14 16:59:46]

Done.

+  /// instantiated.
+  Iterable<MethodElement> get closurizedMembersWithFreeTypeVariables;
 
   /// Returns `true` if [cls] is considered to be implemented by an
   /// instantiated class, either directly, through subclasses or through
   /// subtypes. The latter case only contains spurious information from
   /// instantiations through factory constructors and mixins.
   bool isImplemented(ClassElement cls);
 
   /// Set of all fields that are statically known to be written to.
   Iterable<Element> get fieldSetters;
 
@@ skipping 15 matching lines @@
   /// This is only available after the world builder has been closed.
   ClosedWorld get closedWorldForTesting;
 }
 
 /// Extended [ResolutionWorldBuilder] interface used by the
 /// [ResolutionEnqueuer].
 abstract class ResolutionEnqueuerWorldBuilder extends ResolutionWorldBuilder {
   /// Returns the classes registered as directly or indirectly instantiated.
   Iterable<ClassEntity> get processedClasses;
 
-  /// Registers that the generic [element] has been closurized.
-  void registerClosureWithFreeTypeVariables(MemberEntity element);
-
   /// Registers that [element] has been closurized.
   void registerClosurizedMember(MemberEntity element);
 
   /// Register [type] as (directly) instantiated.
   ///
   /// If [byMirrors] is `true`, the instantiation is through mirrors.
   // TODO(johnniwinther): Fully enforce the separation between exact, through
   // subclass and through subtype instantiated types/classes.
   // TODO(johnniwinther): Support unknown type arguments for generic types.
   void registerTypeInstantiation(
@@ skipping 223 matching lines @@
 
   /// Map containing instance methods of live classes that are not yet
   /// closurized.
   final Map<String, Set<_MemberUsage>> _instanceFunctionsByName =
       <String, Set<_MemberUsage>>{};
 
   /// Fields set.
   final Set<Element> fieldSetters = new Set<Element>();
   final Set<ResolutionDartType> isChecks = new Set<ResolutionDartType>();
 
-  /**
-   * Set of (live) [:call:] methods whose signatures reference type variables.
-   *
-   * A live [:call:] method is one whose enclosing class has been instantiated.
-   */
-  final Set<Element> callMethodsWithFreeTypeVariables = new Set<Element>();
+  /// Set of all closures in the program. Used by the mirror tracking system
+  /// to find all live closure instances.
+  final Set<LocalFunctionElement> localFunctions =
+      new Set<LocalFunctionElement>();
 
-  /**
-   * Set of (live) local functions (closures) whose signatures reference type
-   * variables.
-   *
-   * A live function is one whose enclosing member function has been enqueued.
-   */
-  final Set<Element> closuresWithFreeTypeVariables = new Set<Element>();
+  /// Set of live local functions (closures) whose signatures reference type
+  /// variables.
+  /// A local function is considered live if the enclosing member function is

[Siggi Cherem (dart-lang), 2017/03/14 03:02:34]

\n

[Johnni Winther, 2017/03/14 16:59:46]

Done.

+  /// live.
+  final Set<LocalFunctionElement> localFunctionsWithFreeTypeVariables =
+      new Set<LocalFunctionElement>();
 
-  /**
-   * Set of all closures in the program. Used by the mirror tracking system
-   * to find all live closure instances.
-   */
-  final Set<LocalFunctionElement> allClosures = new Set<LocalFunctionElement>();
+  /// Set of methods in instantiated classes that are potentially closurized.
+  final Set<MethodElement> closurizedMembers = new Set<MethodElement>();
 
-  /**
-   * Set of methods in instantiated classes that are potentially
-   * closurized.
-   */
-  final Set<Element> closurizedMembers = new Set<Element>();
+  /// Set of live closurized members whose signatures reference type variables.
+  ///
+  /// A `call` method is considered live if the enclosing class has been

[Siggi Cherem (dart-lang), 2017/03/14 03:02:34]

`call` method => closurized member

[Johnni Winther, 2017/03/14 16:59:46]

Done.

+  /// instantiated.
+  final Set<MethodElement> closurizedMembersWithFreeTypeVariables =
+      new Set<MethodElement>();
 
   final SelectorConstraintsStrategy selectorConstraintsStrategy;
 
   bool hasRuntimeTypeSupport = false;
   bool hasIsolateSupport = false;
   bool hasFunctionApplySupport = false;
 
   /// Used for testing the new more precise computation of instantiated types
   /// and classes.
   bool useInstantiationMap = false;
@@ skipping 70 matching lines @@
 
   /// Returns `true` if [cls] is considered to be implemented by an
   /// instantiated class, either directly, through subclasses or through
   /// subtypes. The latter case only contains spurious information from
   /// instantiations through factory constructors and mixins.
   // TODO(johnniwinther): Improve semantic precision.
   bool isImplemented(ClassElement cls) {
     return _implementedClasses.contains(cls.declaration);
   }
 
-  void registerClosureWithFreeTypeVariables(MemberElement element) {
-    closuresWithFreeTypeVariables.add(element);
-  }
-
   void registerClosurizedMember(MemberElement element) {
     closurizedMembers.add(element);
+    if (element.type.containsTypeVariables) {
+      closurizedMembersWithFreeTypeVariables.add(element);
+    }
   }
 
   /// Register [type] as (directly) instantiated.
   ///
   /// If [byMirrors] is `true`, the instantiation is through mirrors.
   // TODO(johnniwinther): Fully enforce the separation between exact, through
   // subclass and through subtype instantiated types/classes.
   // TODO(johnniwinther): Support unknown type arguments for generic types.
   void registerTypeInstantiation(
       ResolutionInterfaceType type, ClassUsedCallback classUsed,
@@ skipping 195 matching lines @@
     // [FIELD_SET] contains [BoxFieldElement]s which we cannot enqueue.
     // Also [CLOSURE] contains [LocalFunctionElement] which we cannot
     // enqueue.
     switch (staticUse.kind) {
       case StaticUseKind.FIELD_GET:
         break;
       case StaticUseKind.FIELD_SET:
         fieldSetters.add(element);
         break;
       case StaticUseKind.CLOSURE:
-        LocalFunctionElement closure = staticUse.element;
-        if (closure.type.containsTypeVariables) {
-          closuresWithFreeTypeVariables.add(closure);
+        LocalFunctionElement localFunction = staticUse.element;
+        if (localFunction.type.containsTypeVariables) {
+          localFunctionsWithFreeTypeVariables.add(localFunction);
         }
-        allClosures.add(element);
+        localFunctions.add(element);
         break;
       case StaticUseKind.SUPER_TEAR_OFF:
         useSet.addAll(usage.tearOff());
         methodsNeedingSuperGetter.add(element);
         break;
       case StaticUseKind.SUPER_FIELD_SET:
         fieldSetters.add(element);
         useSet.addAll(usage.normalUse());
         break;
       case StaticUseKind.STATIC_TEAR_OFF:
         useSet.addAll(usage.tearOff());
         break;
       case StaticUseKind.GENERAL:
       case StaticUseKind.DIRECT_USE:
       case StaticUseKind.CONSTRUCTOR_INVOKE:
       case StaticUseKind.CONST_CONSTRUCTOR_INVOKE:
       case StaticUseKind.REDIRECTION:
         useSet.addAll(usage.normalUse());
         break;
       case StaticUseKind.DIRECT_INVOKE:
         invariant(
             element, 'Direct static use is not supported for resolution.');
         break;
     }
     if (useSet.isNotEmpty) {
       memberUsed(usage.entity, useSet);
     }
   }
 
-  // TODO(ahe): Replace this method with something that is O(1), for example,
-  // by using a map.
-  List<LocalFunctionElement> slowDirectlyNestedClosures(Element element) {
-    // Return new list to guard against concurrent modifications.
-    return new List<LocalFunctionElement>.from(
-        allClosures.where((LocalFunctionElement closure) {
-      return closure.executableContext == element;
-    }));
-  }
-
   /// Return the canonical [_ClassUsage] for [cls].
   _ClassUsage _getClassUsage(ClassElement cls) {
     return _processedClasses.putIfAbsent(cls, () {
       cls.ensureResolved(_resolution);
       _ClassUsage usage = new _ClassUsage(cls);
       _resolution.ensureClassMembers(cls);
       return usage;
     });
   }
 
@@ skipping 60 matching lines @@
       bool isNative = _backend.nativeData.isNativeClass(cls);
       _MemberUsage usage = new _MemberUsage(member, isNative: isNative);
       EnumSet<MemberUse> useSet = new EnumSet<MemberUse>();
       useSet.addAll(usage.appliedUse);
       if (member.isField && isNative) {
         registerUsedElement(member);
       }
       if (member.isFunction &&
           member.name == Identifiers.call &&
           !cls.typeVariables.isEmpty) {
-        callMethodsWithFreeTypeVariables.add(member);
+        closurizedMembersWithFreeTypeVariables.add(member);
       }
 
       if (_hasInvokedGetter(member)) {
         useSet.addAll(usage.read());
       }
       if (_hasInvocation(member)) {
         useSet.addAll(usage.invoke());
       }
       if (hasInvokedSetter(member)) {
         useSet.addAll(usage.write());
@@ skipping 181 matching lines @@
   @override
   bool isMemberUsed(MemberEntity member) {
     if (member.isInstanceMember) {
       _MemberUsage usage = _instanceMemberUsage[member];
       if (usage != null && usage.hasUse) return true;
     }
     _StaticMemberUsage usage = _staticMemberUsage[member];
     return usage != null && usage.hasUse;
   }
 }