dart2js: Refactoring, documentation, and a few bugfixes in Namer class.

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 7508513cf928d26777bb6a429275d172d3920e01..7dccb164081b2ba96add783f06e8ae87c365c780 100644
--- a/pkg/compiler/lib/src/js_backend/namer.dart
+++ b/pkg/compiler/lib/src/js_backend/namer.dart
@@ -6,10 +6,98 @@ part of js_backend;
 
 /**
  * Assigns JavaScript identifiers to Dart variables, class-names and members.
+ *
+ * Names are generated through three stages:
+ *
+ * 1. Original names and proposed names
+ * 2. Disambiguated names (also known as "mangled names")
+ * 3. Annotated names
+ *
+ * Original names are names taken directly from the input.
+ *
+ * Proposed names are either original names or synthesized names for input
+ * elements that do not have original names.
+ *
+ * Disambiguated names are derived from the above, but are mangled to ensure
+ * uniqueness within some namespace (e.g. as fields on the same JS object).
+ * In [MinifyNamer], disambiguated names are also minified.
+ *
+ * Annotated names are names generated from a disambiguated name. Annnotated
+ * names must be computable at runtime by prefixing/suffixing constant strings
+ * onto the disambiguated name.
+ *
+ * For example, some entity called `x` might be associated with these names:
+ *
+ *     Original name: `x`
+ *
+ *     Disambiguated name: `x1` (if something else was called `x`)
+ *
+ *     Annotated names: `x1`     (field name)
+ *                      `get$x1` (getter name)
+ *                      `set$x1` (setter name)
+ *                      `x1$2`   (if `x` is a method with 2 parameters)
+ *
+ * The [Namer] can choose the disambiguated names, and to some degree the
+ * prefix/suffix constants used to construct annotated names. It cannot choose
+ * annotated names with total freedom, for example, it cannot choose that the
+ * getter for `x1` should be called `getX` -- the annotated names are always
+ * built by concatenation.
+ *
+ * Disambiguated names must be chosen such that none of the annotated names can
+ * clash with each other. This may happen even if the disambiguated names are
+ * distinct, for example, suppose a field `x` and `get$x` exists in the input:
+ *
+ *     Original names: `x` and `get$x`
+ *
+ *     Disambiguated names: `x` and `get$x` (the two names a different)
+ *
+ *     Annotated names: `x` (field for `x`)
+ *                      `get$x` (getter for `x`)
+ *                      `get$x` (field for `get$x`)
+ *                      `get$get$x` (getter for `get$x`)
+ *
+ * The getter for `x` clashes with the field name for `get$x`, so the
+ * disambiguated names are invalid.
+ *
+ * Additionally, disambiguated names must be chosen such that all annotated
+ * names are valid JavaScript identifiers and do not coincide with a native
+ * JavaScript property such as `__proto__`.
+ *
+ * The following annotated names are generated for instance members, where
+ * <NAME> denotes the disambiguated name.
+ *
+ * 0. The disambiguated name can itself be seen as a annotated name.

[floitsch, 2015/01/30 21:03:38]

as an annotated name.

[asgerf, 2015/02/03 17:39:11]

Done.

+ *
+ * 1. Multiple annotated names exist per method name, encoding arity and named
+ *    parameters with the pattern:
+ *
+ *       <NAME>$<N>$namedParam1...$namedParam<M>
+ *
+ *    where <N> is the number of parameters (required and optional) and <M> is
+ *    the number of named parameters, and namedParam<n> are the names of the
+ *    named parameters in alphabetical order.
+ *
+ *    Note that this convention is not encapsulated in the [Namer], and is
+ *    hardcoded into other components, such as [Element] and [Selector].

[floitsch, 2015/01/30 21:03:36]

:(

[asgerf, 2015/02/03 17:39:12]

Clarified that it's just the ordering of parameters that's hardcoded this way. I
made it sound worse than it was.

[floitsch, 2015/02/04 00:10:10]

ok. that's not a big deal.

+ *
+ * 2. The getter/setter for a field, or tear-off getter for a method:
+ *
+ *        get$<NAME>
+ *        set$<NAME>
+ *
+ *    (The [getterPrefix] and [setterPrefix] are different in [MinifyNamer]).
+ *
+ * 3. The `is` and operator uses the following names:
+ *
+ *        $is<NAME>
+ *        $as<NAME>
+ *
+ * For local variables, the [Namer] only provides *proposed names*. These names
+ * must be disambiguated elsewhere.
  */
 class Namer implements ClosureNamer {
 
-  static const javaScriptKeywords = const <String>[
+  static const List<String> javaScriptKeywords = const <String>[
     // These are current keywords.
     "break", "delete", "function", "return", "typeof", "case", "do", "if",
     "switch", "var", "catch", "else", "in", "this", "void", "continue",
@@ -24,11 +112,11 @@ class Namer implements ClosureNamer {
     "long", "short", "volatile"
   ];
 
-  static const reservedPropertySymbols =
+  static const List<String> reservedPropertySymbols =
       const <String>["__proto__", "prototype", "constructor", "call"];
 
   // Symbols that we might be using in our JS snippets.
-  static const reservedGlobalSymbols = const <String>[
+  static const List<String> reservedGlobalSymbols = const <String>[
     // Section references are from Ecma-262
     // (http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-262.pdf)
 
@@ -148,7 +236,7 @@ class Namer implements ClosureNamer {
     "JavaArray", "JavaMember",
   ];
 
-  static const reservedGlobalObjectNames = const <String>[
+  static const List<String> reservedGlobalObjectNames = const <String>[
       "A",
       "B",
       "C", // Global object for *C*onstants.
@@ -177,12 +265,13 @@ class Namer implements ClosureNamer {
       "Z",
   ];
 
-  static const reservedGlobalHelperFunctions = const <String>[
+  static const List<String> reservedGlobalHelperFunctions = const <String>[
       "init",
       "Isolate",
   ];
 
-  static final userGlobalObjects = new List.from(reservedGlobalObjectNames)
+  static final List<String> userGlobalObjects =
+      new List.from(reservedGlobalObjectNames)
       ..remove('C')
       ..remove('H')
       ..remove('J')
@@ -252,10 +341,11 @@ class Namer implements ClosureNamer {
 
   final Set<String> usedGlobalNames;
   final Set<String> usedInstanceNames;
-  final Map<String, String> globalNameMap;
+  final Map<String, String> internGlobals;

[floitsch, 2015/01/30 21:03:36]

add comment.
internGlobals -> internalGlobals?

Many other fields have "Names" as suffix. Not this one...
Unify?

[asgerf, 2015/02/03 17:39:12]

Done.

   final Map<String, String> suggestedGlobalNames;

[floitsch, 2015/01/30 21:03:38]

I would group the fields:

globals
usedGlobalNames
suggestedGlobalNames

instanceMembers
usedInstanceNames
suggestedInstanceNames

Add comments to all fields that are not completely obvious.

[asgerf, 2015/02/03 17:39:12]

Grouped and renamed, to be more streamlined. I added a comment on the usedX set
for each group and hope it is clear the following maps all are part of the
mapping into that namespace.

-  final Map<String, String> instanceNameMap;
+  final Map<LibraryElement, Map<String, String>> instanceMembers;

[floitsch, 2015/01/30 21:03:37]

Add a comment explaining that `null` is used as key for public members.

[asgerf, 2015/02/03 17:39:13]

The map works differently now, see disambiguateMember.

I decided to omit declaration-site comments for these maps since they are
repetitive and verbose, and most of them are only referenced from one method, so
it should be easy to determine their use if it is not clear from the name and
grouping.

   final Map<String, String> suggestedInstanceNames;
+  final Map<Element, String> directAccessMap = <Element, String>{};

[floitsch, 2015/01/30 21:03:37]

comment.

[asgerf, 2015/02/03 17:39:12]

see above.

 
   final Map<String, String> operatorNameMap;
   final Map<String, int> popularNameCounters;
@@ -274,9 +364,9 @@ class Namer implements ClosureNamer {
         shortPrivateNameOwners = new Map<String, LibraryElement>(),
         usedGlobalNames = new Set<String>(),
         usedInstanceNames = new Set<String>(),
-        instanceNameMap = new Map<String, String>(),
+        instanceMembers = new Map<LibraryElement, Map<String, String>>(),
         operatorNameMap = new Map<String, String>(),
-        globalNameMap = new Map<String, String>(),
+        internGlobals = new Map<String, String>(),
         suggestedGlobalNames = new Map<String, String>(),
         suggestedInstanceNames = new Map<String, String>(),
         popularNameCounters = new Map<String, int>(),
@@ -313,6 +403,9 @@ class Namer implements ClosureNamer {
     }
   }
 
+  /// Disambiguated name for [constant].
+  ///
+  /// Unique within the global-member namespace.

[floitsch, 2015/01/30 21:03:37]

Is that necessary?
After all, we always put constants on the C object where they don't conflict
with anything else.

If in doubt, just leave a TODO.
See also comment in disambiguateGlobal.

[asgerf, 2015/02/03 17:39:13]

You are right, but this is how it worked to begin with. We can do more
fine-grained disambiguation of globals, but I deliberately left it this way to
limit the scope of the CL.

I left a TODO to clarfy that this is just how the implementation works atm.

[floitsch, 2015/02/04 00:10:10]

TODO is fine.

   String constantName(ConstantValue constant) {
     // In the current implementation it doesn't make sense to give names to
     // function constants since the function-implementation itself serves as
@@ -328,7 +421,7 @@ class Namer implements ClosureNamer {
     return result;
   }
 
-  // The long name is unminified and may have collisions.
+  /// Proposed name for [constant].
   String constantLongName(ConstantValue constant) {

[floitsch, 2015/01/30 21:03:37]

This looks like a function that shouldn't be called from the outside. Make it
private?

Same for all other private methods.

[asgerf, 2015/02/03 17:39:13]

The emitters currently call it for some sort of compression optimization.

I've made some other methods private, though, in particular the disambiguateXXX
methods.

[floitsch, 2015/02/04 00:10:11]

Acknowledged.

     String longName = constantLongNames[constant];
     if (longName == null) {
@@ -358,107 +451,94 @@ class Namer implements ClosureNamer {
   }
 
   /**
-   * If the [name] is not private returns [:name:]. Otherwise
-   * mangles the [name] so that each library has a unique name.
+   * If the [originalName] is not private returns [originalName]. Otherwise
+   * mangles the [originalName] so that each library has its own distinguished
+   * version of the name.
+   *
+   * Although the name is not guaranteed to be unique within any namespace,
+   * clashes are very unlikely in practice. Therefore, it can be used in cases
+   * where uniqueness is nice but not a strict requirement.
+   *
+   * The resulting name is a *proposed name* and is never minified.
    */
-  String privateName(LibraryElement library, String name) {
+  String privateName(LibraryElement library, String originalName) {
     // Public names are easy.
-    String nameString = name;
-    if (!isPrivateName(name)) return nameString;
+    if (!isPrivateName(originalName)) return originalName;
 
     // The first library asking for a short private name wins.
     LibraryElement owner =
-        shortPrivateNameOwners.putIfAbsent(nameString, () => library);
+        shortPrivateNameOwners.putIfAbsent(originalName, () => library);
 
-    if (owner == library && !nameString.contains('\$')) {
+    if (owner == library && !originalName.contains('\$')) {
       // Since the name doesn't contain $ it doesn't clash with any
       // of the private names that have the library name as the prefix.
-      return nameString;
+      return originalName;
     } else {
       // Make sure to return a private name that starts with _ so it
       // cannot clash with any public names.
-      String libraryName = getNameOfLibrary(library);
-      return '_$libraryName\$$nameString';
+      String libraryName = disambiguateGlobal(library);
+      return '_$libraryName\$${originalName}';

[floitsch, 2015/01/30 21:03:38]

Add comment why this is not unique.

[asgerf, 2015/02/03 17:39:13]

Done.

     }
   }
 
-  String instanceMethodName(FunctionElement element) {
-    // TODO(ahe): Could this be: return invocationName(new
-    // Selector.fromElement(element))?
-    String elementName = element.name;
-    String name = operatorNameToIdentifier(elementName);
-    if (name != elementName) return getMappedOperatorName(name);
-
-    LibraryElement library = element.library;
-    if (element.isGenerativeConstructorBody) {
-      name = Elements.reconstructConstructorNameSourceString(element);
-    }
-    FunctionSignature signature = element.functionSignature;
-    // We don't mangle the closure invoking function name because it
-    // is generated by string concatenation in applyFunction from
-    // js_helper.dart. To keep code size down, we potentially shorten
-    // the prefix though.
-    String methodName;
-    if (name == closureInvocationSelectorName) {
-      methodName = '$callPrefix\$${signature.parameterCount}';
-    } else {
-      methodName = '${privateName(library, name)}\$${signature.parameterCount}';
+  /// Annotated name for [method] encoding arity and named parameters.
+  String instanceMethodName(FunctionElement method) {
+    if (method.isGenerativeConstructorBody) {
+      return disambiguateDirectAccess(method,
+          () => Elements.reconstructConstructorNameSourceString(method));
     }
-    if (signature.optionalParametersAreNamed &&
-        !signature.optionalParameters.isEmpty) {
-      StringBuffer buffer = new StringBuffer();
-      signature.orderedOptionalParameters.forEach((Element element) {
-        buffer.write('\$${safeName(element.name)}');
-      });
-      methodName = '$methodName$buffer';
+    return invocationName(new Selector.fromElement(method));
+  }
+
+  /// Annotated name for a public method with the given [originalName]
+  /// and [arity] and no named parameters.

[floitsch, 2015/01/30 21:03:36]

Maybe mention that it handles operators?

[asgerf, 2015/02/03 17:39:12]

Not anymore.

+  String publicInstanceMethodNameByArity(String originalName, int arity) {
+    String opName = operatorNameToIdentifier(originalName);
+    if (opName != originalName) return disambiguateOperator(opName);
+    assert(!isPrivateName(originalName));
+    String disambiguatedName = disambiguateMember(null, originalName);
+    Selector selector = new Selector.call(originalName, null, arity);
+    return deriveMethodName(disambiguatedName, selector);
+  }
+
+  /// Returns the annotated name for a method with the given name,
+  /// encoding arity and named parameters with the pattern:
+  ///
+  ///     <NAME>$<N>$namedParam1...$namedParam<M>
+  ///
+  String deriveMethodName(String disambiguatedName, Selector selector) {
+    assert(selector.isCall);
+    // TODO(asgerf): Avoid clash when named parameters contain '$' symbols.

[floitsch, 2015/01/30 21:03:37]

Create a failing test.

[asgerf, 2015/02/03 17:39:12]

Done. And the issue has been fixed.

+    StringBuffer buffer = new StringBuffer();
+    for (String argumentName in selector.getOrderedNamedArguments()) {
+      buffer.write('\$${safeName(argumentName)}');
     }
-    if (name == closureInvocationSelectorName) return methodName;
-    return getMappedInstanceName(methodName);
-  }
-
-  String publicInstanceMethodNameByArity(String name, int arity) {
-    String newName = operatorNameToIdentifier(name);
-    if (newName != name) return getMappedOperatorName(newName);
-    assert(!isPrivateName(name));
-    // We don't mangle the closure invoking function name because it
-    // is generated by string concatenation in applyFunction from
-    // js_helper.dart. To keep code size down, we potentially shorten
-    // the prefix though.
-    if (name == closureInvocationSelectorName) return '$callPrefix\$$arity';
-
-    return getMappedInstanceName('$name\$$arity');
+    return '$disambiguatedName\$${selector.argumentCount}$buffer';
   }
 
+  /// Annotated name for the member being invoked by [selector].
   String invocationName(Selector selector) {
-    if (selector.isGetter) {
-      String proposedName = privateName(selector.library, selector.name);
-      return '$getterPrefix${getMappedInstanceName(proposedName)}';
-    } else if (selector.isSetter) {
-      String proposedName = privateName(selector.library, selector.name);
-      return '$setterPrefix${getMappedInstanceName(proposedName)}';
-    } else {
-      String name = selector.name;
-      if (selector.kind == SelectorKind.OPERATOR
-          || selector.kind == SelectorKind.INDEX) {
-        name = operatorNameToIdentifier(name);
-        assert(name != selector.name);
-        return getMappedOperatorName(name);
-      }
-      assert(name == operatorNameToIdentifier(name));
-      StringBuffer buffer = new StringBuffer();
-      for (String argumentName in selector.getOrderedNamedArguments()) {
-        buffer.write('\$${safeName(argumentName)}');
-      }
-      String suffix = '\$${selector.argumentCount}$buffer';
-      // We don't mangle the closure invoking function name because it
-      // is generated by string concatenation in applyFunction from
-      // js_helper.dart. We potentially shorten the prefix though.
-      if (selector.isClosureCall) {
-        return "$callPrefix$suffix";
-      } else {
-        String proposedName = privateName(selector.library, name);
-        return getMappedInstanceName('$proposedName$suffix');
-      }
+    LibraryElement library = selector.library;
+    switch (selector.kind) {
+      case SelectorKind.GETTER:
+        String disambiguatedName = disambiguateMember(library, selector.name);
+        return deriveGetterName(disambiguatedName);
+
+      case SelectorKind.SETTER:
+        String disambiguatedName = disambiguateMember(library, selector.name);
+        return deriveSetterName(disambiguatedName);
+
+      case SelectorKind.OPERATOR:
+      case SelectorKind.INDEX:
+        String operatorIdentifier = operatorNameToIdentifier(selector.name);
+        String disambiguatedName = disambiguateOperator(operatorIdentifier);
+        return disambiguatedName; // Operators are not annotated.
+
+      case SelectorKind.CALL:
+        String disambiguatedName = disambiguateMember(library, selector.name);
+        return deriveMethodName(disambiguatedName, selector);
+
+      default: throw 'Unexpected selector kind: ${selector.kind}';

[floitsch, 2015/01/30 21:03:37]

throw an internal error. (we tend to avoid throwing strings).

[asgerf, 2015/02/03 17:39:13]

Done.

     }
   }
 
@@ -469,13 +549,13 @@ class Namer implements ClosureNamer {
       => invocationName(selector);
 
   /**
-   * Returns name of accessor (root to getter and setter) for a static or
-   * instance field.
+   * Returns the disambiguated name for the given field, used for constructing
+   * the getter and setter names.
    */
   String fieldAccessorName(Element element) {
     return element.isInstanceMember
-        ? instanceFieldAccessorName(element)
-        : getNameOfField(element);
+        ? disambiguateMember(element.library, element.name)
+        : disambiguateGlobal(element);
   }
 
   /**
@@ -485,126 +565,233 @@ class Namer implements ClosureNamer {
   String fieldPropertyName(Element element) {
     return element.isInstanceMember
         ? instanceFieldPropertyName(element)
-        : getNameOfField(element);
+        : disambiguateGlobal(element);
   }
 
   /**
-   * Returns name of accessor (root to getter and setter) for an instance field.
+   * Returns name of the JavaScript property used to store the given type
+   * variable.

[floitsch, 2015/01/30 21:03:38]

Also mention that this is inside a class and thus at the same level as instance
members. (if I'm right).

[asgerf, 2015/02/03 17:39:13]

Done.

    */
-  String instanceFieldAccessorName(Element element) {
-    String proposedName = privateName(element.library, element.name);
-    return getMappedInstanceName(proposedName);
+  String readTypeVariableName(TypeVariableElement element) {

[floitsch, 2015/01/30 21:03:37]

Why is this prefixed "read" ?

[asgerf, 2015/02/03 17:39:13]

Don't know. Changed to typeVariableName.

+    return disambiguateDirectAccess(element, () => element.name);
   }
 
-  String readTypeVariableName(TypeVariableElement element) {
-    return '\$tv_${instanceFieldAccessorName(element)}';
+  /**
+   * Returns a JavaScript property name used to store [element] on one
+   * of the global objects.
+   *
+   * Should be used together with [globalObjectFor], which denotes the object
+   * on which the returned property name should be used.
+   */
+  String globalPropertyName(Element element) {
+    return disambiguateGlobal(element);
   }
 
   /**
-   * Returns name of the JavaScript property used to store an instance field.
+   * Returns the JavaScript property name used to store an instance field.
    */
   String instanceFieldPropertyName(Element element) {
     if (element.hasFixedBackendName) {
       return element.fixedBackendName;

[floitsch, 2015/01/30 21:03:36]

Does this potentially lead to clashes?

What if there is another member that ends up being assigned the
fixedBackendName.

If it's not possible, please comment (and/or assert).

[asgerf, 2015/02/03 17:39:12]

Turns out clashes are possible. Added custom_element_name_clash_test.dart.

I couldn't find a way to get a list of all fixed backend names, so I gave up on
just blacklisting them.

Instead, we now take extra care for property names on classes that extend native
classes. We prepend a $ in front of such names, and assert that fixed backend
names cannot contain a $.

     }
-    // If a class is used anywhere as a mixin, we must make the name unique so
-    // that it does not accidentally shadow.  Also, the mixin name must be
-    // constant over all mixins.
+
+    // If the name of the field might clash with another field,
+    // use a mangled field name to avoid potential clashes.
     ClassWorld classWorld = compiler.world;
     if (classWorld.isUsedAsMixin(element.enclosingClass) ||
         shadowingAnotherField(element)) {
-      // 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
-      // minified version of the class name, but the result is minified once
-      // again, so that is not visible in the end result.
-      String libraryName = getNameOfLibrary(element.library);
-      String className = getNameOfClass(element.enclosingClass);
-      String instanceName = privateName(element.library, element.name);
-      return getMappedInstanceName('$libraryName\$$className\$$instanceName');
+      return disambiguateDirectAccess(element,
+          () => '${element.enclosingClass.name}_${element.name}');

[floitsch, 2015/01/30 21:03:37]

Is this enough?
What if I have a mixin class A_b with field c and then another mixin class A
with field "b_c", and both have their fields shadowed by a class that mixes in A
and A_b.

[asgerf, 2015/02/03 17:39:13]

The string we're building is only a proposed name. The disambiguated name is
still guaranteed to be unique to the element we're passing in.

I've rewritten it a bit to make it more clear at call-site that the string is
only proposed.

     }
 
-    String proposedName = privateName(element.library, element.name);
-    return getMappedInstanceName(proposedName);
+    // No inheritance-related name clashes exist, so the accessor and property

[floitsch, 2015/01/30 21:03:37]

I don't understand this comment.
Do you mean: no inheritance related clashes exist, so accessors may use the same
naming algorithm as instance members?

If yes, why is that the case?

From what I can tell we can still have a subclass that overrides the field's
name with a getter or method. I'm not sure if that's an issue or not...

[asgerf, 2015/02/03 17:39:12]

Done following offline discussion.

+    // name can be shared. This generates nicer field names since otherwise
+    // one of them would have to be mangled.
+    return disambiguateMember(element.library, element.name);
   }
 
-
   bool shadowingAnotherField(Element element) {

[floitsch, 2015/01/30 21:03:37]

isShadowingASuperField ?

[asgerf, 2015/02/03 17:39:13]

Done.

     return element.enclosingClass.hasFieldShadowedBy(element);
   }
 
-  String setterName(Element element) {
+  /// Annotated name for the setter of [element].
+  String setterForElement(Element element) {
     // We dynamically create setters from the field-name. The setter name must
     // therefore be derived from the instance field-name.
-    LibraryElement library = element.library;
-    String name = getMappedInstanceName(privateName(library, element.name));
-    return '$setterPrefix$name';
+    String name = disambiguateMember(element.library, element.name);
+    return deriveSetterName(name);
   }
 
-  String setterNameFromAccessorName(String name) {
+  /// Annotated name for the setter of any member with [disambiguatedName].
+  String deriveSetterName(String disambiguatedName) {
     // We dynamically create setters from the field-name. The setter name must
     // therefore be derived from the instance field-name.
-    return '$setterPrefix$name';
+    return '$setterPrefix$disambiguatedName';
   }
 
-  String getterNameFromAccessorName(String name) {
+  /// Annotated name for the setter of any member with [disambiguatedName].
+  String deriveGetterName(String disambiguatedName) {
     // We dynamically create getters from the field-name. The getter name must
     // therefore be derived from the instance field-name.
-    return '$getterPrefix$name';
+    return '$getterPrefix$disambiguatedName';
   }
 
-  String getterName(Element element) {
+  /// Annotated name for the getter of [element].
+  String getterForElement(Element element) {

[floitsch, 2015/01/30 21:03:38]

nit: either put getter and setter next to each other, or getterForElement and
deriveGetterName (in the same order). That is:

getterForElement
setterForElement
deriveGetterName
deriveSetterName

or
getterForElement
deriveGetterName
setterForElement
deriveSetterName

[asgerf, 2015/02/03 17:39:12]

I agree. I kept it like this to avoid messing up the diff. Will reorder when
getting close to a final version.

     // We dynamically create getters from the field-name. The getter name must
     // therefore be derived from the instance field-name.
-    LibraryElement library = element.library;
-    String name = getMappedInstanceName(privateName(library, element.name));
-    return '$getterPrefix$name';
+    String name = disambiguateMember(element.library, element.name);
+    return deriveGetterName(name);
   }
 
-  String getMappedGlobalName(String proposedName, {bool ensureSafe: true}) {
-    var newName = globalNameMap[proposedName];
+  /// Property name for the getter of an instance member with [originalName]
+  /// in [library].
+  ///
+  /// [library] may be `null` if [originalName] is known to be public.
+  String getterForMember(LibraryElement library, String originalName) {
+    String disambiguatedName = disambiguateMember(library, originalName);
+    return deriveGetterName(disambiguatedName);
+  }
+
+  /// Disambiguated name for a compiler-owned global variable.
+  ///
+  /// The resulting name is unique within the global-member namespace.
+  String disambiguateInternGlobal(String name, {bool ensureSafe: true}) {

[floitsch, 2015/01/30 21:03:36]

disambiguateInternalGlobal ?
disambiguateCompilerGlobal ?
disambiguateSynthesizedGlobal ?

[floitsch, 2015/01/30 21:03:36]

Comment what "ensureSafe" means.

+    String newName = internGlobals[name];
     if (newName == null) {
-      newName = getFreshName(proposedName, usedGlobalNames,
+      newName = getFreshName(name, usedGlobalNames,
                              suggestedGlobalNames, ensureSafe: ensureSafe);
-      globalNameMap[proposedName] = newName;
+      internGlobals[name] = newName;
+    }
+    return newName;
+  }
+
+  /// Returns the property name to use for a compiler-owner global variable,
+  /// i.e. one that does not correspond to any element but is used as a utility
+  /// global by code generation.
+  ///
+  /// [name] functions as both the proposed name for the global, and as a key
+  /// identifying the global. The [name] should not contain `$` symbols, since

[floitsch, 2015/01/30 21:03:36]

Make this a "must" and assert it.

[asgerf, 2015/02/03 17:39:13]

Done.

+  /// the [Namer] uses those names internally.
+  ///
+  /// This provides an easy mechanism for avoiding a name-clash with user-space
+  /// globals, although the callers of must still take care not to accidentally

[floitsch, 2015/01/30 21:03:36]

-of-

[asgerf, 2015/02/03 17:39:12]

Done.

+  /// pass in the same [name] for two different intern globals.
+  String internGlobal(String name, {bool ensureSafe: true}) {

[floitsch, 2015/01/30 21:03:37]

internGlobal -> internalGlobal ?

[asgerf, 2015/02/03 17:39:13]

Done.

+    return disambiguateInternGlobal(name, ensureSafe: ensureSafe);
+  }
+
+  /// Returns the disambiguated name for a top-level or static element.
+  ///
+  /// The resulting name is unique within the global-member namespace.

[floitsch, 2015/01/30 21:03:37]

Same as for the constant: we might be able to only disambiguate within the
element's holder.

I know that classes need to be unique (for going from type to constructor) but
I'm not sure anything else needs to. (Unless, of course, reflection destroys
everything...)

Also note: in the new emitter we are considering removing the global objects.

[asgerf, 2015/02/03 17:39:12]

Again, probably yes. But this CL is big enough already.

[floitsch, 2015/02/04 00:10:10]

Didn't expect more than a TODO.

+  String disambiguateGlobal(Element element) {
+    element = element.declaration;
+    String newName = globals[element];
+    if (newName == null) {
+      String proposedName = _proposeNameForGlobal(element);
+      newName = getFreshName(proposedName, usedGlobalNames,
+                             suggestedGlobalNames, ensureSafe: true);
+      globals[element] = newName;
     }
     return newName;
   }
 
-  String getMappedInstanceName(String proposedName) {
-    var newName = instanceNameMap[proposedName];
+  /// Returns the disambiguated name for an instance method or field
+  /// with [originalName] in [library].
+  ///
+  /// [library] may be `null` if [originalName] is known to be public.
+  ///
+  /// This is the name used for deriving names of accessors (getters and
+  /// setters) and annotated method names (encoding arity and named parameters).
+  ///
+  /// The resulting name, and its associated annotated names, are unique within
+  /// the instance-member namespace.
+  String disambiguateMember(LibraryElement library, String originalName) {
+    if (originalName == closureInvocationSelectorName) {

[floitsch, 2015/01/30 21:03:36]

assert that library is not null if the originalName is not public.

[asgerf, 2015/02/03 17:39:11]

Done.

+      // We hardcode the disambiguated name of 'call'.
+      return callPrefix;
+    }
+    if (!isPrivateName(originalName)) {
+      library = null;
+    }
+    Map<String, String> nameMap =
+        instanceMembers.putIfAbsent(library, () => new Map<String,String>());
+    String newName = nameMap[originalName];
+    if (newName == null) {
+      String proposedName = privateName(library, originalName);
+      newName = getFreshName(proposedName,
+                             usedInstanceNames, suggestedInstanceNames,
+                             ensureSafe: true,
+                             sanitizeForAnnotations: true);
+      nameMap[originalName] = newName;
+    }
+    return newName;
+  }
+
+  /// Disambiguated name unique to [element].
+  ///
+  /// This is used as the property name for fields, type variables,
+  /// constructor bodies, and super-accessors.
+  ///
+  /// The resulting name is unique within the instance-member namespace.
+  String disambiguateDirectAccess(Element element, String proposeName()) {

[floitsch, 2015/01/30 21:03:38]

Explain how this is different from disambiguateMember.
Basically: two elements could have the same name (even private name) and still
get different disambiguated names here, whereas disambiguateMember only looks at
the name.
Maybe best explained with an example.

[asgerf, 2015/02/03 17:39:12]

I've changed it to disambiguateInternalMember. I hope this makes it clear that
it just generates names that don't clash with anything else.

+    String newName = directAccessMap[element];
     if (newName == null) {
-      newName = getFreshName(proposedName, usedInstanceNames,
-                             suggestedInstanceNames, ensureSafe: true);
-      instanceNameMap[proposedName] = newName;
+      String name = proposeName();
+      newName = getFreshName(name,
+                             usedInstanceNames, suggestedInstanceNames,
+                             ensureSafe: true,
+                             sanitizeForAnnotations: true);
+      directAccessMap[element] = newName;
     }
     return newName;
   }
 
-  String getMappedOperatorName(String proposedName) {
-    var newName = operatorNameMap[proposedName];
+  /// Disambiguated name for the given operator.
+  ///
+  /// [operatorIdentifier] must be the operator's identifier, e.g.
+  /// `$add` and not `+`.
+  ///
+  /// The resulting name is unique within the instance-member namespace.
+  String disambiguateOperator(String operatorIdentifier) {
+    String newName = operatorNameMap[operatorIdentifier];
     if (newName == null) {
-      newName = getFreshName(proposedName, usedInstanceNames,
+      newName = getFreshName(operatorIdentifier, usedInstanceNames,
                              suggestedInstanceNames, ensureSafe: false);
-      operatorNameMap[proposedName] = newName;
+      operatorNameMap[operatorIdentifier] = newName;
     }
     return newName;
   }
 
+  /// Returns an unused name.
+  ///
+  /// [proposedName] must be a valid JavaScript identifier.
+  ///
+  /// If [ensureSafe] is `false`, then [proposedName] must not be a keyword.
+  ///
+  /// If [sanitizeForAnnotations] is `true`, then the result is guaranteed not
+  /// to have the form of an annotated name.
+  ///
+  /// Note that [MinifyNamer] overrides this method with one that produces
+  /// minified names.
   String getFreshName(String proposedName,
                       Set<String> usedNames,
                       Map<String, String> suggestedNames,
-                      {bool ensureSafe: true}) {
-    var candidate;
+                      {bool ensureSafe: true,
+                       bool sanitizeForAnnotations: false}) {
+    if (sanitizeForAnnotations) {
+      proposedName = _sanitizeForAnnotations(proposedName);
+    }
     if (ensureSafe) {
       proposedName = safeName(proposedName);
     }
     assert(!jsReserved.contains(proposedName));
+    String candidate;
     if (!usedNames.contains(proposedName)) {
       candidate = proposedName;
     } else {
-      var counter = popularNameCounters[proposedName];
-      var i = counter == null ? 0 : counter;
+      int counter = popularNameCounters[proposedName];
+      int i = counter == null ? 0 : counter;

[floitsch, 2015/01/30 21:03:36]

(counter == null)

[asgerf, 2015/02/03 17:39:11]

Done.

       while (usedNames.contains("$proposedName$i")) {
         i++;
       }
@@ -615,15 +802,46 @@ class Namer implements ClosureNamer {
     return candidate;
   }
 
+  /// Returns a variant of [name] that cannot clash with the annotated
+  /// version of another name, that is, the resulting name can never be returned
+  /// by [deriveGetterName], [deriveSetterName], [deriveMethodName],
+  /// [operatorIs], or [substitutionName].
+  ///
+  /// For example, a name `get$x` would be converted to `$get$x` to ensure it
+  /// cannot clash with the getter for `x`.

[floitsch, 2015/01/30 21:03:37]

Add '_isAnnotated' method and assert that every annotated method satisfies it,
and the result of this function doesn't.

You could then also add:
String input = name;
...
assert(_isAnnotated(input) || input == name);
return name;

This would make sure that we don't strip too much.

[asgerf, 2015/02/03 17:39:13]

I don't think it's worth it anymore, since it would now just be a duplicate of
the method body, which is now just four checks.

+  ///
+  /// We don't want to register all potential annotated names in
+  /// [usedInstanceNames] (there are too many), so we use this step to avoid
+  /// clashes between annotated and unannotated names.
+  String _sanitizeForAnnotations(String name) {
+    // Ensure name does not contain a $ followed by a digit, since this could
+    // clash with the suffix we append on method names.
+    for (int i = name.indexOf(r'$'); i != -1; i = name.indexOf(r'$', i+1)) {
+      if (i + 1 < name.length && isDigit(name.codeUnitAt(i+1))) {
+        // Strip off everything after the $ to be safe.
+        name = name.substring(0, i + 1);
+        break;
+      }
+    }
+    // Ensure name does not clash with a getter or setter of another name, or
+    // one of the other special names that start with `$`, such as `$is`.
+    if (name.startsWith(r'$') ||
+        name.startsWith(getterPrefix) ||
+        name.startsWith(setterPrefix)) {
+      name = '\$$name';
+    }
+    return name;
+  }
+
   String getClosureVariableName(String name, int id) {
     return "${name}_$id";
   }
 
   /**
-   * Returns a preferred JS-id for the given top-level or static element.
+   * Returns a proposed name for the given top-level or static element.
    * The returned id is guaranteed to be a valid JS-id.
    */
-  String _computeGuess(Element element) {
+  String _proposeNameForGlobal(Element element) {
     assert(!element.isInstanceMember);
     String name;
     if (element.isGenerativeConstructor) {
@@ -695,17 +913,24 @@ class Namer implements ClosureNamer {
     return names.join();
   }
 
-  String getInterceptorName(Element element, Iterable<ClassElement> classes) {
+  /// Property name used for `getInterceptor` or one of its specializations.

[floitsch, 2015/01/30 21:03:37]

as written in another file:
"nameOfGetInterceptorFunctionFor" (or similar).

[asgerf, 2015/02/03 17:39:11]

Done.

+  String getInterceptorName(Iterable<ClassElement> classes) {
+    FunctionElement getInterceptor = backend.getInterceptorMethod;
     if (classes.contains(backend.jsInterceptorClass)) {
       // If the base Interceptor class is in the set of intercepted classes, we
       // need to go through the generic getInterceptorMethod, since any subclass
       // of the base Interceptor could match.
-      return getNameOfInstanceMember(element);
+      // The unspecialized getInterceptor method can also be accessed through
+      // its element, so we treat this as a user-space global instead of an
+      // intern global.
+      return disambiguateGlobal(getInterceptor);
     }
     String suffix = getInterceptorSuffix(classes);
-    return getMappedGlobalName("${element.name}\$$suffix");
+    return disambiguateInternGlobal("${getInterceptor.name}\$$suffix");
   }
 
+  /// Property name used for the one-shot interceptor method for the given
+  /// [selector] and return-type specialization.
   String getOneShotInterceptorName(Selector selector,
                                    Iterable<ClassElement> classes) {
     // The one-shot name is a global name derived from the invocation name.  To
@@ -718,113 +943,67 @@ class Namer implements ClosureNamer {
       // If the base Interceptor class is in the set of intercepted classes,
       // this is the most general specialization which uses the generic
       // getInterceptor method.  To keep the name short, we add '$' only to
-      // distinguish from global getters or setters; operators and methods can't
-      // clash.
+      // distinguish from other intern globals that don't contain '$' symbols.
       // TODO(sra): Find a way to get the simple name when Object is not in the
       // set of classes for most general variant, e.g. "$lt$n" could be "$lt".
       if (selector.isGetter || selector.isSetter) root = '$root\$';

[floitsch, 2015/01/30 21:03:37]

Why does a getter and setter need the "$" ?
The old comment explained the reasoning, although it didn't make it clear why
operators and methods couldn't clash.
I would have interpreted this as 'all other internal globals, except getters and
setters, contain a "$"', but that doesn't seem to be true.

[asgerf, 2015/02/03 17:39:12]

Rephrased comment.

-      return getMappedGlobalName(root, ensureSafe: false);
+      return disambiguateInternGlobal(root, ensureSafe: false);
     } else {
       String suffix = getInterceptorSuffix(classes);
-      return getMappedGlobalName("$root\$$suffix", ensureSafe: false);
+      return disambiguateInternGlobal("$root\$$suffix", ensureSafe: false);
     }
   }
 
-  /// Returns the runtime name for [element].  The result is not safe as an id.
-  String getRuntimeTypeName(Element element) {
+  /// Returns the runtime name for [element].
+  ///
+  /// This name is used as the basis for deriving `is` and `as` property names
+  /// for the given type.
+  ///
+  /// The result is not always safe as a property name unless prefixing
+  /// [operatorIsPrefix] or [operatorAsPrefix]. If this is a function type,
+  /// then by convention, an underscore must also separate [operatorIsPrefix]
+  /// from the type name.

[floitsch, 2015/01/30 21:03:37]

Yeah for weird arbitrary rules...

[asgerf, 2015/02/03 17:39:12]

Still better than undocumented arbitrary rules...

+  String getRuntimeTypeName(TypeDeclarationElement element) {
     if (element == null) return 'dynamic';
-    return getNameForRti(element);
-  }
-
-  /**
-   * 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.
-   *
-   * For accessing statics consider calling [elementAccess] instead.
-   */
-  // TODO(ahe): This is an internal method to the Namer (and its subclasses)
-  // and should not be call from outside.
-  String getNameX(Element element) {
-    if (element.isInstanceMember) {
-      if (element.kind == ElementKind.GENERATIVE_CONSTRUCTOR_BODY
-          || element.kind == ElementKind.FUNCTION) {
-        return instanceMethodName(element);
-      } else if (element.kind == ElementKind.GETTER) {
-        return getterName(element);
-      } else if (element.kind == ElementKind.SETTER) {
-        return setterName(element);
-      } else if (element.kind == ElementKind.FIELD) {
-        compiler.internalError(element,
-            'Use instanceFieldPropertyName or instanceFieldAccessorName.');
-        return null;
-      } else {
-        compiler.internalError(element,
-            'getName for bad kind: ${element.kind}.');
-        return null;
-      }
-    } else {
-      // Use declaration element to ensure invariant on [globals].
-      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 (kind == ElementKind.VARIABLE ||
-          kind == ElementKind.PARAMETER) {
-        // The name is not guaranteed to be unique.
-        return safeName(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) {
-        bool fixedName = false;
-        if (Elements.isInstanceField(element)) {
-          fixedName = element.hasFixedBackendName;
-        }
-        String result = fixedName
-            ? guess
-            : getFreshName(guess, usedGlobalNames, suggestedGlobalNames,
-                           ensureSafe: true);
-        globals[element] = result;
-        return result;
-      }
-      compiler.internalError(element,
-          'getName for unknown kind: ${element.kind}.');
-      return null;
-    }
-  }
-
-  String getNameForRti(Element element) => getNameX(element);
-
-  String getNameOfLibrary(LibraryElement library) => getNameX(library);
-
-  String getNameOfClass(ClassElement cls) => getNameX(cls);
-
-  String getNameOfField(VariableElement field) => getNameX(field);
-
-  // TODO(ahe): Remove this method. Use get getNameOfMember instead.
-  String getNameOfInstanceMember(Element member) => getNameX(member);
-
+    // The returned name affects both the global and instance member namespaces:
+    //
+    // - If given a class, this must coincide with the class name, which
+    //   is also the GLOBAL property name of its constructor.
+    //
+    // - The result is used to derive `$isX` and `$asX` names, which are used
+    //   as INSTANCE property names.
+    //
+    // 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 a '$' character.
+    return disambiguateGlobal(element);
+  }
+
+  /// Returns the disambiguated name of [class_].
+  ///
+  /// This is both the *runtime type* of the class (see [getRuntimeTypeName])
+  /// and a global property name in which to store its JS constructor.
+  String getNameOfClass(ClassElement class_) => disambiguateGlobal(class_);
+
+  /// Property name on which [member] can be accessed directly,
+  /// without clashing with another JS property name.

[floitsch, 2015/01/30 21:03:36]

Might help to give a small example when this is necessary:

foo(x, [y]) has a stub method foo$1(x) that redirects to this.foo$2(x, null);

However, if the method is overridden by another foo, method we must make sure
that we reach the right `foo$2` method. By giving it an alias we can redirect to
`aliased$2(x, null)`.

[asgerf, 2015/02/03 17:39:13]

Done, although I don't understand your example, so I chose a different one.

[floitsch, 2015/02/04 00:10:11]

Interesting. I didn't know that we added the alias for such simple cases (since
we could just grab the prototype and use `B.prototype.foo.call(this, ...)`.

My example is more complex (and was the reason we introduced aliases, since the
prototype approach wasn't working any more):

class A {
  foo([x]);
}

class B extends A {
  foo([x]) => super.foo();
}

Assume that A was compiled to:
A.prototype.foo$0 = function() { return this.foo$1(null); };
A.prototype.foo$1 = function(x) { ... };

with the prototype approach we would compile B.foo as follows:

B.prototype.foo$1 = function(x) { return A.prototype.foo$0.call(this); }

However, A.foo$0 redirects to this.foo$1 which would then again be B.foo$1. To
solve this issue one could either change foo$0 to:
A.prototype.foo$0 = function() { return A.prototype.foo$1.call(this, null); },
or simply give foo$1 another name.

Note: no need to add this as comment. Just FYI.

   String getNameOfAliasedSuperMember(Element member) {
-    ClassElement superClass = member.enclosingClass;
-    String className = getNameOfClass(superClass);
-    String memberName = getNameOfMember(member);
-    String proposal = "$superPrefix$className\$$memberName";
-    // TODO(herhut): Use field naming constraints (unique wrt. inheritance).
-    return getMappedInstanceName(proposal);
+    assert(!member.isField); // Fields do not need super aliases.
+    return disambiguateDirectAccess(member,
+        () => '\$super\$${member.enclosingClass.name}\$${member.name}');
   }
 
-  String getNameOfMember(Element member) => getNameX(member);
-
-  String getNameOfGlobalField(VariableElement field) => getNameX(field);
+  /// Property name in which to store the given static or instance [method].
+  /// For instance methods, this includes the suffix encoding arity and named
+  /// parameters.
+  ///
+  /// The name is not necessarily unique to [method], since a static method
+  /// may share its name with an instance method.

[floitsch, 2015/01/30 21:03:37]

But is it unique within their respective categories?
Is the returned name a proposed name or a disambiguated one?

(it looks like it's the disambiguated one, but add comment).

[asgerf, 2015/02/03 17:39:12]

Ideally, the users of Namer shouldn't have to know about the different kinds of
names (proposed, disambiguated, annotated).

This is why it just says "property name". It's what the caller should care
about.

If we were to go completely formal on this, I'd say some annotated names are
just a disambiguated name with an empty annotation. This is one of those names.
But I think writing this would cause more confusion than clarity.

+  String getNameOfMethod(Element method) {

[floitsch, 2015/01/30 21:03:38]

Why is this prefixed with "get"?

Most of the other methods in this class have: "methodName" or similar naming
conventions. Some drop the "name" suffix (although it feels inconsistent).

[asgerf, 2015/02/03 17:39:13]

I agree. I've changed it to "methodName".

+    return method.isInstanceMember
+        ? instanceMethodName(method)
+        : globalPropertyName(method);
+  }
 
   /// Returns true if [element] is stored on current isolate ('$').  We intend
   /// to store only mutable static state in [currentIsolate], constants are
@@ -861,19 +1040,19 @@ class Namer implements ClosureNamer {
 
   String getLazyInitializerName(Element element) {
     assert(Elements.isStaticOrTopLevelField(element));
-    return getMappedGlobalName("$getterPrefix${getNameX(element)}");
+    return disambiguateInternGlobal("$getterPrefix${globalPropertyName(element)}");

[floitsch, 2015/01/30 21:03:36]

long line.

[asgerf, 2015/02/03 17:39:12]

Done.

   }
 
   String getStaticClosureName(Element element) {
     assert(Elements.isStaticOrTopLevelFunction(element));
-    return getMappedGlobalName("${getNameX(element)}\$closure");
+    return disambiguateInternGlobal("${globalPropertyName(element)}\$closure");
   }
 
   // This name is used as part of the name of a TypeConstant
   String uniqueNameForTypeConstantElement(Element element) {
     // TODO(sra): If we replace the period with an identifier character,
     // TypeConstants will have better names in unminified code.
-    return "${globalObjectFor(element)}.${getNameX(element)}";
+    return "${globalObjectFor(element)}.${globalPropertyName(element)}";
   }
 
   String globalObjectForConstant(ConstantValue constant) => 'C';
@@ -919,7 +1098,7 @@ class Namer implements ClosureNamer {
     return operatorIs(type.element);
   }
 
-  String operatorIs(Element element) {
+  String operatorIs(ClassElement element) {
     // TODO(erikcorry): Reduce from $isx to ix when we are minifying.
     return '${operatorIsPrefix}${getRuntimeTypeName(element)}';
   }
@@ -929,7 +1108,7 @@ class Namer implements ClosureNamer {
    * and also ensures it won't clash with other identifiers.

[floitsch, 2015/01/30 21:03:38]

with other reserved identifiers (like "__proto__").

[asgerf, 2015/02/03 17:39:13]

Actually I think that was a stray comment from when _safeName also was the guard
against clashes with annotated names.

    */
   String _safeName(String name, Set<String> reserved) {
-    if (reserved.contains(name) || name.startsWith(r'$')) {
+    if (reserved.contains(name)) {
       name = '\$$name';
     }
     assert(!reserved.contains(name));
@@ -940,7 +1119,7 @@ class Namer implements ClosureNamer {
     // TODO(ahe): Creating a string here is unfortunate. It is slow (due to

[floitsch, 2015/01/30 21:03:38]

I don't know what Peter means.
The cost in string concatenation for substitutions should be noise.
I don't see what "segmentation of '$'" means.

Unless you think there is a reason we should keep the TODO, please delete it.

[asgerf, 2015/02/03 17:39:12]

Done.

     // string concatenation in the implementation), and may prevent
     // segmentation of '$'.
-    return '${operatorAsPrefix}${getNameForRti(element)}';
+    return '${operatorAsPrefix}${getRuntimeTypeName(element)}';
   }
 
   String safeName(String name) => _safeName(name, jsReserved);