Refactor program traversing logic.

lib/compiler/implementation/dart_backend/backend.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.
 
 class DartBackend extends Backend {
   final List<CompilerTask> tasks;
   final UnparseValidator unparseValidator;
 
   Map<Element, TreeElements> get resolvedElements() =>
       compiler.enqueuer.resolution.resolvedElements;
@@ skipping 27 matching lines @@
     final LIBS_TO_IGNORE = [
       compiler.jsHelperLibrary,
       compiler.interceptorsLibrary,
     ];
     bool shouldOutput(Element element) =>
       element.kind !== ElementKind.VOID &&
       LIBS_TO_IGNORE.indexOf(element.getLibrary()) == -1 &&
       !isDartCoreLib(compiler, element.getLibrary()) &&
       element is !AbstractFieldElement;
 
-    // TODO(smok): Refactor this traverse/collect mess.
-    Set<TypedefElement> typedefs = new Set<TypedefElement>();
-    Set<ClassElement> classes = new Set<ClassElement>();
-    Set<Element> elements = new Set<Element>();
-    Map<ClassElement, Set<Element>> resolvedClassMembers =
+    final emptyTreeElements = new TreeElementMapping();

[Roman, 2012/08/16 13:53:26]

maybe remove this? it is used only twice, it would be better for readability to
explicitly use new TreeElementMapping, and you will actually save 1 loc

[Anton Muhin, 2012/08/16 14:06:42]

I don't want to create a lot of garbage.

On 2012/08/16 13:53:26, Roman wrote:

[Roman, 2012/08/16 14:23:54]

Why not?

[Anton Muhin, 2012/08/16 14:30:44]

Why stress GC unnecessarily?

On 2012/08/16 14:23:54, Roman wrote:

+
+    Set<Element> topLevelElements = new Set<Element>();
+    Map<ClassElement, Set<Element>> classes =
         new Map<ClassElement, Set<Element>>();
+
     PlaceholderCollector collector = new PlaceholderCollector(compiler);
+    var newTypedefElementCallback, newClassElementCallback;
+    collectReferencedElements(element, treeElements) {

[Roman, 2012/08/16 13:53:26]

Inline this method? It is used only once. And if you do that, you can define
callbacks inside processElement() without these variable hacks when you define
callbacks later, it is difficult to read this way.

[Anton Muhin, 2012/08/16 14:06:42]

Alas, variables hacks are a must as Dart requires name to be defined before
usage, but inlining is very good idea, done.

On 2012/08/16 13:53:26, Roman wrote:

+      new ReferencedElementCollector(
+          compiler,
+          element, treeElements,
+          newTypedefElementCallback, newClassElementCallback).collect();
+    }
+    processElement(element, treeElements) {
+      collector.collect(element, treeElements);
+      collectReferencedElements(element, treeElements);
+    }
+
+    addTopLevel(element, treeElements) {
+      if (topLevelElements.contains(element)) return;
+      topLevelElements.add(element);
+      processElement(element, treeElements);
+    }
+    addClass(classElement) {
+      if (classes.containsKey(classElement)) return;
+      classes[classElement] = new Set<Element>();
+      processElement(classElement, emptyTreeElements);
+    }
+
+    newTypedefElementCallback = (TypedefElement element) {
+      if (!shouldOutput(element)) return;
+      addTopLevel(element, emptyTreeElements);
+    };
+    newClassElementCallback = (ClassElement classElement) {
+      if (!shouldOutput(classElement)) return;
+      addClass(classElement);
+    };
+
     resolvedElements.forEach((element, treeElements) {
       if (!shouldOutput(element)) return;
+
       if (element.isMember()) {
         ClassElement enclosingClass = element.getEnclosingClass();
         assert(enclosingClass.isClass());
         assert(enclosingClass.isTopLevel());
-        resolvedClassMembers
-            .putIfAbsent(enclosingClass, () => new Set<Element>())
-            .add(element);
-        return;
+        assert(shouldOutput(enclosingClass));
+        addClass(enclosingClass);
+        classes[enclosingClass].add(element);
+        processElement(element, treeElements);
+      } else {
+        if (!element.isTopLevel()) {
+          compiler.cancel(reason: 'Cannot process $element', element: element);
+        }
+        addTopLevel(element, treeElements);
       }
-      if (!element.isTopLevel()) {
-        compiler.cancel(reason: 'Cannot process $element', element: element);
-      }
-
-      elements.add(element);
     });
-    resolvedElements.forEach((element, treeElements) {
-      if (!shouldOutput(element)) return;
-      collector.collect(element, treeElements);
-      new ReferencedElementCollector(
-          compiler, element, treeElements, typedefs, classes)
-      .collect();
-    });
-
-    final emptyTreeElements = new TreeElementMapping();
-    collectElement(element) { collector.collect(element, emptyTreeElements); }
-    typedefs.forEach(collectElement);
-    classes.forEach(collectElement);
-    resolvedClassMembers.getKeys().forEach(collectElement);
 
     Map<Node, String> renames = new Map<Node, String>();
     Map<LibraryElement, String> imports = new Map<LibraryElement, String>();
     renamePlaceholders(compiler, collector, renames, imports);
 
     Emitter emitter = new Emitter(compiler, renames);
     emitter.outputImports(imports);
-    elements.forEach(emitter.outputElement);
-    typedefs.forEach(emitter.outputElement);
-    final emptySet = new Set<Element>();
-    classes.forEach((classElement) {
-      if (!shouldOutput(classElement)) return;
-      if (resolvedClassMembers.containsKey(classElement)) return;
-      emitter.outputClass(classElement, emptySet);
-    });
+    topLevelElements.forEach(emitter.outputElement);
+    classes.forEach(emitter.outputClass);
 
-    // Now output resolved classes with inner elements we met before.
-    resolvedClassMembers.forEach(emitter.outputClass);
     compiler.assembledCode = emitter.toString();
   }
 
   log(String message) => compiler.log('[DartBackend] $message');
 }
 
 /**
  * Checks if [:libraryElement:] is a core lib, that is a library
  * provided by the implementation like dart:core, dart:coreimpl, etc.
  */
@@ skipping 10 matching lines @@
 /**
  * Some elements are not recorded by resolver now,
  * for example, typedefs or classes which are only
  * used in signatures, as/is operators or in super clauses
  * (just to name a few).  Retraverse AST to pick those up.
  */
 class ReferencedElementCollector extends AbstractVisitor {
   final Compiler compiler;
   final Element rootElement;
   final TreeElements treeElements;
-  final Set<TypedefElement> typedefs;
-  final Set<ClassElement> classes;
+  final newTypedefElementCallback;
+  final newClassElementCallback;
 
   ReferencedElementCollector(
       this.compiler,
       this.rootElement, this.treeElements,
-      this.typedefs, this.classes);
-
-  void collectElement(Element element) {
-    new ReferencedElementCollector(
-        compiler, element, new TreeElementMapping(), typedefs, classes)
-    .collect();
-  }
+      this.newTypedefElementCallback, this.newClassElementCallback);
 
   visitClassNode(ClassNode node) {
     super.visitClassNode(node);
     // Temporary hack which should go away once interfaces
     // and default clauses are out.
     if (node.defaultClause !== null) {
       // Resolver cannot resolve parameterized default clauses.
       TypeAnnotation evilCousine = new TypeAnnotation(
           node.defaultClause.typeName, null);
       evilCousine.accept(this);
     }
   }
 
   visitNode(Node node) { node.visitChildren(this); }
 
   visitTypeAnnotation(TypeAnnotation typeAnnotation) {
     final type = compiler.resolveTypeAnnotation(rootElement, typeAnnotation);
     Element typeElement = type.element;
-    if (typeElement.isTypedef() && !typedefs.contains(typeElement)) {
-      typedefs.add(typeElement);
-      collectElement(typeElement);
-    }
-    if (typeElement.isClass() && !classes.contains(typeElement)) {
-      classes.add(typeElement);
-      collectElement(typeElement);
-    }
+    if (typeElement.isTypedef()) newTypedefElementCallback(typeElement);
+    if (typeElement.isClass()) newClassElementCallback(typeElement);
     typeAnnotation.visitChildren(this);
   }
 
   void collect() {
     compiler.withCurrentElement(rootElement, () {
       rootElement.parseNode(compiler).accept(this);
     });
   }
 }