dart2js: Properly construct rti ast templates.

pkg/compiler/lib/src/ssa/builder.dart

 // Copyright (c) 2012, 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 ssa;
 
 class SsaFunctionCompiler implements FunctionCompiler {
   SsaCodeGeneratorTask generator;
   SsaBuilderTask builder;
   SsaOptimizerTask optimizer;
@@ skipping 3543 matching lines @@
     // to the runtime type information for type variables as instructions.
     if (type.isTypeVariable) {
       return buildLiteralList(<HInstruction>[addTypeVariableReference(type)]);
     } else {
       assert(type.element.isClass);
       InterfaceType interface = type;
       List<HInstruction> inputs = <HInstruction>[];
       bool first = true;
       List<js.Expression> templates = <js.Expression>[];
       for (DartType argument in interface.typeArguments) {
         templates.add(rti.getTypeRepresentationWithPlaceholders(argument, (variable) {

[floitsch, 2015/05/20 13:48:18]

long line.

[herhut, 2015/05/20 14:27:59]

Done.

           HInstruction runtimeType = addTypeVariableReference(variable);
           inputs.add(runtimeType);
-        }));
+        }, firstPlaceholderIndex : inputs.length));

[floitsch, 2015/05/20 13:48:18]

No space after "firstPlaceholderIndex".

I would prefer if it was more explicit.
That is, create an `int placeholderIndex = 0` outside.
Then, in the closure:

  // The runtimeType has one placeholder.
  assert(...);  // If you can find an easy way to check.
  placeholderIndex++;
  inputs.add(runtimeType);
}, firstPlaceholderIndex: placeholderIndex;

[herhut, 2015/05/20 14:27:59]

This constructs a template with holes and the corresponding inputs that will
later fill these holes when we generate the code. So there is always a one to
one mapping between inputs and holes. That is why the length of the inputs is
always the correct starting value. 

Using an explicit counter decouples this on the surface but the underlying
dependency is still there. How about a comment?

       }
       // TODO(sra): This is a fresh template each time.  We can't let the
       // template manager build them.
       js.Template code = new js.Template(null,
                                          new js.ArrayInitializer(templates));
       HInstruction representation =
           new HForeignCode(code, backend.readableArrayType, inputs,
               nativeBehavior: native.NativeBehavior.PURE_ALLOCATION);
       return representation;
     }
@@ skipping 4086 matching lines @@
     if (unaliased is TypedefType) throw 'unable to unalias $type';
     unaliased.accept(this, builder);
   }
 
   void visitDynamicType(DynamicType type, SsaBuilder builder) {
     JavaScriptBackend backend = builder.compiler.backend;
     ClassElement cls = backend.findHelper('DynamicRuntimeType');
     builder.push(new HDynamicType(type, new TypeMask.exact(cls, classWorld)));
   }
 }