Refactor program traversing logic.

lib/compiler/implementation/dart_backend/backend.dart

Index: lib/compiler/implementation/dart_backend/backend.dart
diff --git a/lib/compiler/implementation/dart_backend/backend.dart b/lib/compiler/implementation/dart_backend/backend.dart
index 5fb537de5128adecfaea71b4f1eb2d6a6b051b98..63af490d54a9ffa99f2fdec332a36b2993e4fc74 100644
--- a/lib/compiler/implementation/dart_backend/backend.dart
+++ b/lib/compiler/implementation/dart_backend/backend.dart
@@ -45,61 +45,73 @@ class DartBackend extends Backend {
       !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();
   }
 
@@ -130,19 +142,13 @@ 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);
@@ -161,14 +167,8 @@ class ReferencedElementCollector extends AbstractVisitor {
   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);
   }