Use entities for globals in Namer.

pkg/compiler/lib/src/js_backend/namer.dart

Index: pkg/compiler/lib/src/js_backend/namer.dart
diff --git a/pkg/compiler/lib/src/js_backend/namer.dart b/pkg/compiler/lib/src/js_backend/namer.dart
index 43bb6804b3e4934847540521360696d2f0f7791e..119cccfb4ee5031505f7f24636000c2c5578ff1c 100644
--- a/pkg/compiler/lib/src/js_backend/namer.dart
+++ b/pkg/compiler/lib/src/js_backend/namer.dart
@@ -514,8 +514,7 @@ class Namer {
   /// (see [globalObjectFor]), we currently use a single namespace for all these
   /// names.
   final NamingScope globalScope = new NamingScope();
-  final Map<Element, jsAst.Name> userGlobals =
-      new HashMap<Element, jsAst.Name>();
+  final Map<Entity, jsAst.Name> userGlobals = new HashMap<Entity, jsAst.Name>();
   final Map<String, jsAst.Name> internalGlobals =
       new HashMap<String, jsAst.Name>();
 
@@ -867,7 +866,7 @@ class Namer {
   jsAst.Name fieldAccessorName(FieldElement element) {
     return element.isInstanceMember
         ? _disambiguateMember(element.memberName)
-        : _disambiguateGlobal(element);
+        : _disambiguateGlobalMember(element);
   }
 
   /**
@@ -877,9 +876,18 @@ class Namer {
   jsAst.Name fieldPropertyName(FieldElement element) {
     return element.isInstanceMember
         ? instanceFieldPropertyName(element)
-        : _disambiguateGlobal(element);
+        : _disambiguateGlobalMember(element);
   }
 
+  jsAst.Name globalPropertyNameForMember(MemberEntity element) =>
+      _disambiguateGlobalMember(element);
+
+  jsAst.Name globalPropertyNameForClass(ClassEntity element) =>
+      _disambiguateGlobalType(element);
+
+  jsAst.Name globalPropertyNameForType(TypeDeclarationElement element) =>
+      _disambiguateGlobalType(element);
+
   /**
    * Returns a JavaScript property name used to store [element] on one
    * of the global objects.
@@ -887,9 +895,9 @@ class Namer {
    * Should be used together with [globalObjectFor], which denotes the object
    * on which the returned property name should be used.
    */
-  jsAst.Name globalPropertyName(Element element) {
+  /*jsAst.Name _globalPropertyName(Element element) {

[Siggi Cherem (dart-lang), 2017/05/30 21:37:20]

delete?

[Johnni Winther, 2017/05/31 07:57:06]

Done.

     return _disambiguateGlobal(element);
-  }
+  }*/
 
   /**
    * Returns the JavaScript property name used to store an instance field.
@@ -1023,16 +1031,28 @@ class Namer {
     });
   }
 
+  jsAst.Name _disambiguateGlobalMember(MemberEntity element) {
+    return _disambiguateGlobal(element, _proposeNameForMember);
+  }
+
+  jsAst.Name _disambiguateGlobalType(Entity element) {
+    return _disambiguateGlobal(element, _proposeNameForType);
+  }
+
   /// Returns the disambiguated name for a top-level or static element.
   ///
   /// The resulting name is unique within the global-member namespace.
-  jsAst.Name _disambiguateGlobal(Element element) {
+  jsAst.Name _disambiguateGlobal(
+      Entity element, String proposeName(Entity element)) {
     // TODO(asgerf): We can reuse more short names if we disambiguate with
     // a separate namespace for each of the global holder objects.
-    element = element.declaration;
+    if (element is Element) {

[Siggi Cherem (dart-lang), 2017/05/30 21:37:20]

TODO to ensure we work with declarations all the time?

[Johnni Winther, 2017/05/31 07:57:06]

J/K-Elements are only declaration elements. Added a comment.

+      Element e = element;
+      element = e.declaration;
+    }
     jsAst.Name newName = userGlobals[element];
     if (newName == null) {
-      String proposedName = _proposeNameForGlobal(element);
+      String proposedName = proposeName(element);
       newName = getFreshName(globalScope, proposedName);
       userGlobals[element] = newName;
     }
@@ -1247,33 +1267,40 @@ class Namer {
     return name;
   }
 
-  /**
-   * Returns a proposed name for the given top-level or static element.
-   * The returned id is guaranteed to be a valid JavaScript identifier.
-   */
-  String _proposeNameForGlobal(Element element) {
-    assert(!element.isInstanceMember);
-    if (element.isGenerativeConstructor) {
-      return '${element.enclosingClass.name}\$${element.name}';
+  /// Returns a proposed name for the given typedef or class [element].
+  /// The returned id is guaranteed to be a valid JavaScript identifier.
+  String _proposeNameForType(Entity element) {
+    return element.name.replaceAll('+', '_');
+  }
+
+  /// Returns a proposed name for the given top-level or static member
+  /// [element]. The returned id is guaranteed to be a valid JavaScript
+  /// identifier.
+  String _proposeNameForMember(MemberEntity element) {
+    if (element.isConstructor) {
+      return _proposeNameForConstructor(element);
+    } else if (element.enclosingClass != null) {
+      ClassEntity enclosingClass = element.enclosingClass;
+      return '${enclosingClass.name}_${element.name}';
     }
-    if (element.isFactoryConstructor) {
+    return element.name.replaceAll('+', '_');
+  }
+
+  String _proposeNameForConstructor(ConstructorEntity element) {
+    String className = element.enclosingClass.name;
+    if (element.isGenerativeConstructor) {
+      return '${className}\$${element.name}';
+    } else {
       // TODO(johnniwinther): Change factory name encoding as to not include
       // the class-name twice.
-      String className = element.enclosingClass.name;
-      return '${className}_${Elements.reconstructConstructorName(element)}';
-    }
-    if (Elements.isStaticOrTopLevel(element)) {
-      if (element.isClassMember) {
-        ClassElement enclosingClass = element.enclosingClass;
-        return '${enclosingClass.name}_${element.name}';
+      String constructorName;
+      if (element.name == '') {
+        constructorName = className;
+      } else {
+        constructorName = '${className}\$${element.name}';
       }
-      return element.name.replaceAll('+', '_');
+      return '${className}_${constructorName}';
     }
-    if (element.isLibrary) {
-      LibraryElement library = element;
-      return _proposeNameForLibrary(library);
-    }
-    return element.name;
   }
 
   /**
@@ -1356,7 +1383,7 @@ class Namer {
       // The unspecialized getInterceptor method can also be accessed through
       // its element, so we treat this as a user-space global instead of an
       // internal global.
-      return _disambiguateGlobal(getInterceptor);
+      return _disambiguateGlobalMember(getInterceptor);
     }
     String suffix = suffixForGetInterceptor(classes);
     return _disambiguateInternalGlobal("${getInterceptor.name}\$$suffix");
@@ -1407,14 +1434,14 @@ class Namer {
     // To prevent clashes in both namespaces at once, we disambiguate the name
     // as a global here, and in [_sanitizeForAnnotations] we ensure that
     // ordinary instance members cannot start with `$is` or `$as`.
-    return _disambiguateGlobal(element);
+    return _disambiguateGlobalType(element);
   }
 
   /// Returns the disambiguated name of [class_].
   ///
   /// This is both the *runtime type* of the class (see [runtimeTypeName])
   /// and a global property name in which to store its JS constructor.
-  jsAst.Name className(ClassElement class_) => _disambiguateGlobal(class_);
+  jsAst.Name className(ClassElement class_) => _disambiguateGlobalType(class_);
 
   /// Property name on which [member] can be accessed directly,
   /// without clashing with another JS property name.
@@ -1450,10 +1477,10 @@ class Namer {
   ///
   /// The name is not necessarily unique to [method], since a static method
   /// may share its name with an instance method.
-  jsAst.Name methodPropertyName(Element method) {
+  jsAst.Name methodPropertyName(MethodElement method) {
     return method.isInstanceMember
         ? instanceMethodName(method)
-        : globalPropertyName(method);
+        : globalPropertyNameForMember(method);
   }
 
   /// Returns true if [element] is stored in the static state holder
@@ -1461,40 +1488,57 @@ class Namer {
   /// there, whereas constants are stored in 'C'. Functions, accessors,
   /// classes, etc. are stored in one of the other objects in
   /// [reservedGlobalObjectNames].
-  bool _isPropertyOfStaticStateHolder(Element element) {
+  bool _isPropertyOfStaticStateHolder(MemberEntity element) {
     // TODO(ahe): Make sure this method's documentation is always true and
     // remove the word "intend".
-    return
-        // TODO(ahe): Re-write these tests to be positive (so it only returns
-        // true for static/top-level mutable fields). Right now, a number of
-        // other elements, such as bound closures also live in
-        // [staticStateHolder].
-        !element.isAccessor &&
-            !element.isClass &&
-            !element.isTypedef &&
-            !element.isConstructor &&
-            !element.isFunction &&
-            !element.isLibrary;
+    if (element is MemberElement) {
+      // TODO(johnniwinther): Clean up this method to have a single semantics on
+      // entities.
+      return
+          // TODO(ahe): Re-write these tests to be positive (so it only returns
+          // true for static/top-level mutable fields). Right now, a number of
+          // other elements, such as bound closures also live in
+          // [staticStateHolder].
+          !element.isAccessor &&
+              !element.isClass &&
+              !element.isTypedef &&
+              !element.isConstructor &&
+              !element.isFunction &&
+              !element.isLibrary;
+    }
+    return element.isField;
   }
 
   /// Returns [staticStateHolder] or one of [reservedGlobalObjectNames].
   // TODO(johnniwinther): Verify that the implementation can be changed to
   // `globalObjectForLibrary(element.library)`.
   String globalObjectForMethod(MethodElement element) =>
-      globalObjectFor(element);
+      globalObjectForMember(element);
 
   /// Returns [staticStateHolder] or one of [reservedGlobalObjectNames].
-  String globalObjectFor(Element element) {
+  String globalObjectForMember(MemberEntity element) {
     if (_isPropertyOfStaticStateHolder(element)) return staticStateHolder;
     return globalObjectForLibrary(element.library);
   }
 
+  String globalObjectForClass(ClassEntity element) {
+    return globalObjectForLibrary(element.library);
+  }
+
+  String globalObjectForType(Entity element) {
+    if (element is TypedefElement) {
+      return globalObjectForLibrary(element.library);
+    }
+    return globalObjectForClass(element);
+  }
+
   /// Returns the [reservedGlobalObjectNames] for [library].
-  String globalObjectForLibrary(LibraryElement library) {
+  String globalObjectForLibrary(LibraryEntity library) {
     if (library == _commonElements.interceptorsLibrary) return 'J';
-    if (library.isInternalLibrary) return 'H';
-    if (library.isPlatformLibrary) {
-      if ('${library.canonicalUri}' == 'dart:html') return 'W';
+    Uri uri = library.canonicalUri;
+    if (uri.scheme == 'dart') {
+      if (uri.path == 'html') return 'W';
+      if (uri.path.startsWith('_')) return 'H';
       return 'P';
     }
     return userGlobalObjects[library.name.hashCode % userGlobalObjects.length];
@@ -1505,9 +1549,9 @@ class Namer {
     return new CompoundName([_literalLazyGetterPrefix, name]);
   }
 
-  jsAst.Name lazyInitializerName(Element element) {
-    assert(Elements.isStaticOrTopLevelField(element));
-    jsAst.Name name = _disambiguateGlobal(element);
+  jsAst.Name lazyInitializerName(FieldEntity element) {
+    assert(element.isTopLevel || element.isStatic);
+    jsAst.Name name = _disambiguateGlobalMember(element);
     // These are not real dart getters, so do not use GetterName;
     return deriveLazyInitializerName(name);
   }