Decouple TypeMask from ClassElement

pkg/compiler/lib/src/world.dart

 // Copyright (c) 2012, 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.
 
 library dart2js.world;
 
 import 'closure.dart' show SynthesizedCallMethodElementX;
 import 'common/backend_api.dart' show BackendClasses;
 import 'common.dart';
 import 'compiler.dart' show Compiler;
@@ skipping 37 matching lines @@
   /// Returns `true` if [cls] is either directly or indirectly instantiated.
   bool isInstantiated(ClassElement cls);
 
   /// Returns `true` if [cls] is directly instantiated.
   bool isDirectlyInstantiated(ClassElement cls);
 
   /// Returns `true` if [cls] is indirectly instantiated, that is through a
   /// subclass.
   bool isIndirectlyInstantiated(ClassElement cls);
 
+  /// Returns `true` if [cls] is abstract and thus can only be instantiated
+  /// through subclasses.
+  bool isAbstract(ClassElement cls);
+
   /// Returns `true` if [cls] is implemented by an instantiated class.
   bool isImplemented(ClassElement cls);
 
   /// Return `true` if [x] is a subclass of [y].
   bool isSubclassOf(ClassElement x, ClassElement y);
 
   /// Returns `true` if [x] is a subtype of [y], that is, if [x] implements an
   /// instance of [y].
   bool isSubtypeOf(ClassElement x, ClassElement y);
 
@@ skipping 81 matching lines @@
   /// Returns `true` if [cls] or any superclass mixes in [mixin].
   bool isSubclassOfMixinUseOf(ClassElement cls, ClassElement mixin);
 
   /// Returns `true` if every subtype of [x] is a subclass of [y] or a subclass
   /// of a mixin application of [y].
   bool everySubtypeIsSubclassOfOrMixinUseOf(ClassElement x, ClassElement y);
 
   /// Returns `true` if any subclass of [superclass] implements [type].
   bool hasAnySubclassThatImplements(ClassElement superclass, ClassElement type);
 
+  /// Returns `true` if a call of [selector] on [cls] and/or subclasses/subtypes
+  /// need noSuchMethod handling.
+  ///
+  /// If the receiver is guaranteed to have a member that matches what we're
+  /// looking for, there's no need to introduce a noSuchMethod handler. It will
+  /// never be called.
+  ///
+  /// As an example, consider this class hierarchy:
+  ///
+  ///                   A    <-- noSuchMethod
+  ///                  / \
+  ///                 C   B  <-- foo
+  ///
+  /// If we know we're calling foo on an object of type B we don't have to worry
+  /// about the noSuchMethod method in A because objects of type B implement
+  /// foo. On the other hand, if we end up calling foo on something of type C we
+  /// have to add a handler for it.
+  ///
+  /// If the holders of all user-defined noSuchMethod implementations that might
+  /// be applicable to the receiver type have a matching member for the current
+  /// name and selector, we avoid introducing a noSuchMethod handler.
+  ///
+  /// As an example, consider this class hierarchy:
+  ///
+  ///                        A    <-- foo
+  ///                       / \
+  ///    noSuchMethod -->  B   C  <-- bar
+  ///                      |   |
+  ///                      C   D  <-- noSuchMethod
+  ///
+  /// When calling foo on an object of type A, we know that the implementations
+  /// of noSuchMethod are in the classes B and D that also (indirectly)
+  /// implement foo, so we do not need a handler for it.
+  ///
+  /// If we're calling bar on an object of type D, we don't need the handler
+  /// either because all objects of type D implement bar through inheritance.
+  ///
+  /// If we're calling bar on an object of type A we do need the handler because
+  /// we may have to call B.noSuchMethod since B does not implement bar.
+  bool needsNoSuchMethod(ClassElement cls, Selector selector, ClassMask mask);
+
+  /// Returns whether [element] will be the one used at runtime when being
+  /// invoked on an instance of [cls]. [selector] is used to ensure library
+  /// privacy is taken into account.
+  bool hasElementIn(ClassElement cls, Selector selector, Element element);
+
   /// Returns [ClassHierarchyNode] for [cls] used to model the class hierarchies
   /// of known classes.
   ///
   /// This method is only provided for testing. For queries on classes, use the
   /// methods defined in [ClosedWorld].
   ClassHierarchyNode getClassHierarchyNode(ClassElement cls);
 
   /// Returns [ClassSet] for [cls] used to model the extends and implements
   /// relations of known classes.
   ///
   /// This method is only provided for testing. For queries on classes, use the
   /// methods defined in [ClosedWorld].
   ClassSet getClassSet(ClassElement cls);
 
-  // TODO(johnniwinther): Find a better strategy for caching these.
-  @deprecated
-  List<Map<ClassElement, TypeMask>> get canonicalizedTypeMasks;
+  /// Return the cached mask for [base] with the given flags, or
+  /// calls [createMask] to create the mask and cache it.
+  // TODO(johnniwinther): Find a better strategy for caching these?
+  TypeMask getCachedMask(ClassElement base, int flags, TypeMask createMask());
 
   /// Returns the [FunctionSet] containing all live functions in the closed
   /// world.
   FunctionSet get allFunctions;
 
   /// Returns `true` if the field [element] is known to be effectively final.
   bool fieldNeverChanges(Element element);
 
   /// Extends the receiver type [mask] for calling [selector] to take live
   /// `noSuchMethod` handlers into account.
@@ skipping 81 matching lines @@
 
   ClosedWorld closeWorld();
 
   /// Returns an iterable over all mixin applications that mixin [cls].
   Iterable<MixinApplicationElement> allMixinUsesOf(ClassElement cls);
 }
 
 class WorldImpl implements ClosedWorld, ClosedWorldRefiner, OpenWorld {
   bool _closed = false;
 
+  TypeMask getCachedMask(ClassElement base, int flags, TypeMask createMask()) {
+    Map<ClassElement, TypeMask> cachedMasks = _canonicalizedTypeMasks[flags];
+    if (cachedMasks == null) {
+      _canonicalizedTypeMasks[flags] = cachedMasks = <ClassElement, TypeMask>{};
+    }

[Siggi Cherem (dart-lang), 2016/10/14 16:49:57]

we can also replace the 3 lines above with:

  var cachedMasks = _canonicalizedTypeMasks[flags] ??= <ClassElement,
TypeMask>{};

or combine with the one below:

  return (_canonicalizedTypeMasks[flags] ??= <ClassElement, TypeMask>{})
      .putIfAbsent(base, createMask);

[Johnni Winther, 2016/10/17 07:57:40]

Done(-ish).

+    return cachedMasks.putIfAbsent(
+        base, () => new FlatTypeMask.internal(base, flags));

[Siggi Cherem (dart-lang), 2016/10/14 16:49:57]

use createMask here

[Johnni Winther, 2016/10/17 07:57:39]

Done.

+  }
+
   /// Cache of [FlatTypeMask]s grouped by the 8 possible values of the
   /// `FlatTypeMask.flags` property.
-  List<Map<ClassElement, TypeMask>> canonicalizedTypeMasks =
+  final List<Map<ClassElement, TypeMask>> _canonicalizedTypeMasks =
       new List<Map<ClassElement, TypeMask>>.filled(8, null);
 
   bool checkInvariants(ClassElement cls, {bool mustBeInstantiated: true}) {
     return invariant(cls, cls.isDeclaration,
                 message: '$cls must be the declaration.') &&
             invariant(cls, cls.isResolved,
                 message:
                     '$cls must be resolved.') /* &&
       // TODO(johnniwinther): Reinsert this or similar invariant.
       (!mustBeInstantiated ||
@@ skipping 45 matching lines @@
     return node != null && node.isDirectlyInstantiated;
   }
 
   @override
   bool isIndirectlyInstantiated(ClassElement cls) {
     assert(isClosed);
     ClassHierarchyNode node = _classHierarchyNodes[cls.declaration];
     return node != null && node.isIndirectlyInstantiated;
   }
 
+  @override
+  bool isAbstract(ClassElement cls) => cls.isAbstract;
+
   /// Returns `true` if [cls] is implemented by an instantiated class.
   bool isImplemented(ClassElement cls) {
     assert(isClosed);
     return _compiler.resolverWorld.isImplemented(cls);
   }
 
   /// Returns an iterable over the directly instantiated classes that extend
   /// [cls] possibly including [cls] itself, if it is live.
   Iterable<ClassElement> subclassesOf(ClassElement cls) {
     assert(isClosed);
@@ skipping 260 matching lines @@
   /// Returns `true` if any live class that mixes in [mixin] is also a subclass
   /// of [superclass].
   bool hasAnySubclassThatMixes(ClassElement superclass, ClassElement mixin) {
     assert(isClosed);
     return mixinUsesOf(mixin).any((each) => each.isSubclassOf(superclass));
   }
 
   /// Returns `true` if [cls] or any superclass mixes in [mixin].
   bool isSubclassOfMixinUseOf(ClassElement cls, ClassElement mixin) {
     assert(isClosed);
+    assert(cls.isDeclaration);
+    assert(mixin.isDeclaration);
     if (isUsedAsMixin(mixin)) {
-      ClassElement current = cls.declaration;
-      mixin = mixin.declaration;
+      ClassElement current = cls;
       while (current != null) {
-        current = current.declaration;
         if (current.isMixinApplication) {
           MixinApplicationElement application = current;
-          if (application.mixin.declaration == mixin) return true;
+          if (application.mixin == mixin) return true;
         }
         current = current.superclass;

[Siggi Cherem (dart-lang), 2016/10/14 16:49:57]

I assume we don't need to assert that superclass is a declaration, correct?

[Johnni Winther, 2016/10/17 07:57:40]

Correct.

       }
     }
     return false;
   }
 
   /// Returns `true` if every subtype of [x] is a subclass of [y] or a subclass
   /// of a mixin application of [y].
   bool everySubtypeIsSubclassOfOrMixinUseOf(ClassElement x, ClassElement y) {
     assert(isClosed);
-    x = x.declaration;
-    y = y.declaration;
+    assert(x.isDeclaration);
+    assert(y.isDeclaration);
     Map<ClassElement, bool> secondMap =
         _subtypeCoveredByCache[x] ??= <ClassElement, bool>{};
     return secondMap[y] ??= subtypesOf(x).every((ClassElement cls) =>
         isSubclassOf(cls, y) || isSubclassOfMixinUseOf(cls, y));
   }
 
   /// Returns `true` if any subclass of [superclass] implements [type].
   bool hasAnySubclassThatImplements(
       ClassElement superclass, ClassElement type) {
     assert(isClosed);
     Set<ClassElement> subclasses = typesImplementedBySubclassesOf(superclass);
     if (subclasses == null) return false;
     return subclasses.contains(type);
   }
 
+  @override
+  bool hasElementIn(ClassElement cls, Selector selector, Element element) {
+    // Use [:implementation:] of [element]
+    // because our function set only stores declarations.
+    Element result = findMatchIn(cls, selector);
+    return result == null
+        ? false
+        : result.implementation == element.implementation;
+  }
+
+  Element findMatchIn(ClassElement cls, Selector selector) {
+    // Use the [:implementation] of [cls] in case the found [element]
+    // is in the patch class.
+    var result = cls.implementation.lookupByName(selector.memberName);
+    return result;
+  }
+
+  /// Returns whether a [selector] call on an instance of [cls]
+  /// will hit a method at runtime, and not go through [noSuchMethod].
+  bool hasConcreteMatch(ClassElement cls, Selector selector) {
+    assert(invariant(cls, isInstantiated(cls),
+        message: '$cls has not been instantiated.'));
+    Element element = findMatchIn(cls, selector);
+    if (element == null) return false;
+
+    if (element.isAbstract) {
+      ClassElement enclosingClass = element.enclosingClass;
+      return hasConcreteMatch(enclosingClass.superclass, selector);
+    }
+    return selector.appliesUntyped(element);
+  }
+
+  @override
+  bool needsNoSuchMethod(ClassElement base, Selector selector, ClassMask mask) {
+    /// Returns `true` if [cls] is an instantiated class that does not have
+    /// a concrete method matching [selector].
+    bool needsNoSuchMethod(ClassElement cls) {
+      // We can skip uninstantiated subclasses.
+      if (!isInstantiated(cls)) {
+        return false;
+      }
+      // We can just skip abstract classes because we know no
+      // instance of them will be created at runtime, and
+      // therefore there is no instance that will require
+      // [noSuchMethod] handling.
+      return !cls.isAbstract && !hasConcreteMatch(cls, selector);
+    }
+
+    bool baseNeedsNoSuchMethod = needsNoSuchMethod(base);
+    if (mask == ClassMask.EXACT || baseNeedsNoSuchMethod) {
+      return baseNeedsNoSuchMethod;
+    }
+
+    Iterable<ClassElement> subclassesToCheck;
+    if (mask == ClassMask.SUBTYPE) {
+      subclassesToCheck = strictSubtypesOf(base);
+    } else {
+      assert(mask == ClassMask.SUBCLASS);
+      subclassesToCheck = strictSubclassesOf(base);
+    }
+
+    return subclassesToCheck != null &&
+        subclassesToCheck.any(needsNoSuchMethod);
+  }
+
   final Compiler _compiler;
   BackendClasses get backendClasses => _backend.backendClasses;
   JavaScriptBackend get _backend => _compiler.backend;
   CommonMasks get commonMasks => _compiler.commonMasks;
   final FunctionSet allFunctions;
   final Set<Element> functionsCalledInLoop = new Set<Element>();
   final Map<Element, SideEffects> sideEffects = new Map<Element, SideEffects>();
 
   final Set<TypedefElement> _allTypedefs = new Set<TypedefElement>();
 
@@ skipping 333 matching lines @@
       return getMightBePassedToApply(element.expression);
     }
     return functionsThatMightBePassedToApply.contains(element);
   }
 
   @override
   bool getCurrentlyKnownMightBePassedToApply(Element element) {
     return getMightBePassedToApply(element);
   }
 }
+
+/// Enum values defining subset of classes included in queries.
+enum ClassMask {

[Siggi Cherem (dart-lang), 2016/10/14 16:49:57]

I'm not convinced about adding this enum. Since it is only used in one query,
what if we remove the enum and use a couple boolean named arguments there? For
example:

  bool needsNoSuchMethod(ClassElement cls, Selector selector,
     {includeSubclasses: false, includeSubtypes: false});

If we want to have the enum, I'd suggest to rename it to `ClassQuery` or
something else (the word Mask seems to be there only because it is coupled
somehow with TypeMask)

[Johnni Winther, 2016/10/17 07:57:40]

Renamed to `ClassQuery`. We'll need it more function moved from xTypeMask to
ClosedWorld.

+  /// Only the class itself is included.
+  EXACT,
+
+  /// The class and all subclasses (transitively) are included.
+  SUBCLASS,
+
+  /// The class and all classes that implement or subclass it (transitively)
+  /// are included.
+  SUBTYPE,
+}