Take inheritance into account when computing the elements accessible by mirrors.

sdk/lib/_internal/compiler/implementation/enqueue.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.
 
 part of dart2js;
 
 typedef ItemCompilationContext ItemCompilationContextCreator();
 
 class EnqueueTask extends CompilerTask {
   final ResolutionEnqueuer resolution;
@@ skipping 55 matching lines @@
 
 abstract class Enqueuer {
   final String name;
   final Compiler compiler; // TODO(ahe): Remove this dependency.
   final ItemCompilationContextCreator itemCompilationContextCreator;
   final Map<String, Link<Element>> instanceMembersByName
       = new Map<String, Link<Element>>();
   final Map<String, Link<Element>> instanceFunctionsByName
       = new Map<String, Link<Element>>();
   final Set<ClassElement> seenClasses = new Set<ClassElement>();
+  Set<ClassElement> recentClasses = new Setlet<ClassElement>();
   final Universe universe = new Universe();
 
+  static final TRACE_MIRROR_ENQUEUING = false;

[floitsch, 2014/06/27 08:59:15]

should we start making these things const X.fromEnvironment?

for example:
 ... = const bool.fromEnvironment("TRACE_MIRROR_ENQUEUING");

[herhut, 2014/06/27 12:34:35]

I did not know it was that easy to read an environment variable.

Done.

+
   bool queueIsClosed = false;
   EnqueueTask task;
   native.NativeEnqueuer nativeEnqueuer;  // Set by EnqueueTask
 
-  bool hasEnqueuedEverything = false;
+  bool hasEnqueuedReflectiveElements = false;
   bool hasEnqueuedReflectiveStaticFields = false;
 
   Enqueuer(this.name, this.compiler, this.itemCompilationContextCreator);
 
   Queue<WorkItem> get queue;
   bool get queueIsEmpty => queue.isEmpty;
 
   /// Returns [:true:] if this enqueuer is the resolution enqueuer.
   bool get isResolutionQueue => false;
 
   /// Returns [:true:] if [member] has been processed by this enqueuer.
   bool isProcessed(Element member);
 
   /**
    * Documentation wanted -- johnniwinther
    *
    * Invariant: [element] must be a declaration element.
    */
   void addToWorkList(Element element) {
     assert(invariant(element, element.isDeclaration));
     internalAddToWorkList(element);
   }
 
   /**
    * Adds [element] to the work list if it has not already been processed.
    */
   void internalAddToWorkList(Element element);
 
-  void registerInstantiatedType(InterfaceType type, Registry registry) {
+  void registerInstantiatedType(InterfaceType type, Registry registry,
+                                {mirrorUsage: false}) {

[karlklose, 2014/06/27 07:45:57]

Only use registerInstantiatedType for types that are instantiated (via new) in
the source code.

If we really need to track the classes used from the mirrors, we should have a
separate set and include the classes in this set in
CodeEmitterTask.computeNeededClasses.

[herhut, 2014/06/27 12:34:35]

This only replicated what the original implementation did. I will leave it like
that in this CL with a TODO and refactor in a follow up.

     task.measure(() {
       ClassElement cls = type.element;
       registry.registerDependency(cls);
       cls.ensureResolved(compiler);
       universe.instantiatedTypes.add(type);
       if (!cls.isAbstract
           // We can't use the closed-world assumption with native abstract
           // classes; a native abstract class may have non-abstract subclasses
           // not declared to the program.  Instances of these classes are
           // indistinguishable from the abstract class.
-          || cls.isNative) {
+          || cls.isNative
+          // Likewise, if this registration comes from the mirror system,
+          // all bets are off.
+          || mirrorUsage) {
         universe.instantiatedClasses.add(cls);
       }
       onRegisterInstantiatedClass(cls);
     });
   }
 
-  void registerInstantiatedClass(ClassElement cls, Registry registry) {
+  void registerInstantiatedClass(ClassElement cls, Registry registry,
+                                 {mirrorUsage: false}) {
     cls.ensureResolved(compiler);
-    registerInstantiatedType(cls.rawType, registry);
+    registerInstantiatedType(cls.rawType, registry, mirrorUsage: mirrorUsage);
   }
 
   bool checkNoEnqueuedInvokedInstanceMethods() {
     task.measure(() {
       // Run through the classes and see if we need to compile methods.
       for (ClassElement classElement in universe.instantiatedClasses) {
         for (ClassElement currentClass = classElement;
              currentClass != null;
              currentClass = currentClass.superclass) {
           processInstantiatedClass(currentClass);
@@ skipping 111 matching lines @@
     task.measure(() {
       if (seenClasses.contains(cls)) return;
       // The class must be resolved to compute the set of all
       // supertypes.
       cls.ensureResolved(compiler);
 
       void processClass(ClassElement cls) {
         if (seenClasses.contains(cls)) return;
 
         seenClasses.add(cls);
+        recentClasses.add(cls);
         cls.ensureResolved(compiler);
         cls.implementation.forEachMember(processInstantiatedClassMember);
         if (isResolutionQueue) {
           compiler.resolver.checkClass(cls);
         }
         // We only tell the backend once that [cls] was instantiated, so
         // any additional dependencies must be treated as global
         // dependencies.
         compiler.backend.registerInstantiatedClass(
             cls, this, compiler.globalDependencies);
@@ skipping 28 matching lines @@
       registerNewSelector(selector, universe.invokedGetters);
     });
   }
 
   void registerInvokedSetter(Selector selector) {
     task.measure(() {
       registerNewSelector(selector, universe.invokedSetters);
     });
   }
 
-  void pretendElementWasUsed(Element element, Registry registry) {
-    if (!compiler.backend.isNeededForReflection(element)) return;
-    if (Elements.isUnresolved(element)) {
-      // Ignore.
-    } else if (element.isSynthesized
-               && element.library.isPlatformLibrary) {
-      // TODO(ahe): Work-around for http://dartbug.com/11205.
-    } else if (element.isConstructor) {
-      ClassElement cls = element.declaration.enclosingClass;
-      registerInstantiatedType(cls.rawType, registry);
-      registerStaticUse(element.declaration);
-    } else if (element.isClass) {
-      ClassElement cls = element.declaration;
-      registerInstantiatedClass(cls, registry);
-      // Make sure that even abstract classes are considered instantiated.
-      universe.instantiatedClasses.add(cls);
-    } else if (element.impliesType) {
-      // Don't enqueue typedefs, and type variables.
-    } else if (Elements.isStaticOrTopLevel(element)) {
-      registerStaticUse(element.declaration);
-    } else if (element.isInstanceMember) {
-      Selector selector = new Selector.fromElement(element, compiler);
-      registerSelectorUse(selector);
-      if (element.isField) {
-        Selector selector =
-            new Selector.setter(element.name, element.library);
-        registerInvokedSetter(selector);
+  /**
+   * Decides whether an element should be included to satisfy requirements
+   * of the mirror system.
+   *
+   * The actual implementation depends on the current compiler phase.
+   */
+  bool includeElementDueToMirrors(bool includeEnclosing, Element element);

[Johnni Winther, 2014/06/27 07:43:01]

Rename to [shouldIncludeElementDueToMirror] to indicate that this is a question
and not a command.

[Johnni Winther, 2014/06/27 07:43:01]

Make [includeEnclosing] a named parameter.

[herhut, 2014/06/27 12:34:36]

Done.

[herhut, 2014/06/27 12:34:36]

Done.

+
+  void logEnqueueReflectiveAction(action, [msg = ""]) {
+    if (TRACE_MIRROR_ENQUEUING) {
+      print("MIRROR_ENQUEUE (${isResolutionQueue ? "R" : "B"}): $action $msg");

[karlklose, 2014/06/27 07:45:57]

'R' -> 'resolution'?

What is 'B'?

[herhut, 2014/06/27 12:34:36]

Backend. Probably C for codegen is better. I have changed it.

+    }
+  }
+
+  void enqueueReflectiveConstructor(ConstructorElement ctor, bool add) {

[Johnni Winther, 2014/06/27 07:43:00]

Add comment. I guess it is a helper method for [enqueueReflectiveElements].

[Johnni Winther, 2014/06/27 07:43:01]

Make [add] named.

[karlklose, 2014/06/27 07:45:57]

Could you add a comment about the semantics of 'add'?

[floitsch, 2014/06/27 08:59:15]

nit: s/add/forceAdd/

I'm guessing that's the meaning.

Since it's not completely clear, maybe document what "add" does.

[herhut, 2014/06/27 12:34:36]

Done.

[herhut, 2014/06/27 12:34:36]

Done.

[herhut, 2014/06/27 12:34:36]

Done.

[herhut, 2014/06/27 12:34:37]

Done.

+    if (includeElementDueToMirrors(add, ctor)) {
+      logEnqueueReflectiveAction(ctor);
+      ClassElement cls = ctor.declaration.enclosingClass;
+      registerInstantiatedType(cls.rawType, compiler.mirrorDependencies);

[Johnni Winther, 2014/06/27 07:43:01]

Shouldn't this call have `mirrorUsage: true` ?

[karlklose, 2014/06/27 07:45:57]

I am not sure we should register this type here.

[herhut, 2014/06/27 12:34:35]

It will not have an effect currently but it is more appropriate to set it to
true.

Done.

[herhut, 2014/06/27 12:34:36]

This implements what was done before. I will redo the tracking in a follow up
CL.

+      registerStaticUse(ctor.declaration);
+    }
+  }
+
+  void enqueueReflectiveMember(Element element, bool add) {

[Johnni Winther, 2014/06/27 07:43:00]

Make [add] named.

[Johnni Winther, 2014/06/27 07:43:01]

Add comment.

[herhut, 2014/06/27 12:34:35]

Done.

[herhut, 2014/06/27 12:34:36]

Done.

+    add = includeElementDueToMirrors(add, element);

[karlklose, 2014/06/27 07:45:57]

Inline call to includeElementsDueToMirrors? (Also below)

[herhut, 2014/06/27 12:34:36]

Below I have to pass the result on to the processing of members. 

Done.

+    if (add && !element.impliesType) {

[floitsch, 2014/06/27 08:59:15]

Keep comment exlpaining what "impliesType" means.
(The old code had the comment "Don't enqueue typedefs, and type variables")

Btw: is this true? When I iterate over a library I would like to see the
typedef.

[herhut, 2014/06/27 12:34:36]

Done.

+      logEnqueueReflectiveAction(element);
+      if (Elements.isStaticOrTopLevel(element)) {
+        registerStaticUse(element.declaration);
+      } else if (element.isInstanceMember) {
+        // We need to enqueue all members matching this one in subclasses, as
+        // well.
+        // TODO(herhut): Use TypedSelector.subtype for enqueueing
+        Selector selector = new Selector.fromElement(element, compiler);
+        registerSelectorUse(selector);
+        if (element.isField) {
+          Selector selector =
+              new Selector.setter(element.name, element.library);
+          registerInvokedSetter(selector);
+        }
       }
     }
   }
 
-  void enqueueEverything() {
-    if (hasEnqueuedEverything) return;
-    compiler.log('Enqueuing everything');
-    task.ensureAllElementsByName();
-    for (Link link in task.allElementsByName.values) {
-      for (Element element in link) {
-        pretendElementWasUsed(element, compiler.mirrorDependencies);
+  void enqueueReflectiveElementsInClass(ClassElement cls,

[Johnni Winther, 2014/06/27 07:43:00]

Add comment.

[herhut, 2014/06/27 12:34:36]

Done.

+                                        Iterable<ClassElement> recents,
+                                        bool add) {

[Johnni Winther, 2014/06/27 07:43:00]

Make [add] named.

[herhut, 2014/06/27 12:34:37]

Done.

+    if (cls.library.isInternalLibrary) return;
+    if (cls.isInjected) return;

[Johnni Winther, 2014/06/27 07:43:00]

Merge conditions.

[herhut, 2014/06/27 12:34:36]

Done.

+    add = includeElementDueToMirrors(add, cls);
+    if (add) {
+      logEnqueueReflectiveAction(cls, "register");
+      ClassElement decl = cls.declaration;
+      registerInstantiatedClass(decl, compiler.mirrorDependencies,
+          mirrorUsage: true);
+    }
+    // If the class is never instantiated, we know nothing of it can possibly
+    // be reflected upon.
+    // TODO(herhut): Add a warning if a mirrors annotation cannot hit.
+    if (recents.contains(cls.declaration)) {
+      logEnqueueReflectiveAction(cls, "members");
+      cls.constructors.forEach((Element element) {
+        enqueueReflectiveConstructor(element, add);
+      });
+      cls.forEachClassMember((Member member) {
+        enqueueReflectiveMember(member.element, add);
+      });
+    }
+  }
+
+  void enqueueReflectiveSpecialClasses() {

[Johnni Winther, 2014/06/27 07:43:00]

Add comment.

[herhut, 2014/06/27 12:34:36]

Done.

+    // [Closure] is treated specially as it is the superclass of all closures.
+    // Although it is in an internal library, we mark it as reflectable. Note
+    // that none of its methods are reflectable, unless reflectable by
+    // inheritance.
+    ClassElement closureClass = compiler.closureClass;
+    if (compiler.backend.referencedFromMirrorSystem(closureClass)) {
+      logEnqueueReflectiveAction(closureClass);
+      registerInstantiatedClass(closureClass, compiler.mirrorDependencies,
+          mirrorUsage: true);
+    }
+  }
+
+  void enqueueReflectiveElementsInLibrary(LibraryElement lib,

[Johnni Winther, 2014/06/27 07:43:00]

Add comment.

[herhut, 2014/06/27 12:34:36]

Done.

+                                          Iterable<ClassElement> recents,
+                                          bool add) {
+    add = includeElementDueToMirrors(add, lib);
+    lib.forEachLocalMember((Element member) {
+      if (member.isClass) {
+        enqueueReflectiveElementsInClass(member, recents, add);
+      } else {
+        enqueueReflectiveMember(member, add);
       }
+    });
+  }
+
+  /// Enqueue all elements that are matched by the mirrors used
+  /// annotation or, in lack thereof, all elements.
+  void enqueueReflectiveElements(Iterable<ClassElement> recents) {
+    if (!hasEnqueuedReflectiveElements) {
+      logEnqueueReflectiveAction("!START enqueueAll");
+      // First round of enqueuing, visit everything that is visible to
+      // also pick up static top levels, etc.
+      // Also, during the first round, consider all classes that have been seen
+      // as recently seen, as a couple of rounds of resolution might have run

[floitsch, 2014/06/27 08:59:15]

maybe?

... as treeshaking might only be disabled at a later point (forcing us to
enqueue everything), when several rounds of resolution had happened.

[herhut, 2014/06/27 12:34:35]

Rephrased:

Also, during the first round, consider all classes that have been seen as
recently seen, as we do not know how many rounds of resolution might have run
before tree shaking is disabled and thus everything is enqueued. 

+      // before treeshaking is disabled and we thus enqueue everything.
+      recents = seenClasses.toSet();
+      compiler.log('Enqueuing everything');
+      for (LibraryElement lib in compiler.libraries.values) {
+        enqueueReflectiveElementsInLibrary(lib, recents, false);
+      }
+      enqueueReflectiveSpecialClasses();
+      hasEnqueuedReflectiveElements = true;
+      hasEnqueuedReflectiveStaticFields = true;
+      logEnqueueReflectiveAction("!DONE enqueueAll");
+    } else if (recents.isNotEmpty) {
+      // Keep looking at new classes until fixpoint is reached.
+      logEnqueueReflectiveAction("!START enqueueRecents");
+      recents.forEach((ClassElement cls) {
+        enqueueReflectiveElementsInClass(cls, recents,
+            includeElementDueToMirrors(false, cls.library));
+      });
+      logEnqueueReflectiveAction("!DONE enqueueRecents");
     }
-    hasEnqueuedEverything = true;
-    hasEnqueuedReflectiveStaticFields = true;
   }
 
   /// Enqueue the static fields that have been marked as used by reflective
   /// usage through `MirrorsUsed`.
   void enqueueReflectiveStaticFields(Iterable<Element> elements) {
     if (hasEnqueuedReflectiveStaticFields) return;
     hasEnqueuedReflectiveStaticFields = true;
     for (Element element in elements) {
-      pretendElementWasUsed(element, compiler.mirrorDependencies);
+      enqueueReflectiveMember(element, true);
     }
   }
 
   processLink(Map<String, Link<Element>> map,
               String memberName,
               bool f(Element e)) {
     Link<Element> members = map[memberName];
     if (members != null) {
       // [f] might add elements to [: map[memberName] :] during the loop below
       // so we create a new list for [: map[memberName] :] and prepend the
@@ skipping 149 matching lines @@
   }
 
 
   void registerIfGeneric(Element element, Registry registry) {
     if (element.computeType(compiler).containsTypeVariables) {
       compiler.backend.registerGenericClosure(element, this, registry);
       universe.genericClosures.add(element);
     }
   }
 
   void registerClosure(Element element, Registry registry) {

[Johnni Winther, 2014/06/27 07:43:00]

Change [Element] to [FunctionElement].

[herhut, 2014/06/27 12:34:36]

Done.

+    universe.allClosures.add(element);
     registerIfGeneric(element, registry);
   }
 
   void forEach(f(WorkItem work)) {
     do {
-      while (!queue.isEmpty) {
+      while (queue.isNotEmpty) {
         // TODO(johnniwinther): Find an optimal process order.
         f(queue.removeLast());
       }
-      onQueueEmpty();
-    } while (!queue.isEmpty);
+      List recents = recentClasses.toList(growable: false);
+      recentClasses.clear();
+      if (!onQueueEmpty(recents)) recentClasses.addAll(recents);
+    } while (queue.isNotEmpty || recentClasses.isNotEmpty);
   }
 
-  void onQueueEmpty() {
-    compiler.backend.onQueueEmpty(this);
+  /// [onQueueEmpty] is called whenever the queue is drained. [recentClasses]
+  /// contains the set of all classes seen for the first time since
+  /// [onQueueEmpty] was called last. A return value of [true] indicates that
+  /// the [recentClasses] have been processed and may be cleared. If [false] is
+  /// returned, [onQueueEmpty] will be called once the queue is empty again (or
+  /// still empty) and [recentClasses] will be a superset of the current value.
+  bool onQueueEmpty(Iterable<ClassElement> recentClasses) {
+    return compiler.backend.onQueueEmpty(this, recentClasses);
   }
 
   void logSummary(log(message)) {
     _logSpecificSummary(log);
     nativeEnqueuer.logSummary(log);
   }
 
   /// Log summary specific to the concrete enqueuer.
   void _logSpecificSummary(log(message));
 
@@ skipping 39 matching lines @@
     resolvedElements.add(element);
   }
 
   /// Returns [:true:] if [element] has actually been used.
   bool isLive(Element element) {
     if (seenClasses.contains(element)) return true;
     if (hasBeenResolved(element)) return true;
     return false;
   }
 
+  /**
+   * Decides whether an element should be included to satisfy requirements
+   * of the mirror system.
+   *
+   * During resoltion, we have to resort to matching elements against the

[Johnni Winther, 2014/06/27 07:43:01]

'resoltion' -> 'resolution'.

[karlklose, 2014/06/27 07:45:57]

'resoltion' -> 'resolution'.

[herhut, 2014/06/27 12:34:36]

Done.

[herhut, 2014/06/27 12:34:37]

Done.

+   * [MirrorsUsed] pattern, as we do not have a complete picture of the world,
+   * yet.
+   */
+  bool includeElementDueToMirrors(bool includeEnclosing, Element element) {

[karlklose, 2014/06/27 07:45:57]

Does it make sense to move functionality like this to the MirrorUsageAnalyzer or
create a class to handle these?

[herhut, 2014/06/27 12:34:36]

Yes, this is also on the list of further refactoring.

+    return includeEnclosing || compiler.backend.requiredByMirrorSystem(element);
+  }
+
   void internalAddToWorkList(Element element) {
     assert(invariant(element, element is AnalyzableElement,
         message: 'Element $element is not analyzable.'));
     if (hasBeenResolved(element)) return;
     if (queueIsClosed) {
       throw new SpannableAssertionFailure(element,
           "Resolution work list is closed. Trying to add $element.");
     }
 
     compiler.world.registerUsedElement(element);
@@ skipping 66 matching lines @@
    */
   void addDeferredAction(Element element, DeferredAction action) {
     if (queueIsClosed) {
       throw new SpannableAssertionFailure(element,
           "Resolution work list is closed. "
           "Trying to add deferred action for $element");
     }
     deferredTaskQueue.add(new DeferredTask(element, action));
   }
 
-  void onQueueEmpty() {
+  bool onQueueEmpty(Iterable<ClassElement> recentClasses) {
     emptyDeferredTaskQueue();
-    super.onQueueEmpty();
+    return super.onQueueEmpty(recentClasses);
   }
 
   void emptyDeferredTaskQueue() {
     while (!deferredTaskQueue.isEmpty) {
       DeferredTask task = deferredTaskQueue.removeFirst();
       compiler.withCurrentElement(task.element, task.action);
     }
   }
 
   void registerJsCall(Send node, ResolverVisitor resolver) {
@@ skipping 15 matching lines @@
 
   CodegenEnqueuer(Compiler compiler,
                   ItemCompilationContext itemCompilationContextCreator())
       : queue = new Queue<CodegenWorkItem>(),
         newlyEnqueuedElements = compiler.cacheStrategy.newSet(),
         super('codegen enqueuer', compiler, itemCompilationContextCreator);
 
   bool isProcessed(Element member) =>
       member.isAbstract || generatedCode.containsKey(member);
 
+  /**
+   * Decides whether an element should be included to satisfy requirements
+   * of the mirror system.
+   *
+   * For code generation, we rely on the precomputed set of elements that takes
+   * subtyping constraints into account.
+   */
+  bool includeElementDueToMirrors(bool includeEnclosing, Element element) {
+    return compiler.backend.isAccessibleByReflection(element);
+  }
+
   void internalAddToWorkList(Element element) {
     if (compiler.hasIncrementalSupport) {
       newlyEnqueuedElements.add(element);
     }
     // Don't generate code for foreign elements.
     if (element.isForeign(compiler)) return;
 
     // Codegen inlines field initializers. It only needs to generate
     // code for checked setters.
     if (element.isField && element.isInstanceMember) {
       if (!compiler.enableTypeAssertions
           || element.enclosingElement.isClosure) {
         return;
       }
     }
 
     if (queueIsClosed) {
       throw new SpannableAssertionFailure(element,
           "Codegen work list is closed. Trying to add $element");
     }
     CodegenWorkItem workItem = new CodegenWorkItem(
         element, itemCompilationContextCreator());
     queue.add(workItem);
   }
 
   void _logSpecificSummary(log(message)) {
     log('Compiled ${generatedCode.length} methods.');
   }
 }