Cleanup of function_set.dart

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

Index: pkg/compiler/lib/src/universe/function_set.dart
diff --git a/pkg/compiler/lib/src/universe/function_set.dart b/pkg/compiler/lib/src/universe/function_set.dart
index 0b4b0bc9990cddaea99aa8426fa507bc4e1e7696..a6db16986f88927064b2ffb1d6167819c948b844 100644
--- a/pkg/compiler/lib/src/universe/function_set.dart
+++ b/pkg/compiler/lib/src/universe/function_set.dart
@@ -13,6 +13,8 @@ class FunctionSet {
       new Map<String, FunctionSetNode>();
   FunctionSet(this.compiler);
 
+  ClassWorld get classWorld => compiler.world;
+
   FunctionSetNode newNode(String name)
       => new FunctionSetNode(name);
 
@@ -44,30 +46,50 @@ class FunctionSet {
         : false;
   }
 
-  /**
-   * Returns an object that allows iterating over all the functions
-   * that may be invoked with the given [selector].
-   */
+  /// Returns an object that allows iterating over all the functions
+  /// that may be invoked with the given [selector].
   Iterable<Element> filter(Selector selector, TypeMask mask) {
     return query(selector, mask).functions;
   }
 
+  /// Returns the mask for the potential receivers of a dynamic call to
+  /// [selector] on [mask].
+  ///
+  /// This will reduces the set of classes in [mask] to a [TypeMask] of the set

[karlklose, 2015/09/01 07:47:03]

'reduces' -> 'reduce'.

[Johnni Winther, 2015/09/01 11:54:47]

Done.

+  /// of classes that actually implement the selected member or implement the
+  /// handling 'noSuchMethod' where the selected member is unimplemented.
   TypeMask receiverType(Selector selector, TypeMask mask) {
-    return query(selector, mask).computeMask(compiler.world);
+    return query(selector, mask).computeMask(classWorld);
   }
 
+  SelectorMask _createSelectorMask(
+      Selector selector, TypeMask mask, ClassWorld classWorld) {
+    return mask != null
+        ? new SelectorMask(selector, mask)
+        : new SelectorMask(selector,
+            new TypeMask.subclass(classWorld.objectClass, classWorld));
+  }
+
+  /// Returns the set of functions that can be the target of a call to
+  /// [selector] on a receiver of type [mask] including 'noSuchMethod' methods
+  /// where applicable.
   FunctionSetQuery query(Selector selector, TypeMask mask) {
     String name = selector.name;
+    SelectorMask selectorMask = _createSelectorMask(selector, mask, classWorld);
+    SelectorMask noSuchMethodMask =
+        new SelectorMask(Selectors.noSuchMethod_, selectorMask.mask);
     FunctionSetNode node = nodes[name];
     FunctionSetNode noSuchMethods = nodes[Identifiers.noSuchMethod_];
     if (node != null) {
-      return node.query(selector, mask, compiler, noSuchMethods);
+      return node.query(
+          selectorMask, classWorld, noSuchMethods, noSuchMethodMask);
     }
     // If there is no method that matches [selector] we know we can
     // only hit [:noSuchMethod:].
-    if (noSuchMethods == null) return const FunctionSetQuery(const <Element>[]);
-    return noSuchMethods.query(
-        Selectors.noSuchMethod_, mask, compiler, null);
+    if (noSuchMethods == null) {
+      return const FunctionSetQuery();
+    }
+    return noSuchMethods.query(noSuchMethodMask, classWorld);
   }
 
   void forEach(Function action) {
@@ -77,6 +99,8 @@ class FunctionSet {
   }
 }
 
+/// A selector/mask pair representing the dynamic invocation of [selector] on
+/// a receiver of type [mask].
 class SelectorMask {
   final Selector selector;
   final TypeMask mask;
@@ -86,16 +110,21 @@ class SelectorMask {
       : this.selector = selector,
         this.mask = mask,
         this.hashCode =
-            Hashing.mixHashCodeBits(selector.hashCode, mask.hashCode);
+            Hashing.mixHashCodeBits(selector.hashCode, mask.hashCode) {
+    assert(mask != null);

[karlklose, 2015/09/01 07:47:03]

The initializer for hashCode uses mask as receiver, so the constructor will
throw for mask == null already.

[Johnni Winther, 2015/09/01 11:54:47]

Null implements hashCode so it will not fail.

+  }
 
   String get name => selector.name;
 
   bool applies(Element element, ClassWorld classWorld) {
     if (!selector.appliesUnnamed(element, classWorld)) return false;
-    if (mask == null) return true;
     return mask.canHit(element, selector, classWorld);
   }
 
+  bool needsNoSuchMethodHandling(ClassWorld classWorld) {
+    return mask.needsNoSuchMethodHandling(selector, classWorld);
+  }
+
   bool operator ==(other) {
     if (identical(this, other)) return true;
     return selector == other.selector && mask == other.mask;
@@ -104,6 +133,8 @@ class SelectorMask {
   String toString() => '($selector,$mask)';
 }
 
+/// A node in the [FunctionSet] caching all [FunctionSetQuery] object for
+/// selectors with the same [name].
 class FunctionSetNode {
   final String name;
   final Map<SelectorMask, FunctionSetQuery> cache =
@@ -164,24 +195,13 @@ class FunctionSetNode {
     elements.forEach(action);
   }
 
-  TypeMask getNonNullTypeMaskOfSelector(TypeMask mask, ClassWorld classWorld) {
-    // TODO(ngeoffray): We should probably change untyped selector
-    // to always be a subclass of Object.
-    return mask != null
-        ? mask
-        : new TypeMask.subclass(classWorld.objectClass, classWorld);
-    }
-
-  // TODO(johnniwinther): Use [SelectorMask] instead of [Selector] and
-  // [TypeMask].
-  FunctionSetQuery query(Selector selector,
-                         TypeMask mask,
-                         Compiler compiler,
-                         FunctionSetNode noSuchMethods) {
-    mask = getNonNullTypeMaskOfSelector(mask, compiler.world);
-    SelectorMask selectorMask = new SelectorMask(selector, mask);
-    ClassWorld classWorld = compiler.world;
-    assert(selector.name == name);
+  /// Returns the set of functions that can be the target of [selectorMask]
+  /// including no such method handling where applicable.
+  FunctionSetQuery query(SelectorMask selectorMask,
+                         ClassWorld classWorld,
+                         [FunctionSetNode noSuchMethods,
+                          SelectorMask noSuchMethodMask]) {
+    assert(selectorMask.name == name);
     FunctionSetQuery result = cache[selectorMask];
     if (result != null) return result;
 
@@ -201,13 +221,10 @@ class FunctionSetNode {
     // If we cannot ensure a method will be found at runtime, we also
     // add [noSuchMethod] implementations that apply to [mask] as
     // potential targets.
-    if (noSuchMethods != null
-        && mask.needsNoSuchMethodHandling(selector, classWorld)) {
-      FunctionSetQuery noSuchMethodQuery = noSuchMethods.query(
-          Selectors.noSuchMethod_,
-          mask,
-          compiler,
-          null);
+    if (noSuchMethods != null &&
+        selectorMask.needsNoSuchMethodHandling(classWorld)) {
+      FunctionSetQuery noSuchMethodQuery =
+          noSuchMethods.query(noSuchMethodMask, classWorld);
       if (!noSuchMethodQuery.functions.isEmpty) {
         if (functions == null) {
           functions = new Setlet<Element>.from(noSuchMethodQuery.functions);
@@ -217,31 +234,30 @@ class FunctionSetNode {
       }
     }
     cache[selectorMask] = result = (functions != null)
-        ? newQuery(functions, selector, mask, compiler)
-        : const FunctionSetQuery(const <Element>[]);
+        ? new FullFunctionSetQuery(functions)
+        : const FunctionSetQuery();
     return result;
   }
-
-  FunctionSetQuery newQuery(Iterable<Element> functions,
-                            Selector selector,
-                            TypeMask mask,
-                            Compiler compiler) {
-    return new FullFunctionSetQuery(functions);
-  }
 }
 
+/// A set of function that are the potential targets of all call sites sharing

[karlklose, 2015/09/01 07:47:03]

'function' -> 'functions'.

[Johnni Winther, 2015/09/01 11:54:47]

Done.

+/// the same receiver mask and selector.
 class FunctionSetQuery {
-  final Iterable<Element> functions;
+  const FunctionSetQuery();
+
+  /// Compute the type of all potential receivers of this function set.
   TypeMask computeMask(ClassWorld classWorld) => const TypeMask.nonNullEmpty();
-  const FunctionSetQuery(this.functions);
+
+  /// Returns all potential targets of this function set.
+  Iterable<Element> get functions => const <Element>[];
 }
 
 class FullFunctionSetQuery extends FunctionSetQuery {
+  final Iterable<Element> _functions;
   TypeMask _mask;
 
-  /**
-   * Compute the type of all potential receivers of this function set.
-   */
+  FullFunctionSetQuery(this._functions);
+
   TypeMask computeMask(ClassWorld classWorld) {
     assert(classWorld.hasAnyStrictSubclass(classWorld.objectClass));
     if (_mask != null) return _mask;
@@ -259,6 +275,4 @@ class FullFunctionSetQuery extends FunctionSetQuery {
         }),
         classWorld);
   }
-
-  FullFunctionSetQuery(functions) : super(functions);
 }