dart2js: Use an abstract Name class for names in the generated JavaScript ast.

pkg/compiler/lib/src/js_emitter/old_emitter/nsm_emitter.dart

 // Copyright (c) 2013, 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 dart2js.js_emitter;
 
 class NsmEmitter extends CodeEmitterHelper {
   final List<Selector> trivialNsmHandlers = <Selector>[];
 
   /// If this is true then we can generate the noSuchMethod handlers at startup
   /// time, instead of them being emitted as part of the Object class.
   bool get generateTrivialNsmHandlers => true;
 
   // If we need fewer than this many noSuchMethod handlers we can save space by
   // just emitting them in JS, rather than emitting the JS needed to generate
   // them at run time.
   static const VERY_FEW_NO_SUCH_METHOD_HANDLERS = 10;
 
   static const MAX_MINIFIED_LENGTH_FOR_DIFF_ENCODING = 4;
 
   void emitNoSuchMethodHandlers(AddPropertyFunction addProperty) {
 
     ClassStubGenerator generator =
         new ClassStubGenerator(compiler, namer, backend);
 
     // Keep track of the JavaScript names we've already added so we
     // do not introduce duplicates (bad for code size).
-    Map<String, Selector> addedJsNames
+    Map<jsAst.Name, Selector> addedJsNames
         = generator.computeSelectorsForNsmHandlers();
 
     // Set flag used by generateMethod helper below.  If we have very few
     // handlers we use addProperty for them all, rather than try to generate
     // them at runtime.
     bool haveVeryFewNoSuchMemberHandlers =
         (addedJsNames.length < VERY_FEW_NO_SUCH_METHOD_HANDLERS);
-    for (String jsName in addedJsNames.keys.toList()..sort()) {
+    List<jsAst.Name> names = addedJsNames.keys.toList()..sort();

[floitsch, 2015/06/22 17:43:45]

sort on a different line?

[herhut, 2015/06/23 13:26:32]

Done.

+    for (jsAst.Name jsName in names) {
       Selector selector = addedJsNames[jsName];
       String reflectionName = emitter.getReflectionName(selector, jsName);
 
       if (reflectionName != null) {
         emitter.mangledFieldNames[jsName] = reflectionName;
       }
 
       List<jsAst.Expression> argNames =
           selector.callStructure.getOrderedNamedArguments().map((String name) =>
               js.string(name)).toList();
       int type = selector.invocationMirrorKind;
       if (!haveVeryFewNoSuchMemberHandlers &&
           isTrivialNsmHandler(type, argNames, selector, jsName) &&
           reflectionName == null) {
         trivialNsmHandlers.add(selector);
       } else {
         StubMethod method =
             generator.generateStubForNoSuchMethod(jsName, selector);
         addProperty(method.name, method.code);
         if (reflectionName != null) {
           bool accessible =
               compiler.world.allFunctions.filter(selector, null).any(
               (Element e) => backend.isAccessibleByReflection(e));
-          addProperty('+$reflectionName', js(accessible ? '2' : '0'));
+          addProperty(namer.asName('+$reflectionName'),
+                      js(accessible ? '2' : '0'));
         }
       }
     }
   }
 
   // Identify the noSuchMethod handlers that are so simple that we can
   // generate them programatically.
   bool isTrivialNsmHandler(
-      int type, List argNames, Selector selector, String internalName) {
+      int type, List argNames, Selector selector, jsAst.Name internalName) {
     if (!generateTrivialNsmHandlers) return false;
     // Check for interceptor calling convention.
     if (backend.isInterceptedName(selector.name)) {

[floitsch, 2015/06/22 17:43:45]

TBH I don't see why we do special casing for the intercepted names.
Maybe Peter remembers.

[herhut, 2015/06/23 13:26:31]

This is probably an artifact. I checked the decode and I am sure it does the
right thing in both minified and non-minified mode.

       // We can handle the calling convention used by intercepted names in the
       // diff encoding, but we don't use that for non-minified code.
       if (!compiler.enableMinification) return false;
-      String shortName = namer.invocationMirrorInternalName(selector);
-      if (shortName.length > MAX_MINIFIED_LENGTH_FOR_DIFF_ENCODING) {
-        return false;
-      }
+      // TODO(herhut): We can no longer answer the below with the late bound
+      //               names. However, names longer than 4 are bad anyway.
+      // jsAst.Name shortName = namer.invocationMirrorInternalName(selector);
+      // if (shortName.length > MAX_MINIFIED_LENGTH_FOR_DIFF_ENCODING) {
+      //   return false;
+      // }
     }
     // Check for named arguments.
     if (argNames.length != 0) return false;
     // Check for unexpected name (this doesn't really happen).
-    if (internalName.startsWith(namer.getterPrefix[0])) return type == 1;
-    if (internalName.startsWith(namer.setterPrefix[0])) return type == 2;
+    if (internalName is GetterName) return type == 1;
+    if (internalName is SetterName) return type == 2;
     return type == 0;
   }
 
   /**
    * Adds (at runtime) the handlers to the Object class which catch calls to
    * methods that the object does not have.  The handlers create an invocation
    * mirror object.
    *
    * The current version only gives you the minified name when minifying (when
    * not minifying this method is not called).
@@ skipping 25 matching lines @@
    * The reason we don't encode long minified names with this method is that
    * decoding the base 88 numbers would overflow JavaScript's puny integers.
    *
    * There are some selectors that have a special calling convention (because
    * they are called with the receiver as the first argument).  They need a
    * slightly different noSuchMethod handler, so we handle these first.
    */
   List<jsAst.Statement> buildTrivialNsmHandlers() {
     List<jsAst.Statement> statements = <jsAst.Statement>[];
     if (trivialNsmHandlers.length == 0) return statements;
-    // Sort by calling convention, JS name length and by JS name.
+    // Sort by calling convention and by JS name.
     trivialNsmHandlers.sort((a, b) {
       bool aIsIntercepted = backend.isInterceptedName(a.name);
       bool bIsIntercepted = backend.isInterceptedName(b.name);
       if (aIsIntercepted != bIsIntercepted) return aIsIntercepted ? -1 : 1;
-      String aName = namer.invocationMirrorInternalName(a);
-      String bName = namer.invocationMirrorInternalName(b);
-      if (aName.length != bName.length) return aName.length - bName.length;
+      jsAst.Name aName = namer.invocationMirrorInternalName(a);
+      jsAst.Name bName = namer.invocationMirrorInternalName(b);
       return aName.compareTo(bName);

[sra1, 2015/06/23 04:47:54]

How does this work?
jsAst.Name does not implement Comparable or define compareTo.
Does the type need to be _NamerName?

For the delta table:

1. It is essential for the that the sort takes place after deferred names are
assigned.
2. short names need to precede longer names.
3. setters and getters need to be interleaved with regular names.

What we need is length-then-lexicographic ordering, not the crazy _NamerName
compareTo ordering.

You need to defer this sort until the string is generated. get the final name
and do the original comparison on the final name:

    if (aName.length != bName.length) return aName.length - bName.length;	
    return aName.compareTo(bName);

[herhut, 2015/06/23 13:26:32]

Done.

     });
 
     // Find out how many selectors there are with the special calling
     // convention.
     int firstNormalSelector = trivialNsmHandlers.length;
     for (int i = 0; i < trivialNsmHandlers.length; i++) {
       if (!backend.isInterceptedName(trivialNsmHandlers[i].name)) {
         firstNormalSelector = i;

[sra1, 2015/06/23 04:47:54]

You can find this without sorting by counting:

trivialNsmHandlers.where((e) => backend.isInterceptedName(e.name)).length;

[herhut, 2015/06/23 13:26:32]

Done.

         break;
       }
     }
 
     // Get the short names (JS names, perhaps minified).
-    Iterable<String> shorts = trivialNsmHandlers.map((selector) =>
-         namer.invocationMirrorInternalName(selector));
-    var diffEncoding = new StringBuffer();
-
-    // Treat string as a number in base 88 with digits in ASCII order from # to
-    // z.  The short name sorting is based on length, and uses ASCII order for
-    // equal length strings so this means that names are ascending.  The hash
-    // character, #, is never given as input, but we need it because it's the
-    // implicit leading zero (otherwise we could not code names with leading
-    // dollar signs).
-    int fromBase88(String x) {
-      int answer = 0;
-      for (int i = 0; i < x.length; i++) {
-        int c = x.codeUnitAt(i);
-        // No support for Unicode minified identifiers in JS.
-        assert(c >= $$ && c <= $z);
-        answer *= 88;
-        answer += c - $HASH;
-      }
-      return answer;
-    }
-
-    // Big endian encoding, A = 0, B = 1...
-    // A lower case letter terminates the number.
-    String toBase26(int x) {
-      int c = x;
-      var encodingChars = <int>[];
-      encodingChars.add($a + (c % 26));
-      while (true) {
-        c ~/= 26;
-        if (c == 0) break;
-        encodingChars.add($A + (c % 26));
-      }
-      return new String.fromCharCodes(encodingChars.reversed.toList());
-    }
+    Iterable<jsAst.Name> shorts = trivialNsmHandlers.map((selector) =>
+    namer.invocationMirrorInternalName(selector));

[floitsch, 2015/06/22 17:43:45]

indentation.

[herhut, 2015/06/23 13:26:32]

Done.

 
     bool minify = compiler.enableMinification;
     bool useDiffEncoding = minify && shorts.length > 30;
 
-    int previous = 0;
-    int nameCounter = 0;
-    for (String short in shorts) {
-      // Emit period that resets the diff base to zero when we switch to normal
-      // calling convention (this avoids the need to code negative diffs).
-      if (useDiffEncoding && nameCounter == firstNormalSelector) {
-        diffEncoding.write(".");
-        previous = 0;
-      }
-      if (short.length <= MAX_MINIFIED_LENGTH_FOR_DIFF_ENCODING &&
-          useDiffEncoding) {
-        int base63 = fromBase88(short);
-        int diff = base63 - previous;
-        previous = base63;
-        String base26Diff = toBase26(diff);
-        diffEncoding.write(base26Diff);
-      } else {
-        if (useDiffEncoding || diffEncoding.length != 0) {
-          diffEncoding.write(",");
-        }
-        diffEncoding.write(short);
-      }
-      nameCounter++;
+    jsAst.Expression diffEncoding;
+    if (useDiffEncoding) {
+      diffEncoding = new _DiffEncodedListOfNames(shorts, firstNormalSelector);
+    } else {
+      diffEncoding = jsAst.concatenateStrings(
+          jsAst.joinLiterals(shorts, jsAst.stringPart(",")),

[sra1, 2015/06/23 04:47:54]

This also needs to be deferred, since the sort order depends on the deferred
names.

I think you need several deferred expressions which use the same object to do
the processing (sorting, returning concatenated strings etc).

Plus side is that we can just return a deferred string for the short names list
too.

[herhut, 2015/06/23 13:26:31]

Done.

+          addQuotes: true);
     }
 
     // Startup code that loops over the method names and puts handlers on the
     // Object class to catch noSuchMethod invocations.
     ClassElement objectClass = compiler.objectClass;
     jsAst.Expression createInvocationMirror = backend.emitter
         .staticFunctionAccess(backend.getCreateInvocationMirror());
     if (useDiffEncoding) {
       statements.add(js.statement('''{
           var objectClassObject = processedClasses.collected[#objectClass],
@@ skipping 27 matching lines @@
               for (var remaining = nameNumber;
                    remaining > 0;
                    remaining = (remaining / 88) | 0) {
                 codes.unshift(${$HASH} + remaining % 88);
               }
               calculatedShortNames.push(
                 String.fromCharCode.apply(String, codes));
             }
             shortNames.splice.apply(shortNames, calculatedShortNames);
           }
-        }''', {'objectClass': js.string(namer.className(objectClass)),
-               'diffEncoding': js.string('$diffEncoding')}));
+        }''', {'objectClass': js.quoteName(namer.className(objectClass)),
+               'diffEncoding': diffEncoding}));
     } else {
       // No useDiffEncoding version.
       Iterable<String> longs = trivialNsmHandlers.map((selector) =>
              selector.invocationMirrorMemberName);
       statements.add(js.statement(
           'var objectClassObject = processedClasses.collected[#objectClass],'
           '    shortNames = #diffEncoding.split(",")',
-          {'objectClass': js.string(namer.className(objectClass)),
-           'diffEncoding': js.string('$diffEncoding')}));
+          {'objectClass': js.quoteName(namer.className(objectClass)),
+           'diffEncoding': diffEncoding}));
       if (!minify) {
         statements.add(js.statement('var longNames = #longs.split(",")',
                 {'longs': js.string(longs.join(','))}));
       }
       statements.add(js.statement(
           'if (objectClassObject instanceof Array)'
           '  objectClassObject = objectClassObject[1];'));
     }
 
     List<jsAst.Expression> sliceOffsetArguments =
@@ skipping 34 matching lines @@
       }''', {
           'sliceOffsetParams': sliceOffsetParams,
           'noSuchMethodName': namer.noSuchMethodName,
           'createInvocationMirror': createInvocationMirror,
           'names': minify ? 'shortNames' : 'longNames',
           'sliceOffsetArguments': sliceOffsetArguments}));
 
     return statements;
   }
 }
+
+/// When pretty printed, this node computes a diff-encoded string for the list
+/// of given names.

[floitsch, 2015/06/22 17:43:45]

Refer or copy the documentation from buildTrivialNsmHandler

[herhut, 2015/06/23 13:26:32]

Done.

+class _DiffEncodedListOfNames extends jsAst.DeferredString
+                              implements AstContainer {
+  String _cachedValue;
+  jsAst.ArrayInitializer ast;
+  int firstNormalSelector;
+
+  _DiffEncodedListOfNames(Iterable<jsAst.Name> names,
+                          this.firstNormalSelector) {
+    ast = new jsAst.ArrayInitializer(names.toList());
+  }
+
+  _computeDiffEncoding() {
+    var diffEncoding = new StringBuffer();
+
+    // Treat string as a number in base 88 with digits in ASCII order from # to
+    // z.  The short name sorting is based on length, and uses ASCII order for
+    // equal length strings so this means that names are ascending.  The hash
+    // character, #, is never given as input, but we need it because it's the
+    // implicit leading zero (otherwise we could not code names with leading
+    // dollar signs).
+    int fromBase88(String x) {
+      int answer = 0;
+      for (int i = 0; i < x.length; i++) {
+        int c = x.codeUnitAt(i);
+        // No support for Unicode minified identifiers in JS.
+        assert(c >= $$ && c <= $z);
+        answer *= 88;
+        answer += c - $HASH;
+      }
+      return answer;
+    }
+
+    // Big endian encoding, A = 0, B = 1...
+    // A lower case letter terminates the number.
+    String toBase26(int x) {
+      int c = x;
+      var encodingChars = <int>[];
+      encodingChars.add($a + (c % 26));
+      while (true) {
+        c ~/= 26;
+        if (c == 0) break;
+        encodingChars.add($A + (c % 26));
+      }
+      return new String.fromCharCodes(encodingChars.reversed.toList());
+    }
+
+    Iterable<String> shorts = ast.elements.map((jsAst.Name name) => name.name);
+
+    int previous = 0;
+    int nameCounter = 0;
+    for (String short in shorts) {
+      // Emit period that resets the diff base to zero when we switch to normal
+      // calling convention (this avoids the need to code negative diffs).
+      if (nameCounter == firstNormalSelector) {
+        diffEncoding.write('.');
+        previous = 0;
+      }
+      if (short.length <= NsmEmitter.MAX_MINIFIED_LENGTH_FOR_DIFF_ENCODING) {
+        int base63 = fromBase88(short);
+        int diff = base63 - previous;
+        previous = base63;
+        String base26Diff = toBase26(diff);
+        diffEncoding.write(base26Diff);
+      } else {
+        if (diffEncoding.length != 0) {
+          diffEncoding.write(',');
+        }
+        diffEncoding.write(short);
+      }
+      nameCounter++;
+    }
+
+    _cachedValue = diffEncoding.toString();
+  }
+
+  String get value {
+    if (_cachedValue == null) {
+      _cachedValue = _computeDiffEncoding();
+    }
+
+    return _cachedValue;
+  }
+}