Reapply class/method/field minification

sdk/lib/_internal/compiler/implementation/js_backend/namer.dart

 // Copyright (c) 2011, 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.
 
 part of js_backend;
 
 /**
  * Assigns JavaScript identifiers to Dart variables, class-names and members.
  */
 class Namer {
-  final Compiler compiler;
-
   static Set<String> _jsReserved = null;
   Set<String> get jsReserved {
     if (_jsReserved == null) {
       _jsReserved = new Set<String>();
       _jsReserved.addAll(JsNames.javaScriptKeywords);
       _jsReserved.addAll(JsNames.reservedPropertySymbols);
     }
     return _jsReserved;
   }
 
+  final String CURRENT_ISOLATE = r'$';
+
   /**
    * Map from top-level or static elements to their unique identifiers provided
    * by [getName].
    *
    * Invariant: Keys must be declaration elements.
    */
+  final Compiler compiler;
   final Map<Element, String> globals;
-  final Map<String, int> usedGlobals;
   final Map<String, LibraryElement> shortPrivateNameOwners;
+  final Set<String> usedGlobalNames;
+  final Set<String> usedInstanceNames;
+  final Map<String, String> instanceNameMap;
+  final Map<String, String> globalNameMap;
+  final Map<String, int> popularNameCounters;
 
   /**
    * A cache of names used for bailout methods. We make sure two
    * bailout methods cannot have the same name because if the two
    * bailout methods are in a class and a subclass, we would
    * call the wrong bailout method at runtime. To make it
    * simple, we don't keep track of inheritance and always avoid
    * similar names.
    */
   final Set<String> usedBailoutInstanceNames;
   final Map<Element, String> bailoutNames;
 
   final Map<Constant, String> constantNames;
 
   Namer(this.compiler)
       : globals = new Map<Element, String>(),
-        usedGlobals = new Map<String, int>(),
         shortPrivateNameOwners = new Map<String, LibraryElement>(),
         bailoutNames = new Map<Element, String>(),
         usedBailoutInstanceNames = new Set<String>(),
-        constantNames = new Map<Constant, String>();
+        usedGlobalNames = new Set<String>(),
+        usedInstanceNames = new Set<String>(),
+        instanceNameMap = new Map<String, String>(),
+        globalNameMap = new Map<String, String>(),
+        constantNames = new Map<Constant, String>(),
+        popularNameCounters = new Map<String, int>();
 
-  final String CURRENT_ISOLATE = r'$';
-  final String ISOLATE = 'Isolate';
-  final String ISOLATE_PROPERTIES = r"$isolateProperties";
+  String get ISOLATE => 'Isolate';
+  String get ISOLATE_PROPERTIES => r'$isolateProperties';
   /**
    * Some closures must contain their name. The name is stored in
    * [STATIC_CLOSURE_NAME_NAME].
    */
-  final String STATIC_CLOSURE_NAME_NAME = r'$name';
-  static const SourceString CLOSURE_INVOCATION_NAME =
-      Compiler.CALL_OPERATOR_NAME;
+  String get STATIC_CLOSURE_NAME_NAME => r'$name';
+  SourceString get CLOSURE_INVOCATION_NAME => Compiler.CALL_OPERATOR_NAME;
+  bool get shouldMinify => false;
+
+  bool isReserved(String name) => name == ISOLATE;
 
   String constantName(Constant constant) {
     // In the current implementation it doesn't make sense to give names to
     // function constants since the function-implementation itself serves as
     // constant and can be accessed directly.
     assert(!constant.isFunction());
     String result = constantNames[constant];
     if (result == null) {
-      result = getFreshGlobalName("CTC");
+      String longName;
+      if (shouldMinify) {
+        if (constant.isString()) {
+          StringConstant stringConstant = constant;
+          // The minifier never returns the same string as we suggested so we

[floitsch, 2012/12/07 12:01:01]

The minifier always constructs a new name, using the argument as input to its
hashing algorithm. The given name does not need to be valid.

No need to talk about the tendency to have similar names. That should be a
comment on the minifier, but not here.

[erikcorry, 2012/12/10 08:36:01]

Done.

+          // can suggest any name and it will use it as input to the hashing
+          // algorithm.  This means that constants will tend to have the same
+          // name from version to version of the program being minfied.
+          longName = stringConstant.value.slowToString();
+        } else {
+          longName = "C";
+        }
+      } else {
+        longName = "CTC";

[floitsch, 2012/12/07 12:01:01]

We could maybe switch to "CONSTANT" now? (but no strong opinion).

[erikcorry, 2012/12/10 08:36:01]

Done.

+      }
+      result = getFreshName(longName, usedGlobalNames);
       constantNames[constant] = result;
     }
     return result;
   }
 
   String closureInvocationName(Selector selector) {
-    // TODO(floitsch): mangle, while not conflicting with instance names.
-    return instanceMethodInvocationName(null, CLOSURE_INVOCATION_NAME,
-                                        selector);
+    return
+        instanceMethodInvocationName(null, CLOSURE_INVOCATION_NAME, selector);
   }
 
   String breakLabelName(LabelElement label) {
     return '\$${label.labelName}\$${label.target.nestingLevel}';
   }
 
   String implicitBreakLabelName(TargetElement target) {
     return '\$${target.nestingLevel}';
   }
 
   // We sometimes handle continue targets differently from break targets,
   // so we have special continue-only labels.
   String continueLabelName(LabelElement label) {
     return 'c\$${label.labelName}\$${label.target.nestingLevel}';
   }
 
   String implicitContinueLabelName(TargetElement target) {
     return 'c\$${target.nestingLevel}';
   }
 
   /**
    * If the [name] is not private returns [:name.slowToString():]. Otherwise
    * mangles the [name] so that each library has a unique name.
    */
   String privateName(LibraryElement lib, SourceString name) {
+    String result;
     if (name.isPrivate()) {
       String nameString = name.slowToString();
       // The first library asking for a short private name wins.
       LibraryElement owner =

[floitsch, 2012/12/07 12:01:01]

I don't think you need to fill the shortPrivateNameOwners if you are minifying.
You should probably guard this with the 'shouldMinify' flag.

[erikcorry, 2012/12/10 08:36:01]

Done.

           shortPrivateNameOwners.putIfAbsent(nameString, () => lib);
       // If a private name could clash with a mangled private name we don't
       // use the short name. For example a private name "_lib3_foo" would
       // clash with "_foo" from "lib3".
-      if (identical(owner, lib) && !nameString.startsWith('_$LIBRARY_PREFIX')) {
-        return nameString;
+      if (owner == lib &&
+          !nameString.startsWith('_$LIBRARY_PREFIX') &&
+          !shouldMinify) {
+        result = nameString;
+      } else {
+        String libName = getName(lib);
+        // If a library name does not start with the [LIBRARY_PREFIX] then our
+        // assumptions about clashing with mangled private members do not hold.
+        assert(shouldMinify || libName.startsWith(LIBRARY_PREFIX));
+        // TODO(erikcorry): Fix this with other manglings to avoid clashes.
+        result = '_lib$libName\$$nameString';
       }
-      String libName = getName(lib);
-      // If a library name does not start with the [LIBRARY_PREFIX] then our
-      // assumptions about clashing with mangled private members do not hold.
-      assert(libName.startsWith(LIBRARY_PREFIX));
-      return '_$libName$nameString';
     } else {
-      return name.slowToString();
+      result = name.slowToString();
     }
+    return result;
   }
 
   String instanceMethodName(FunctionElement element) {
     SourceString name = Elements.operatorNameToIdentifier(element.name);
     LibraryElement lib = element.getLibrary();
     if (element.kind == ElementKind.GENERATIVE_CONSTRUCTOR_BODY) {
       ConstructorBodyElement bodyElement = element;
       name = bodyElement.constructor.name;
     }
     FunctionSignature signature = element.computeSignature(compiler);
     String methodName =
         '${privateName(lib, name)}\$${signature.parameterCount}';
-    if (!signature.optionalParametersAreNamed) {
-      return methodName;
-    } else if (!signature.optionalParameters.isEmpty) {
+    if (signature.optionalParametersAreNamed &&
+        !signature.optionalParameters.isEmpty) {
       StringBuffer buffer = new StringBuffer();
       signature.orderedOptionalParameters.forEach((Element element) {
         buffer.add('\$${JsNames.getValid(element.name.slowToString())}');
       });
-      return '$methodName$buffer';
+      methodName = '$methodName$buffer';
     }
+    if (name == CLOSURE_INVOCATION_NAME) return methodName;
+    return getMappedInstanceName(methodName);
   }
 
   String publicInstanceMethodNameByArity(SourceString name, int arity) {
     name = Elements.operatorNameToIdentifier(name);
     assert(!name.isPrivate());
-    return '${name.slowToString()}\$$arity';
+    var base = name.slowToString();
+    // We don't mangle the closure invoking function name because it is
+    // generated in applyFunction.
+    var proposedName = '$base\$$arity';
+    if (base == CLOSURE_INVOCATION_NAME) return proposedName;
+    return getMappedInstanceName(proposedName);
   }
 
   String instanceMethodInvocationName(LibraryElement lib, SourceString name,
                                       Selector selector) {
     name = Elements.operatorNameToIdentifier(name);
     // TODO(floitsch): mangle, while preserving uniqueness.
     StringBuffer buffer = new StringBuffer();
     List<SourceString> names = selector.getOrderedNamedArguments();
     for (SourceString argumentName in names) {
       buffer.add(r'$');
       argumentName.printOn(buffer);
     }
-    return '${privateName(lib, name)}\$${selector.argumentCount}$buffer';
+    if (name == CLOSURE_INVOCATION_NAME) {

[floitsch, 2012/12/07 12:01:01]

Copy comment from above. (saying that closure invoking function name is not
mangled because of "applyFunction").

[erikcorry, 2012/12/10 08:36:01]

Done.

+      return '$CLOSURE_INVOCATION_NAME\$${selector.argumentCount}$buffer';
+    }
+    return getMappedInstanceName(
+        '${privateName(lib, name)}\$${selector.argumentCount}$buffer');
   }
 
   String instanceFieldName(LibraryElement libraryElement, SourceString name) {
     String proposedName = privateName(libraryElement, name);
-    return safeName(proposedName);
+    return getMappedInstanceName(proposedName);
   }
 
+  // Construct a new name for the element based on the library and class it is
+  // in.  The name here is not important, we just need to make sure it is
+  // unique.  If we are minifying, we actually construct the name from the
+  // minfiied versions of the class and instance names, but the result is

[sra1, 2012/12/07 06:03:18]

minfiied -> minified

[erikcorry, 2012/12/10 08:36:01]

Done.

+  // minified once again, so that is not visible in the end result.
   String shadowedFieldName(Element fieldElement) {
+    // Check for following situation: Native field ${fieldElement.name} has
+    // fixed JSName ${fieldElement.nativeName()}, but a subclass shadows this
+    // name.  We normally handle that by renaming the superclass field, but we
+    // can't do that because native fields have fixed JSNames.  In practice
+    // this can't happen because we can't inherit from native classes.
+    assert (!fieldElement.isNative());
+
     ClassElement cls = fieldElement.getEnclosingClass();
     LibraryElement libraryElement = fieldElement.getLibrary();
     String libName = getName(libraryElement);
     String clsName = getName(cls);
     String instanceName = instanceFieldName(libraryElement, fieldElement.name);
-    return safeName('$libName\$$clsName\$$instanceName');
+    return getMappedInstanceName('$libName\$$clsName\$$instanceName');
   }
 
   String setterName(LibraryElement lib, SourceString name) {
     // We dynamically create setters from the field-name. The setter name must
     // therefore be derived from the instance field-name.
-    String fieldName = safeName(privateName(lib, name));
+    String fieldName = getMappedInstanceName(privateName(lib, name));
     return 'set\$$fieldName';
   }
 
+  String setterNameFromAccessorName(String name) {
+    // We dynamically create setters from the field-name. The setter name must
+    // therefore be derived from the instance field-name.
+    return 'set\$$name';
+  }
+
   String publicGetterName(SourceString name) {
     // We dynamically create getters from the field-name. The getter name must
     // therefore be derived from the instance field-name.
-    String fieldName = safeName(name.slowToString());
+    String fieldName = getMappedInstanceName(name.slowToString());
     return 'get\$$fieldName';
   }
 
+  String getterNameFromAccessorName(String name) {
+    // We dynamically create getters from the field-name. The getter name must
+    // therefore be derived from the instance field-name.
+    return 'get\$$name';
+  }
+
   String getterName(LibraryElement lib, SourceString name) {
     // We dynamically create getters from the field-name. The getter name must
     // therefore be derived from the instance field-name.
-    String fieldName = safeName(privateName(lib, name));
+    String fieldName = getMappedInstanceName(privateName(lib, name));
     return 'get\$$fieldName';
   }
 
-  String getFreshGlobalName(String proposedName) {
-    String name = proposedName;
-    int count = usedGlobals[name];
-    if (count != null) {
-      // Not the first time we see this name. Append a number to make it unique.
-      do {
-        name = '$proposedName${count++}';
-      } while (usedGlobals[name] != null);
-      // Record the count in case we see this name later. We
-      // frequently see names multiple times, as all our closures use
-      // the same name for their class.
-      usedGlobals[proposedName] = count;
+  String getMappedGlobalName(String proposedName) {
+    var newName = globalNameMap[proposedName];
+    if (newName == null) {
+      newName = getFreshName(proposedName, usedGlobalNames);
+      globalNameMap[proposedName] = newName;
     }
-    usedGlobals[name] = 0;
-    return name;
+    return newName;
+  }
+
+  String getMappedInstanceName(String proposedName) {
+    var newName = instanceNameMap[proposedName];
+    if (newName == null) {
+      newName = getFreshName(proposedName, usedInstanceNames);
+      instanceNameMap[proposedName] = newName;
+    }
+    return newName;
+  }
+
+  String getFreshName(String proposedName, Set<String> usedNames) {
+    var candidate;
+    proposedName = safeName(proposedName);
+    if (!usedNames.contains(proposedName)) {
+      candidate = proposedName;
+    } else {
+      var counter = popularNameCounters[proposedName];
+      var i = counter == null ? 0 : counter;
+      while (usedNames.contains("$proposedName$i")) {
+        i++;
+      }
+      popularNameCounters[proposedName] = i + 1;
+      candidate = "$proposedName$i";
+    }
+    usedNames.add(candidate);
+    return candidate;
   }
 
   static const String LIBRARY_PREFIX = "lib";
 
   /**
    * Returns a preferred JS-id for the given top-level or static element.
    * The returned id is guaranteed to be a valid JS-id.
    */
   String _computeGuess(Element element) {
     assert(!element.isInstanceMember());
@@ skipping 12 matching lines @@
         name = "${enclosingClass.name.slowToString()}_"
                "${element.name.slowToString()}";
       } else {
         name = element.name.slowToString();
       }
     } else if (element.isLibrary()) {
       name = LIBRARY_PREFIX;
     } else {
       name = element.name.slowToString();
     }
-    return safeName(name);
+    return name;
   }
 
   String getSpecializedName(Element element, Collection<ClassElement> classes) {
+    // This gets the minified name, but it doesn't really make much difference.
+    // the important thing is that it is a unique name.
     StringBuffer buffer = new StringBuffer('${getName(element)}\$');
     for (ClassElement cls in classes) {
       buffer.add(getName(cls));
     }
-    return safeName(buffer.toString());
+    return getMappedGlobalName(buffer.toString());
   }
 
   String getBailoutName(Element element) {
     String name = bailoutNames[element];
     if (name != null) return name;
     bool global = !element.isInstanceMember();
+    // Despite the name of the variable, this gets the minified name when we
+    // are minifying, but it doesn't really make much difference.  The
+    // important thing is that it is a unique name.  We add $bailout and, if we
+    // are minifying, we minify the minified name and '$bailout'.
     String unminifiedName = '${getName(element)}\$bailout';
-    name = unminifiedName;
-    if (!global) {
+    if (global) {
+      name = getMappedGlobalName(unminifiedName);
+    } else {
+      name = unminifiedName;
       int i = 0;
       while (usedBailoutInstanceNames.contains(name)) {
         name = '$unminifiedName${i++}';
       }
       usedBailoutInstanceNames.add(name);
+      name = getMappedInstanceName(name);
     }
     bailoutNames[element] = name;
     return name;
   }
 
   /**
    * Returns a preferred JS-id for the given element. The returned id is
    * guaranteed to be a valid JS-id. Globals and static fields are furthermore
    * guaranteed to be unique.
    *
@@ skipping 20 matching lines @@
       element = element.declaration;
       // Dealing with a top-level or static element.
       String cached = globals[element];
       if (cached != null) return cached;
 
       String guess = _computeGuess(element);
       ElementKind kind = element.kind;
       if (identical(kind, ElementKind.VARIABLE) ||
           identical(kind, ElementKind.PARAMETER)) {
         // The name is not guaranteed to be unique.
-        return guess;
+        return safeName(guess);
       }
-      if (identical(kind, ElementKind.GENERATIVE_CONSTRUCTOR) ||
-          identical(kind, ElementKind.FUNCTION) ||
-          identical(kind, ElementKind.CLASS) ||
-          identical(kind, ElementKind.FIELD) ||
-          identical(kind, ElementKind.GETTER) ||
-          identical(kind, ElementKind.SETTER) ||
-          identical(kind, ElementKind.TYPEDEF) ||
-          identical(kind, ElementKind.LIBRARY) ||
-          identical(kind, ElementKind.MALFORMED_TYPE)) {
-        String result = getFreshGlobalName(guess);
+      if (kind == ElementKind.GENERATIVE_CONSTRUCTOR ||
+          kind == ElementKind.FUNCTION ||
+          kind == ElementKind.CLASS ||
+          kind == ElementKind.FIELD ||
+          kind == ElementKind.GETTER ||
+          kind == ElementKind.SETTER ||
+          kind == ElementKind.TYPEDEF ||
+          kind == ElementKind.LIBRARY ||
+          kind == ElementKind.MALFORMED_TYPE) {
+        bool isNative = false;
+        if (identical(kind, ElementKind.CLASS)) {

[floitsch, 2012/12/07 12:01:01]

use "=="

[erikcorry, 2012/12/10 08:36:01]

Done.

+          ClassElement class_elt = element;
+          isNative = class_elt.isNative();
+        }
+        if (Elements.isInstanceField(element)) {
+          isNative = element.isNative();
+        }
+        String result = isNative ? guess : getFreshName(guess, usedGlobalNames);
         globals[element] = result;
         return result;
       }
       compiler.internalError('getName for unknown kind: ${element.kind}',
                               node: element.parseNode(compiler));
     }
   }
 
   String getLazyInitializerName(Element element) {
-    // TODO(floitsch): mangle while not conflicting with other statics.
     assert(Elements.isStaticOrTopLevelField(element));
-    return "get\$${getName(element)}";
+    return getMappedGlobalName("get\$${getName(element)}");
   }
 
   String isolatePropertiesAccess(Element element) {
     return "$ISOLATE.$ISOLATE_PROPERTIES.${getName(element)}";
   }
 
   String isolateAccess(Element element) {
     return "$CURRENT_ISOLATE.${getName(element)}";
   }
 
   String isolateBailoutAccess(Element element) {
-    return '${isolateAccess(element)}\$bailout';
+    String newName = getMappedGlobalName('${getName(element)}\$bailout');
+    return '$CURRENT_ISOLATE.$newName';
   }
 
   String isolateLazyInitializerAccess(Element element) {
     return "$CURRENT_ISOLATE.${getLazyInitializerName(element)}";
   }
 
   String operatorIs(Element element) {
+    // TODO(erikcorry): Reduce from is$x to ix when we are minifying.
     return 'is\$${getName(element)}';
   }
 
   String safeName(String name) {
     if (jsReserved.contains(name) || name.startsWith('\$')) {
       name = "\$$name";
       assert(!jsReserved.contains(name));
     }
     return name;
   }
 }