Rename elements after sorting.

lib/compiler/implementation/dart_backend/renamer.dart

Index: lib/compiler/implementation/dart_backend/renamer.dart
diff --git a/lib/compiler/implementation/dart_backend/renamer.dart b/lib/compiler/implementation/dart_backend/renamer.dart
index 6cb47f76933ce788de7d7a7ba4b5556e60b75b84..eb4f24dac54ac3909d5dd7e41fca9358a7f8911e 100644
--- a/lib/compiler/implementation/dart_backend/renamer.dart
+++ b/lib/compiler/implementation/dart_backend/renamer.dart
@@ -50,35 +50,38 @@ void renamePlaceholders(
                    () => generateUniqueName(originalName));
   }
 
-  placeholderCollector.nullNodes.forEach((Node node) {
+  for (Node node in placeholderCollector.nullNodes) {
     renames[node] = '';
-  });
-  placeholderCollector.unresolvedNodes.forEach((Node node) {
+  }
+  for (Node node in placeholderCollector.unresolvedNodes) {

[Roman, 2012/08/21 07:59:46]

Should we use sortedForEach here too? Otherwise it is undefined which unresolved
we meet first. We can sort nodes by its hashcodes.

[Anton Muhin, 2012/08/21 10:15:26]

Okay.  I'll implement somewhat suboptimal solution sorting by tokens.  I don't
want to sort by hash codes as they depend on order of node construction.

Fortunately enough, I don't think we'll have tons of cases for unresolved
classes in realistic programs/tests.

On 2012/08/21 07:59:46, Roman wrote:

     renames[node] = generateUniqueName('Unresolved');
+  }
+
+  sortedForEach(mapping, f) {

[Roman, 2012/08/21 07:59:46]

Great method, but can you please make it more readable? Now it looks like
one-lined strcpy.
1) Add types to parameters. It is important that we have a map which keys are
elements.
2) this may be controversial, but I think 'final' makes little sense if used on
variables (as opposed to common usage with fields), as a reader I care about
variable type instead of whether it is final or not.
3) Please don't one-line for-s, while-s etc, it is difficult to read:
http://www.dartlang.org/articles/style-guide/#do-use-curly-braces-for-all-flo...

[Anton Muhin, 2012/08/21 10:15:26]

Done.

+    for (final e in sortElements(mapping.getKeys())) f(e, mapping[e]);
+  }
+
+  sortedForEach(placeholderCollector.elementNodes, (element, nodes) {
+    String newName = renameElement(element);
+    for (Node node in nodes) {
+      renames[node] = newName;
+    }
   });
-  placeholderCollector.elementNodes.forEach(
-      (Element element, Set<Node> nodes) {
-        String renamedElement = renameElement(element);
-        nodes.forEach((Node node) {
-          renames[node] = renamedElement;
-        });
+  sortedForEach(placeholderCollector.localPlaceholders,
+      (element, placeholders) {
+    // TODO(smok): Check for conflicts with class fields and take usages
+    // into account.
+    for (LocalPlaceholder placeholder in placeholders) {
+      for (Node node in placeholder.nodes) {
+        renames[node] = placeholder.identifier;
+      }
+    }
   });
-  placeholderCollector.localPlaceholders.forEach(
-      (FunctionElement element, Set<LocalPlaceholder> localPlaceholders) {
-        // TODO(smok): Check for conflicts with class fields and take usages
-        // into account.
-        localPlaceholders.forEach((LocalPlaceholder placeholder) {
-          placeholder.nodes.forEach((Node node) {
-            renames[node] = placeholder.identifier;
-          });
-        });
-      });
-  placeholderCollector.privateNodes.forEach(
-      (LibraryElement library, Set<Identifier> nodes) {
-        nodes.forEach((Identifier node) {
-          renames[node] =
-              renamePrivateIdentifier(library, node.source.slowToString());
-        });
+  sortedForEach(placeholderCollector.privateNodes, (library, nodes) {
+    for (Node node in nodes) {
+      renames[node] =
+          renamePrivateIdentifier(library, node.source.slowToString());
+    }
   });
 }