Enhance precision of recorded instantiation info in ResolutionWorldBuilder

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

Index: pkg/compiler/lib/src/universe/world_builder.dart
diff --git a/pkg/compiler/lib/src/universe/world_builder.dart b/pkg/compiler/lib/src/universe/world_builder.dart
index fa4108aa282ab8ce8ca2fcb149cb9862be2891a1..730de5ff3dc56067b1a1232f7157d224a4882866 100644
--- a/pkg/compiler/lib/src/universe/world_builder.dart
+++ b/pkg/compiler/lib/src/universe/world_builder.dart
@@ -164,9 +164,10 @@ abstract class ResolutionWorldBuilder implements WorldBuilder {
   /// Set of all fields that are statically known to be written to.
   Iterable<Element> get fieldSetters;
 
-  /// Call [f] for all directly or abstractly instantiated classes.
-  void forEachInstantiatedClass(
-      f(ClassElement cls, EnumSet<Instantiation> instantiations));
+  /// Call [f] for all classes with instantiated types. This includes the
+  /// directly and abstractly instantiated classes but also classes whose type
+  /// arguments are used in live factory constructors.
+  void forEachInstantiatedClass(f(ClassElement cls, InstantiationInfo info));
 
   /// `true` of `Object.runtimeType` is supported.
   bool get hasRuntimeTypeSupport;
@@ -182,22 +183,136 @@ abstract class ResolutionWorldBuilder implements WorldBuilder {
   OpenWorld get openWorld;
 }
 
-class ResolutionWorldBuilderImpl implements ResolutionWorldBuilder {
-  /// The set of all directly instantiated classes, that is, classes with a
-  /// generative constructor that has been called directly and not only through
-  /// a super-call.
+/// The type and kind of a register instantiation.

[Harry Terkelsen, 2016/11/18 00:11:35]

what do you mean by "register instantiation"?

[Johnni Winther, 2016/11/18 09:52:49]

register -> registered

+class Instance {
+  final InterfaceType type;
+  final Instantiation kind;
+  final bool isRedirection;
+
+  Instance(this.type, this.kind, {this.isRedirection: false});
+
+  int get hashCode {
+    return Hashing.objectHash(
+        type, Hashing.objectHash(kind, Hashing.objectHash(isRedirection)));
+  }
+
+  bool operator ==(other) {
+    if (identical(this, other)) return true;
+    if (other is! Instance) return false;
+    return type == other.type &&
+        kind == other.kind &&
+        isRedirection == other.isRedirection;
+  }
+
+  String toString() {
+    StringBuffer sb = new StringBuffer();
+    sb.write(type);
+    if (kind == Instantiation.DIRECTLY_INSTANTIATED) {
+      sb.write(' directly');
+    } else if (kind == Instantiation.ABSTRACTLY_INSTANTIATED) {
+      sb.write(' abstractly');
+    }
+    if (isRedirection) {
+      sb.write(' redirect');
+    }
+    return sb.toString();
+  }
+}
+
+/// Information about instantiations of a class.
+class InstantiationInfo {
+  /// A map from constructor of the class to their instantiated types.
   ///
-  /// Invariant: Elements are declaration elements.
-  // TODO(johnniwinther): [_directlyInstantiatedClasses] and
-  // [_instantiatedTypes] sets should be merged.
-  final Map<ClassElement, EnumSet<Instantiation>> _directlyInstantiatedClasses =
-      <ClassElement, EnumSet<Instantiation>>{};
+  /// For instance
+  ///
+  ///    class Class<T> {
+  ///      Class.a();
+  ///      Class.b();
+  ///      Class.c() = Class.b<T>;
+  ///    }
+  ///
+  ///    main() {
+  ///      new Class.a(),
+  ///      new Class<int>.a(),
+  ///      new Class<String>.b()
+  ///      new Class<num>.c()
+  ///    }
+  ///
+  /// will generate the map
+  ///
+  ///    {
+  ///      Class.a: { Class directly, Class<int> directly },
+  ///      Class.b: { Class<String> directly, Class<T> directly redirect },
+  ///      Class.c: { Class<num> directly },
+  ///    }

[Harry Terkelsen, 2016/11/18 00:11:35]

consider also giving an example of a class that is instantiated "abstractly"

[Johnni Winther, 2016/11/18 09:52:49]

Done.

+  ///
+  /// If the constructor is unknown, for instance for native or mirror usage,
+  /// `null` is used as key.
+  Map<ConstructorElement, Set<Instance>> instantiationMap;
+
+  /// Register [type] as the instantiation [kind] using [constructor].
+  void addInstantiation(
+      ConstructorElement constructor, InterfaceType type, Instantiation kind,
+      {bool isRedirection: false}) {
+    instantiationMap ??= new Maplet<ConstructorElement, Set<Instance>>();

[Harry Terkelsen, 2016/11/18 00:11:35]

are maplets still better than maps? I think there have been optimizations for
maps in the vm where if they are small (< 16 keys) they are represented as
lists. same for setlets

[Johnni Winther, 2016/11/18 09:52:49]

Changed.

+    instantiationMap
+        .putIfAbsent(constructor, () => new Setlet<Instance>())
+        .add(new Instance(type, kind, isRedirection: isRedirection));
+    switch (kind) {
+      case Instantiation.DIRECTLY_INSTANTIATED:
+        isDirectlyInstantiated = true;
+        break;
+      case Instantiation.ABSTRACTLY_INSTANTIATED:
+        isAbstractlyInstantiated = true;
+        break;
+      case Instantiation.UNINSTANTIATED:
+        break;
+      default:
+        throw new StateError("Instantiation $kind is not allowed.");
+    }
+  }
 
-  /// The set of all directly instantiated types, that is, the types of the
-  /// directly instantiated classes.
+  /// `true` if the class is either directly or abstractly instantiated.
+  bool get hasInstantiation =>
+      isDirectlyInstantiated || isAbstractlyInstantiated;
+
+  /// `true` if the class is directly instantiated.
+  bool isDirectlyInstantiated = false;
+
+  /// `true` if the class is abstractly instantiated.
+  bool isAbstractlyInstantiated = false;
+
+  String toString() {
+    StringBuffer sb = new StringBuffer();
+    sb.write('InstantiationInfo[');
+    if (instantiationMap != null) {
+      bool needsComma = false;
+      instantiationMap
+          .forEach((ConstructorElement constructor, Set<Instance> set) {
+        if (needsComma) {
+          sb.write(', ');
+        }
+        if (constructor != null) {
+          sb.write(constructor);
+        } else {
+          sb.write('<unknown>');
+        }
+        sb.write(': ');
+        sb.write(set);
+        needsComma = true;
+      });
+    }
+    sb.write(']');
+    return sb.toString();
+  }
+}
+
+class ResolutionWorldBuilderImpl implements ResolutionWorldBuilder {
+  /// Instantiation information for all classes with instantiated types.
   ///
-  /// See [_directlyInstantiatedClasses].
-  final Set<DartType> _instantiatedTypes = new Set<DartType>();
+  /// Invariant: Elements are declaration elements.
+  final Map<ClassElement, InstantiationInfo> _instantiationInfo =
+      <ClassElement, InstantiationInfo>{};
 
   /// Classes implemented by directly instantiated classes.
   final Set<ClassElement> _implementedClasses = new Set<ClassElement>();
@@ -272,7 +387,13 @@ class ResolutionWorldBuilderImpl implements ResolutionWorldBuilder {
   /// super-call.
   // TODO(johnniwinther): Improve semantic precision.
   Iterable<ClassElement> get directlyInstantiatedClasses {
-    return _directlyInstantiatedClasses.keys;
+    Set<ClassElement> classes = new Set<ClassElement>();
+    _instantiationInfo.forEach((ClassElement cls, InstantiationInfo info) {
+      if (info.hasInstantiation) {
+        classes.add(cls);
+      }
+    });
+    return classes;
   }
 
   /// All directly instantiated types, that is, the types of the directly
@@ -280,7 +401,19 @@ class ResolutionWorldBuilderImpl implements ResolutionWorldBuilder {
   ///
   /// See [directlyInstantiatedClasses].
   // TODO(johnniwinther): Improve semantic precision.
-  Iterable<DartType> get instantiatedTypes => _instantiatedTypes;
+  Iterable<DartType> get instantiatedTypes {
+    Set<InterfaceType> types = new Set<InterfaceType>();
+    _instantiationInfo.forEach((_, InstantiationInfo info) {
+      if (info.instantiationMap != null) {
+        for (Set<Instance> instances in info.instantiationMap.values) {
+          for (Instance instance in instances) {
+            types.add(instance.type);
+          }
+        }
+      }
+    });
+    return types;
+  }
 
   /// Returns `true` if [cls] is considered to be implemented by an
   /// instantiated class, either directly, through subclasses or through
@@ -298,11 +431,15 @@ class ResolutionWorldBuilderImpl implements ResolutionWorldBuilder {
   // subclass and through subtype instantiated types/classes.
   // TODO(johnniwinther): Support unknown type arguments for generic types.
   void registerTypeInstantiation(InterfaceType type,
-      {bool byMirrors: false,
+      {ConstructorElement constructor,
+      bool byMirrors: false,
       bool isNative: false,
+      bool isRedirection: false,
       void onImplemented(ClassElement cls)}) {
-    _instantiatedTypes.add(type);
     ClassElement cls = type.element;
+    InstantiationInfo info =
+        _instantiationInfo.putIfAbsent(cls, () => new InstantiationInfo());
+    Instantiation kind = Instantiation.UNINSTANTIATED;
     if (!cls.isAbstract
         // We can't use the closed-world assumption with native abstract
         // classes; a native abstract class may have non-abstract subclasses
@@ -315,17 +452,17 @@ class ResolutionWorldBuilderImpl implements ResolutionWorldBuilder {
         // TODO(herhut): Track classes required by mirrors seperately.
         ||
         byMirrors) {
-      EnumSet<Instantiation> instantiations = _directlyInstantiatedClasses
-          .putIfAbsent(cls, () => new EnumSet<Instantiation>());
       if (isNative || byMirrors) {
-        instantiations.add(Instantiation.ABSTRACTLY_INSTANTIATED);
+        kind = Instantiation.ABSTRACTLY_INSTANTIATED;
       } else {
-        instantiations.add(Instantiation.DIRECTLY_INSTANTIATED);
+        kind = Instantiation.DIRECTLY_INSTANTIATED;
       }
     }
+    info.addInstantiation(constructor, type, kind,
+        isRedirection: isRedirection);
 
-    // TODO(johnniwinther): Replace this by separate more specific mappings that
-    // include the type arguments.
+    // TODO(johnniwinther): Use [_instantiationInfo] to compute this information
+    // instead.
     if (_implementedClasses.add(cls)) {
       onImplemented(cls);
       cls.allSupertypes.forEach((InterfaceType supertype) {
@@ -337,9 +474,8 @@ class ResolutionWorldBuilderImpl implements ResolutionWorldBuilder {
   }
 
   @override
-  void forEachInstantiatedClass(
-      f(ClassElement cls, EnumSet<Instantiation> instantiations)) {
-    _directlyInstantiatedClasses.forEach(f);
+  void forEachInstantiatedClass(f(ClassElement cls, InstantiationInfo info)) {
+    _instantiationInfo.forEach(f);
   }
 
   bool _hasMatchingSelector(Map<Selector, SelectorConstraints> selectors,
@@ -422,6 +558,7 @@ class ResolutionWorldBuilderImpl implements ResolutionWorldBuilder {
       case StaticUseKind.FIELD_GET:
       case StaticUseKind.CONSTRUCTOR_INVOKE:
       case StaticUseKind.CONST_CONSTRUCTOR_INVOKE:
+      case StaticUseKind.REDIRECTION:
         break;
       case StaticUseKind.CLOSURE:
         allClosures.add(element);
@@ -438,12 +575,7 @@ class ResolutionWorldBuilderImpl implements ResolutionWorldBuilder {
     slowDirectlyNestedClosures(element).forEach(compiler.forgetElement);
     closurizedMembers.remove(element);
     fieldSetters.remove(element);
-    _directlyInstantiatedClasses.remove(element);
-    if (element is ClassElement) {
-      assert(invariant(element, element.thisType.isRaw,
-          message: 'Generic classes not supported (${element.thisType}).'));
-      _instantiatedTypes..remove(element.rawType)..remove(element.thisType);
-    }
+    _instantiationInfo.remove(element);
   }
 
   // TODO(ahe): Replace this method with something that is O(1), for example,
@@ -701,6 +833,7 @@ class CodegenWorldBuilderImpl implements CodegenWorldBuilder {
       case StaticUseKind.FIELD_GET:
       case StaticUseKind.CONSTRUCTOR_INVOKE:
       case StaticUseKind.CONST_CONSTRUCTOR_INVOKE:
+      case StaticUseKind.REDIRECTION:
       case StaticUseKind.DIRECT_INVOKE:
         break;
     }