Rename elements after sorting.

lib/compiler/implementation/dart_backend/renamer.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.
 
 /**
  * Renames only top-level elements that would let to ambiguity if not renamed.
  * TODO(smok): Make sure that top-level fields are correctly renamed.
  */
 void renamePlaceholders(
     Compiler compiler,
@@ skipping 32 matching lines @@
     if (isDartCoreLib(compiler, library)) {
       final prefix =
           imports.putIfAbsent(library, () => generateUniqueName('p'));
       return '$prefix.$originalName';
     }
 
     return getName(library, originalName,
                    () => generateUniqueName(originalName));
   }
 
-  placeholderCollector.nullNodes.forEach((Node node) {
+  for (Node node in placeholderCollector.nullNodes) {
     renames[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.unresolvedNodes.forEach((Node node) {
-    renames[node] = generateUniqueName('Unresolved');
+  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.elementNodes.forEach(
-      (Element element, Set<Node> nodes) {
-        String renamedElement = renameElement(element);
-        nodes.forEach((Node node) {
-          renames[node] = renamedElement;
-        });
-  });
-  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());
+    }
   });
 }
 
 typedef String Generator(String originalName, bool isForbidden(String name));
 
 /** Always tries to return original identifier name unless it is forbidden. */
 String conservativeGenerator(
     String originalName, bool isForbidden(String name)) {
   String newName = originalName;
   while (isForbidden(newName)) {
@@ skipping 29 matching lines @@
     int length = alphabet.length;
     StringBuffer resultBuilder = new StringBuffer();
     while (index >= length) {
       resultBuilder.add(alphabet[index % length]);
       index ~/= length;
     }
     resultBuilder.add(alphabet[index]);
     return resultBuilder.toString();
   }
 }