Refactor and simplify the dart2dart renamer.

sdk/lib/_internal/compiler/implementation/dart_backend/renamer.dart

Index: sdk/lib/_internal/compiler/implementation/dart_backend/renamer.dart
diff --git a/sdk/lib/_internal/compiler/implementation/dart_backend/renamer.dart b/sdk/lib/_internal/compiler/implementation/dart_backend/renamer.dart
index ff5b28a05e2af362616b1a05d976464ffcd8fee1..76a8129695cb31b3132a09f945040088a17098ba 100644
--- a/sdk/lib/_internal/compiler/implementation/dart_backend/renamer.dart
+++ b/sdk/lib/_internal/compiler/implementation/dart_backend/renamer.dart
@@ -7,254 +7,220 @@ part of dart_backend;
 Comparator get _compareNodes =>
     compareBy((n) => n.getBeginToken().charOffset);
 
-typedef String _Renamer(Renamable renamable);
+typedef String _NameGenerator(String original);
 
-abstract class Renamable {
+abstract class Renamable implements Comparable {
   final int RENAMABLE_TYPE_ELEMENT = 1;
   final int RENAMABLE_TYPE_MEMBER = 2;
   final int RENAMABLE_TYPE_LOCAL = 3;
 
   final Set<Node> nodes;
-  final _Renamer renamer;
 
-  Renamable(this.nodes, this.renamer);
+  Renamable(this.nodes);
   int compareTo(Renamable other) {
     int nodesDiff = other.nodes.length.compareTo(this.nodes.length);
     if (nodesDiff != 0) return nodesDiff;
-    int typeDiff = this.getTypeId().compareTo(other.getTypeId());
+    int typeDiff = this.kind.compareTo(other.kind);
     return typeDiff != 0 ? typeDiff : compareInternals(other);
   }
 
   int compareInternals(Renamable other);
-  int getTypeId();
-
-  String rename() => renamer(this);
+  int get kind;
 }
 
-class ElementRenamable extends Renamable {
-  final Element element;
+class GlobalRenamable extends Renamable {
+  final Entity entity;
 
-  ElementRenamable(this.element, Set<Node> nodes, _Renamer renamer)
-      : super(nodes, renamer);
+  GlobalRenamable(this.entity, Set<Node> nodes)
+      : super(nodes);
 
-  int compareInternals(ElementRenamable other) =>
-      compareElements(this.element, other.element);
-  int getTypeId() => RENAMABLE_TYPE_ELEMENT;
+  int compareInternals(GlobalRenamable other) =>
+      compareElements(this.entity, other.entity);
+  int get kind => RENAMABLE_TYPE_ELEMENT;
 }
 
 class MemberRenamable extends Renamable {
   final String identifier;
-  MemberRenamable(this.identifier, Set<Node> nodes, _Renamer renamer)
-      : super(nodes, renamer);
+  MemberRenamable(this.identifier, Set<Node> nodes)
+      : super(nodes);
   int compareInternals(MemberRenamable other) =>
       this.identifier.compareTo(other.identifier);
-  int getTypeId() => RENAMABLE_TYPE_MEMBER;
+  int get kind => RENAMABLE_TYPE_MEMBER;
 }
 
 class LocalRenamable extends Renamable {
-  LocalRenamable(Set<Node> nodes, _Renamer renamer) : super(nodes, renamer);
+  LocalRenamable(Set<Node> nodes)
+      : super(nodes);
   int compareInternals(LocalRenamable other) =>
       _compareNodes(sorted(this.nodes, _compareNodes)[0],
           sorted(other.nodes, _compareNodes)[0]);
-  int getTypeId() => RENAMABLE_TYPE_LOCAL;
+  int get kind => RENAMABLE_TYPE_LOCAL;
 }
 
 /**
- * Renames only top-level elements that would let to ambiguity if not renamed.
+ * Renames only top-level elements that would lead to ambiguity if not renamed.
  */
-void renamePlaceholders(
-    Compiler compiler,
-    PlaceholderCollector placeholderCollector,
-    Map<Node, String> renames,
-    Map<LibraryElement, String> imports,
-    Set<String> fixedMemberNames,
-    Map<Element, LibraryElement> reexportingLibraries,
-    bool cutDeclarationTypes,
-    {bool uniqueGlobalNaming: false}) {
-  final Map<LibraryElement, Map<String, String>> renamed
-      = new Map<LibraryElement, Map<String, String>>();
-
-  renameNodes(Iterable<Node> nodes, renamer) {
+class PlaceholderRenamer {
+  /// After running [computeRenames] this will contain the computed renames.
+  final Map<Node, String> renames = new Map<Node, String>();
+  /// After running [computeRenames] this will contain the used platform
+  /// libraries.
+  final Set<LibraryElement> platformImports = new Set<LibraryElement>();
+
+  final Compiler _compiler;
+  final Set<String> fixedMemberNames;
+  final Map<Element, LibraryElement> reexportingLibraries;
+  final bool cutDeclarationTypes;
+
+  final Map<Entity, String> _renamedCache = new Map<Entity, String>();
+  final Map<Entity, Map<String, String>> _privateCache =
+      new Map<Entity, Map<String, String>>();
+
+  final MinifyingGenerator _minifyingGenerator = new MinifyingGenerator();
+  _NameGenerator _generateUniqueName;
+
+  PlaceholderRenamer(this._compiler, this.fixedMemberNames,
+      this.reexportingLibraries, {this.cutDeclarationTypes}) {
+

[Johnni Winther, 2014/08/15 07:49:19]

Replace body by ;

[sigurdm, 2014/08/15 13:06:27]

Done.

+  }
+
+  void _renameNodes(Iterable<Node> nodes, String renamer(Node node)) {
     for (Node node in sorted(nodes, _compareNodes)) {
       renames[node] = renamer(node);
     }
   }
 
-  sortedForEach(Map<Element, dynamic> map, f) {
-    for (Element element in sortElements(map.keys)) {
-      f(element, map[element]);
-    }
-  }
 
-  String renameType(DartType type, Function renameElement) {
-    if (type.isDynamic) return 'dynamic';
-    // TODO(smok): Do not rename type if it is in platform library or
-    // js-helpers.
-    StringBuffer result = new StringBuffer(renameElement(type.element));
-    if (type is GenericType && !type.treatAsRaw) {
-      result.write('<');
-      List<DartType> arguments = type.typeArguments;
-      result.write(renameType(arguments.first, renameElement));
-      for (int index = 1; index < arguments.length; index++) {
-        result.write(',');
-        result.write(renameType(arguments[index], renameElement));
-      }
-      result.write('>');
-    }
-    return result.toString();
+  /// Looks up [originalName] in the [_privateCache] cache of [library].
+  /// If [originalName] was not renamed before, generate a new name.
+  String _getPrivateName(LibraryElement library, String originalName) {
+    return _privateCache.putIfAbsent(library, () => new Map<String, String>())
+        .putIfAbsent(originalName, () => _generateUniqueName(originalName));
   }
 
-  String renameConstructor(Element element, ConstructorPlaceholder placeholder,
-      Function renameString, Function renameElement) {
-    assert(element.isConstructor);
-    StringBuffer result = new StringBuffer();
-    String name = element.name;
-    if (element.name != '') {
-      // Named constructor or factory. Is there a more reliable way to check
-      // this case?
-      if (!placeholder.isRedirectingCall) {
-        result.write(renameType(placeholder.type, renameElement));
-        result.write('.');
-      }
-      if (!element.library.isPlatformLibrary) {
-        name = renameString(element.library, name);
-      }
-      result.write(name);
-    } else {
-      assert(!placeholder.isRedirectingCall);
-      result.write(renameType(placeholder.type, renameElement));
-    }
-    return result.toString();
+  String _renameConstructor(ConstructorPlaceholder placeholder) {
+    String name = placeholder.element.name;
+    if (name == '') return "";
+    String result = _renameGlobal(placeholder.element);
+    return result;
   }
 
-  Function makeElementRenamer(rename, generateUniqueName) => (element) {
-    assert(Elements.isErroneousElement(element) ||
-           Elements.isStaticOrTopLevel(element) ||
-           element is TypeVariableElement);
+  String _renameGlobal(Entity entity) {
+    assert(entity is! Element ||
+           Elements.isErroneousElement(entity) ||
+           Elements.isStaticOrTopLevel(entity) ||
+           entity is TypeVariableElement);
     // TODO(smok): We may want to reuse class static field and method names.
-    String originalName = element.name;
-    LibraryElement library = element.library;
-    if (identical(element.library, compiler.coreLibrary)) {
-      return originalName;
-    }
-    if (library.isPlatformLibrary) {
-      assert(element.isTopLevel);
-      if (reexportingLibraries.containsKey(element)) {
-        library = reexportingLibraries[element];
+    if (entity is Element) {
+      LibraryElement library = entity.library;
+      if (reexportingLibraries.containsKey(entity)) {
+        library = reexportingLibraries[entity];
       }
-      if (!library.isInternalLibrary) {
-        final prefix =
-            imports.putIfAbsent(library, () => generateUniqueName('p'));
-        return '$prefix.$originalName';
+      if (library.isPlatformLibrary) {
+        if (library != _compiler.coreLibrary) {
+          platformImports.add(library);
+        }
+        if (library.isInternalLibrary) {
+          throw new SpannableAssertionFailure(entity,
+              "Internal library $library should never have been imported from "
+              "the code compiled by dart2dart.");
+        }
+        return entity.name;
       }
     }
 
-    return rename(library, originalName);
-  };
-
-  Function makeRenamer(generateUniqueName) =>
-      (library, originalName) =>
-          renamed.putIfAbsent(library, () => {})
-              .putIfAbsent(originalName,
-                  () => generateUniqueName(originalName));
-
-  // Renamer function that takes library and original name and returns a new
-  // name for given identifier.
-  Function rename;
-  Function renameElement;
-  // A function that takes original identifier name and generates a new unique
-  // identifier.
-  Function generateUniqueName;
-
-  Set<String> allNamedParameterIdentifiers = new Set<String>();
-  for (var functionScope in placeholderCollector.functionScopes.values) {
-    allNamedParameterIdentifiers.addAll(functionScope.parameterIdentifiers);
+    // Look up in [_renamedCache] for a name for [entity] .
+    // If it was not renamed before, generate a new name.
+    return _renamedCache.putIfAbsent(entity,
+        () => _generateUniqueName(entity.name));
   }
 
-  if (compiler.enableMinification) {
-    MinifyingGenerator generator = new MinifyingGenerator();
-    Set<String> forbiddenIdentifiers = new Set<String>.from(['main']);
-    forbiddenIdentifiers.addAll(Keyword.keywords.keys);
-    forbiddenIdentifiers.addAll(fixedMemberNames);
-    generateUniqueName = (_) =>
-        generator.generate((name) =>
-            forbiddenIdentifiers.contains(name)
-            || allNamedParameterIdentifiers.contains(name));
-    rename = makeRenamer(generateUniqueName);
-    renameElement = makeElementRenamer(rename, generateUniqueName);
-
-    List<Set<Node>> allLocals = new List<Set<Node>>();
-
+  void _computeMinifiedRenames(PlaceholderCollector placeholderCollector,
+                               Set<String> allNamedParameterIdentifiers,
+                               Set<String> forbiddenIdentifiers) {
+    String generateUniqueMinifiedName() {
+      return _minifyingGenerator.generate((name) =>
+          forbiddenIdentifiers.contains(name)
+          || allNamedParameterIdentifiers.contains(name));
+    }
+    _generateUniqueName = (_) => generateUniqueMinifiedName();
     // Build a list sorted by usage of local nodes that will be renamed to
     // the same identifier. So the top-used local variables in all functions
     // will be renamed first and will all share the same new identifier.
-    for (var functionScope in placeholderCollector.functionScopes.values) {
+    int maxLength = placeholderCollector.functionScopes.values.fold(0,
+        (a, b) => max(a, b.localPlaceholders.length));
+
+    List<Set<Node>> allLocals = new List<Set<Node>>
+        .generate(maxLength, (_) => new Set<Node>());
+
+    for (FunctionScope functionScope
+        in placeholderCollector.functionScopes.values) {
       // Add current sorted local identifiers to the whole sorted list
       // of all local identifiers for all functions.
       List<LocalPlaceholder> currentSortedPlaceholders =
           sorted(functionScope.localPlaceholders,
               compareBy((LocalPlaceholder ph) => -ph.nodes.length));
-      List<Set<Node>> currentSortedNodes =
-          currentSortedPlaceholders.map((ph) => ph.nodes).toList();
-      // Make room in all sorted locals list for new stuff.
-      while (currentSortedNodes.length > allLocals.length) {
-        allLocals.add(new Set<Node>());
-      }
+
+      List<Set<Node>> currentSortedNodes = currentSortedPlaceholders
+          .map((LocalPlaceholder ph) => ph.nodes).toList();
+
       for (int i = 0; i < currentSortedNodes.length; i++) {
         allLocals[i].addAll(currentSortedNodes[i]);
       }
     }
 
-    // Rename elements, members and locals together based on their usage count,
-    // otherwise when we rename elements first there will be no good identifiers
-    // left for members even if they are used often.
-    String elementRenamer(ElementRenamable elementRenamable) =>
-        renameElement(elementRenamable.element);
-    String memberRenamer(MemberRenamable memberRenamable) =>
-        generator.generate(forbiddenIdentifiers.contains);
-    Function localRenamer = generateUniqueName;
-    List<Renamable> renamables = [];
+    // Rename elements, members and locals together based on their usage
+    // count, otherwise when we rename elements first there will be no good
+    // identifiers left for members even if they are used often.
+    List<Renamable> renamables = new List<Renamable>();
     placeholderCollector.elementNodes.forEach(
         (Element element, Set<Node> nodes) {
-      renamables.add(new ElementRenamable(element, nodes, elementRenamer));
+      renamables.add(new GlobalRenamable(element, nodes));
     });
     placeholderCollector.memberPlaceholders.forEach(
         (String memberName, Set<Identifier> identifiers) {
       renamables.add(
-          new MemberRenamable(memberName, identifiers, memberRenamer));
+          new MemberRenamable(memberName, identifiers));
     });
     for (Set<Node> localIdentifiers in allLocals) {
-      renamables.add(new LocalRenamable(localIdentifiers, localRenamer));
+      renamables.add(new LocalRenamable(localIdentifiers));
     }
-    renamables.sort((Renamable renamable1, Renamable renamable2) =>
-        renamable1.compareTo(renamable2));
+    renamables.sort();
     for (Renamable renamable in renamables) {
-      String newName = renamable.rename();
-      renameNodes(renamable.nodes, (_) => newName);
+      String newName;
+      if (renamable is GlobalRenamable) {

[Johnni Winther, 2014/08/15 07:49:19]

These could be different implementations of a [createNewName] function on
Renamble.

[sigurdm, 2014/08/15 13:06:27]

Done.

+        newName = _renameGlobal(renamable.entity);
+      } else if (renamable is MemberRenamable) {
+        newName = _minifyingGenerator.generate(forbiddenIdentifiers.contains);
+      } else if (renamable is LocalRenamable) {
+        newName = generateUniqueMinifiedName();
+      } else {
+        throw "Unknown type of renamable";
+      }
+      _renameNodes(renamable.nodes, (_) => newName);
     }
-  } else {
-    // Never rename anything to 'main'.
-    final usedTopLevelOrMemberIdentifiers = new Set<String>();
-    usedTopLevelOrMemberIdentifiers.add('main');
-    usedTopLevelOrMemberIdentifiers.addAll(fixedMemberNames);
-    generateUniqueName = (originalName) {
-      String newName = conservativeGenerator(
-          originalName, (name) =>
-              usedTopLevelOrMemberIdentifiers.contains(name)
-              || allNamedParameterIdentifiers.contains(name));
-      usedTopLevelOrMemberIdentifiers.add(newName);
+  }
+
+  void _computeNonMinifiedRenames(PlaceholderCollector placeholderCollector,
+                                  Set<String> allNamedParameterIdentifiers,
+                                  Set<String> forbiddenIdentifiers) {
+    String generateUniqueNonminifiedName(originalName) {
+      String newName = conservativeGenerator(originalName, (name) {
+        return forbiddenIdentifiers.contains(name) ||
+            allNamedParameterIdentifiers.contains(name);
+      });
+      forbiddenIdentifiers.add(newName);
       return newName;
-    };
-    rename = makeRenamer(generateUniqueName);
-    renameElement = makeElementRenamer(rename, generateUniqueName);
+    }
+    _generateUniqueName = generateUniqueNonminifiedName;
     // Rename elements.
-    sortedForEach(placeholderCollector.elementNodes,
+    placeholderCollector.elementNodes.forEach(
         (Element element, Set<Node> nodes) {
-      renameNodes(nodes, (_) => renameElement(element));
+      _renameNodes(nodes, (_) => _renameGlobal(element));
     });
 
     // Rename locals.
-    sortedForEach(placeholderCollector.functionScopes,
+    placeholderCollector.functionScopes.forEach(
         (functionElement, functionScope) {
       Set<LocalPlaceholder> placeholders = functionScope.localPlaceholders;
       Set<String> memberIdentifiers = new Set<String>();
@@ -269,43 +235,78 @@ void renamePlaceholders(
         String nextId =
             conservativeGenerator(placeholder.identifier, (name) =>
                 functionScope.parameterIdentifiers.contains(name)
-                || usedTopLevelOrMemberIdentifiers.contains(name)
+                || forbiddenIdentifiers.contains(name)
                 || usedLocalIdentifiers.contains(name)
                 || memberIdentifiers.contains(name));
         usedLocalIdentifiers.add(nextId);
-        renameNodes(placeholder.nodes, (_) => nextId);
+        _renameNodes(placeholder.nodes, (_) => nextId);
       }
     });
 
-    final usedMemberIdentifiers = new Set<String>.from(fixedMemberNames);
     // Do not rename members to top-levels, that allows to avoid renaming
     // members to constructors.
-    usedMemberIdentifiers.addAll(usedTopLevelOrMemberIdentifiers);
     placeholderCollector.memberPlaceholders.forEach((identifier, nodes) {
       String newIdentifier = conservativeGenerator(
-          identifier, usedMemberIdentifiers.contains);
-      renameNodes(nodes, (_) => newIdentifier);
+          identifier, forbiddenIdentifiers.contains);
+      _renameNodes(nodes, (_) => newIdentifier);
     });
   }
 
-  // Rename constructors.
-  sortedForEach(placeholderCollector.constructorPlaceholders,
-      (Element constructor, List<ConstructorPlaceholder> placeholders) {
-        for (ConstructorPlaceholder ph in placeholders) {
-          renames[ph.node] =
-              renameConstructor(constructor, ph, rename, renameElement);
-        }
-  });
-  sortedForEach(placeholderCollector.privateNodes, (library, nodes) {
-    renameNodes(nodes, (node) => rename(library, node.source));
-  });
-  renameNodes(placeholderCollector.unresolvedNodes,
-      (_) => generateUniqueName('Unresolved'));
-  renameNodes(placeholderCollector.nullNodes, (_) => '');
-  if (cutDeclarationTypes) {
-    for (DeclarationTypePlaceholder placeholder in
-         placeholderCollector.declarationTypePlaceholders) {
-      renames[placeholder.typeNode] = placeholder.requiresVar ? 'var' : '';
+  /// Finds renamings for all the placeholders in [placeholderCollector] and
+  /// stores them in [renames].
+  /// Also adds to [platformImports] all the platform-libraries that are used.
+  void computeRenames(PlaceholderCollector placeholderCollector) {
+    Set<String> allNamedParameterIdentifiers = new Set<String>();
+    for (FunctionScope functionScope in
+        placeholderCollector.functionScopes.values) {
+      allNamedParameterIdentifiers.addAll(functionScope.parameterIdentifiers);
+    }
+
+    Set<String> forbiddenIdentifiers = new Set<String>.from(fixedMemberNames);
+    forbiddenIdentifiers.addAll(Keyword.keywords.keys);
+    forbiddenIdentifiers.add('main');
+
+    if (_compiler.enableMinification) {
+      _computeMinifiedRenames(placeholderCollector,
+                                allNamedParameterIdentifiers,
+                                forbiddenIdentifiers);
+
+    } else {
+      _computeNonMinifiedRenames(placeholderCollector,
+                                 allNamedParameterIdentifiers,
+                                 forbiddenIdentifiers);
+    }
+
+    // Rename constructors.
+    for (ConstructorPlaceholder placeholder in
+        placeholderCollector.constructorPlaceholders) {
+      renames[placeholder.node] =
+          _renameConstructor(placeholder);
+    };
+
+    // Rename private identifiers uniquely for each library.
+    placeholderCollector.privateNodes.forEach(
+        (Element library, Set<Identifier> identifiers) {

[Johnni Winther, 2014/08/15 07:49:19]

Element library -> LibraryElement library

[sigurdm, 2014/08/15 13:06:27]

Done.

+      for (Identifier identifier in identifiers) {
+        renames[identifier] = _getPrivateName(library, identifier.source);
+      }
+    });
+
+    // Rename unresolved nodes, to make sure they still do not resolve.
+    for (Node node in placeholderCollector.unresolvedNodes) {
+      renames[node] = _generateUniqueName('Unresolved');
+    }
+
+    // Erase prefixes that are now not needed.
+    for (Node node in placeholderCollector.prefixNodesToErase) {
+      renames[node] = '';
+    }
+
+    if (cutDeclarationTypes) {
+      for (DeclarationTypePlaceholder placeholder in
+           placeholderCollector.declarationTypePlaceholders) {
+        renames[placeholder.typeNode] = placeholder.requiresVar ? 'var' : '';
+      }
     }
   }
 }
@@ -341,7 +342,7 @@ String conservativeGenerator(String name, bool isForbidden(String name)) {
   String result = name;
   int index = 0;
   while (isForbidden(result)) {
-    result = '${generateMiniId(index++)}_$name';
+    result = '${name}_${generateMiniId(index++)}';
   }
   return result;
 }