dart2js: Properly construct rti ast templates.

dart2js: Properly construct rti ast templates.

BUG=
R=floitsch@google.com
TBR=floitsch@google.com

Committed: https://github.com/dart-lang/sdk/commit/0d24a53a560dcf1ce884cdb53478475b0c562a16

[herhut, 2015-05-20 13:40:50]

Since moving to constructing a full ast, rti annotations with type variables
turned into functions like

(a,b) => [a, a]

because the template was constructed wrongly. Now, we should see the original
result again.

[floitsch, 2015-05-20 13:48:19]

LGTM.

https://codereview.chromium.org/1146003004/diff/1/pkg/compiler/lib/src/ssa/bu...
File pkg/compiler/lib/src/ssa/builder.dart (right):

https://codereview.chromium.org/1146003004/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder.dart:3564:
templates.add(rti.getTypeRepresentationWithPlaceholders(argument, (variable) {
long line.

https://codereview.chromium.org/1146003004/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder.dart:3567: }, firstPlaceholderIndex :
inputs.length));
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 13:49:00]

Committed patchset #1 (id:1) manually as
0d24a53a560dcf1ce884cdb53478475b0c562a16 (presubmit successful).

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

https://codereview.chromium.org/1146003004/diff/1/pkg/compiler/lib/src/ssa/bu...
File pkg/compiler/lib/src/ssa/builder.dart (right):

https://codereview.chromium.org/1146003004/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder.dart:3564:
templates.add(rti.getTypeRepresentationWithPlaceholders(argument, (variable) {
On 2015/05/20 13:48:18, floitsch wrote:
> long line.

Done.

https://codereview.chromium.org/1146003004/diff/1/pkg/compiler/lib/src/ssa/bu...
pkg/compiler/lib/src/ssa/builder.dart:3567: }, firstPlaceholderIndex :
inputs.length));
On 2015/05/20 13:48:18, floitsch wrote:
> 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;

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?

[herhut, 2015-05-20 14:34:12]

On 2015/05/20 14:27:59, herhut wrote:
>
https://codereview.chromium.org/1146003004/diff/1/pkg/compiler/lib/src/ssa/bu...
> File pkg/compiler/lib/src/ssa/builder.dart (right):
> 
>
https://codereview.chromium.org/1146003004/diff/1/pkg/compiler/lib/src/ssa/bu...
> pkg/compiler/lib/src/ssa/builder.dart:3564:
> templates.add(rti.getTypeRepresentationWithPlaceholders(argument, (variable) {
> On 2015/05/20 13:48:18, floitsch wrote:
> > long line.
> 
> Done.
> 
>
https://codereview.chromium.org/1146003004/diff/1/pkg/compiler/lib/src/ssa/bu...
> pkg/compiler/lib/src/ssa/builder.dart:3567: }, firstPlaceholderIndex :
> inputs.length));
> On 2015/05/20 13:48:18, floitsch wrote:
> > 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;
> 
> 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?

See https://codereview.chromium.org/1143243002.