dart2js OldEmitter: Change to named holes in js output strings plus some extra clean-ups.

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
@@ skipping 63 matching lines @@
       String internalName = namer.invocationMirrorInternalName(selector);
       String reflectionName = emitter.getReflectionName(selector, internalName);
       if (!haveVeryFewNoSuchMemberHandlers &&
           isTrivialNsmHandler(type, argNames, selector, internalName) &&
           reflectionName == null) {
         trivialNsmHandlers.add(selector);
         return null;
       }
 
       assert(backend.isInterceptedName(Compiler.NO_SUCH_METHOD));
-      jsAst.Expression expression = js('this.#(this, #(#, #, #, #, #))', [
-          noSuchMethodName,
-          backend.emitter.staticFunctionAccess(
-              backend.getCreateInvocationMirror()),
-          js.string(compiler.enableMinification ?
-              internalName : methodName),
-          js.string(internalName),
-          js.number(type),
-          new jsAst.ArrayInitializer(parameterNames.map(js).toList()),
-          new jsAst.ArrayInitializer(argNames)]);
+      jsAst.Expression expression =
+          js('''this.#noSuchMethodName(this,
+                    #createInvocationMirror(#methodName,
+                                            #internalName,
+                                            #type,
+                                            #arguments,
+                                            #namedArguments))''',
+             {'noSuchMethodName': noSuchMethodName,
+              'createInvocationMirror':
+                  backend.emitter.staticFunctionAccess(
+                      backend.getCreateInvocationMirror()),
+              'methodName':
+                  js.string(compiler.enableMinification
+                      ? internalName : methodName),
+              'internalName': js.string(internalName),
+              'type': js.number(type),
+              'arguments':
+                  new jsAst.ArrayInitializer(parameterNames.map(js).toList()),
+              'namedArguments': new jsAst.ArrayInitializer(argNames)});
 
       if (backend.isInterceptedName(selector.name)) {
         return js(r'function($receiver, #) { return # }',
                   [parameterNames, expression]);
       } else {
         return js(r'function(#) { return # }', [parameterNames, expression]);
       }
     }
 
     for (String jsName in addedJsNames.keys.toList()..sort()) {
@@ skipping 166 matching lines @@
     // 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());
     String noSuchMethodName = namer.publicInstanceMethodNameByArity(
         Compiler.NO_SUCH_METHOD, Compiler.NO_SUCH_METHOD_ARG_COUNT);
     var type = 0;
     if (useDiffEncoding) {
       statements.add(js.statement('''{
           var objectClassObject =

[floitsch, 2014/12/29 18:09:12]

fits on one line?

[zarah, 2014/12/30 10:29:26]

Done.

-                  collectedClasses[#],    // # is name of class Object.
-              shortNames = #.split(","),  // # is diffEncoding.
+                  collectedClasses[#objectClass],
+              shortNames = #diffEncoding.split(","),
               nameNumber = 0,
               diffEncodedString = shortNames[0],
               calculatedShortNames = [0, 1];    // 0, 1 are args for splice.
           // If we are loading a deferred library the object class will not be in
           // the collectedClasses so objectClassObject is undefined, and we skip
           // setting up the names.
 
           if (objectClassObject) {
             if (objectClassObject instanceof Array)
               objectClassObject = objectClassObject[1];
@@ skipping 16 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);
           }
-        }''', [
-            js.string(namer.getNameOfClass(objectClass)),
-            js.string('$diffEncoding')]));
+        }''', {'objectClass': js.string(namer.getNameOfClass(objectClass)),
+               'diffEncoding': js.string('$diffEncoding')}));
     } else {
       // No useDiffEncoding version.
       Iterable<String> longs = trivialNsmHandlers.map((selector) =>
              selector.invocationMirrorMemberName);
       statements.add(js.statement(
-          'var objectClassObject = collectedClasses[#],'
-          '    shortNames = #.split(",")', [
-              js.string(namer.getNameOfClass(objectClass)),
-              js.string('$diffEncoding')]));
+          'var objectClassObject = collectedClasses[#objectClass],'
+          '    shortNames = #diffEncoding.split(",")', {

[floitsch, 2014/12/29 18:09:12]

move the "{" to the next line.
At least for the code review it's easier to see that it isn't part of the JS
code then.

[zarah, 2014/12/30 10:29:26]

Done.

+              'objectClass': js.string(namer.getNameOfClass(objectClass)),
+              'diffEncoding': js.string('$diffEncoding')}));
       if (!minify) {
-        statements.add(js.statement('var longNames = #.split(",")',
-                js.string(longs.join(','))));
+        statements.add(js.statement('var longNames = #longs.split(",")',

[floitsch, 2014/12/29 18:09:12]

ok. But I didn't find it hard to read.

[zarah, 2014/12/30 10:29:26]

Acknowledged.

+                {'longs': js.string(longs.join(','))}));
       }
       statements.add(js.statement(
           'if (objectClassObject instanceof Array)'
           '  objectClassObject = objectClassObject[1];'));
     }
 
     // TODO(9631): This is no longer valid for native methods.
     String whatToPatch = emitter.nativeEmitter.handleNoSuchMethod ?
                          "Object.prototype" :
                          "objectClassObject";
@@ skipping 15 matching lines @@
         for (var j = 0; j < shortNames.length; j++) {
           var type = 0;
           var short = shortNames[j];
           if (short[0] == "${namer.getterPrefix[0]}") type = 1;
           if (short[0] == "${namer.setterPrefix[0]}") type = 2;
           // Generate call to:
           //
           //     createInvocationMirror(String name, internalName, type,
           //         arguments, argumentNames)
           //
-          $whatToPatch[short] = (function(name, short, type, #) {
+          $whatToPatch[short] = (function(name, short,
+                                          type, #sliceOffsetParams) {
               return function() {
-                return this.#(this,
-                    #(name, short, type,
-                      Array.prototype.slice.call(arguments, #),
-                      []));
+                return this.#noSuchMethodName(this,
+                    #createInvocationMirror(name, short, type,
+                        Array.prototype.slice.call(arguments,
+                                                   #sliceOffsetParams),
+                                                   []));
               }
-          })(#[j], short, type, #);
+          })(#names[j], short, type, #sliceOffsetArguments);
         }
-      }''', [
-          sliceOffsetParams,  // parameter
-          noSuchMethodName,
-          createInvocationMirror,
-          sliceOffsetParams,  // argument to slice
-          minify ? 'shortNames' : 'longNames',
-          sliceOffsetArguments
-      ]));
+      }''', {
+          'sliceOffsetParams': sliceOffsetParams,
+          'noSuchMethodName': noSuchMethodName,
+          'createInvocationMirror': createInvocationMirror,
+          'names': minify ? 'shortNames' : 'longNames',
+          'sliceOffsetArguments': sliceOffsetArguments}));
 
     return statements;
   }
 }