Support tearoffs in the new emitter.

Support tearoffs in the new emitter.

BUG=
R=floitsch@google.com

Committed: https://code.google.com/p/dart/source/detail?r=43429

[herhut, 2015-02-03 14:49:03]

Patchset #1 (id:1) has been deleted

[herhut, 2015-02-03 14:53:21]

Patchset #1 (id:20001) has been deleted

[herhut, 2015-02-03 14:55:52]

Patchset #1 (id:40001) has been deleted

[herhut, 2015-02-03 14:57:08]

This includes

 https://codereview.chromium.org/889703004
 http://crrev.com/873883006

plus

 added function types
 formatting
 minor fixes

and should complete the implementation of tear offs.

[herhut, 2015-02-03 14:57:28]

herhut@google.com changed reviewers:
+ floitsch@google.com, zarah@google.com

[herhut, 2015-02-03 14:57:52]

Resending, this time with reviewers...

On 2015/02/03 14:57:08, herhut wrote:
> This includes
> 
>  https://codereview.chromium.org/889703004
>  http://crrev.com/873883006
> 
> plus
> 
>  added function types
>  formatting
>  minor fixes
> 
> and should complete the implementation of tear offs.

[floitsch, 2015-02-03 19:40:02]

LGTM.

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart (right):

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:447: var stub =
descriptor[pos+2];
nit: "pos + 2"
(and "pos + 1" in the next line).

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:529: //
TODO(floitsch): can there be anything else than a StaticDartMethod?
Well now there can be, since you run it over every method.

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:542: return
[js.string(method.name), holderIndex, new js.ArrayInitializer(data)];
long line.

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:560: var
functionCounter = 0;
:/
Can this be emitted with the tear-off function?

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:603: //
TODO(herhut): Share parser with instance methods.
I'm not sure that's worth the effort, but I agree that we should at least try.

https://codereview.chromium.org/895083002/diff/60001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/js_lib/js_helper.dart (right):

https://codereview.chromium.org/895083002/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/js_lib/js_helper.dart:1966: var reflectionInfo,
Add comments to the function with the information we have.

[herhut, 2015-02-03 21:18:51]

New patchsets have been uploaded after l-g-t-m from floitsch@google.com

[herhut, 2015-02-03 21:18:59]

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart (right):

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:447: var stub =
descriptor[pos+2];
On 2015/02/03 19:40:02, floitsch wrote:
> nit: "pos + 2"
> (and "pos + 1" in the next line).

Done.

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:529: //
TODO(floitsch): can there be anything else than a StaticDartMethod?
On 2015/02/03 19:40:02, floitsch wrote:
> Well now there can be, since you run it over every method.

Doesn't this still only run over static methods?

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:542: return
[js.string(method.name), holderIndex, new js.ArrayInitializer(data)];
On 2015/02/03 19:40:02, floitsch wrote:
> long line.

Done.

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:560: var
functionCounter = 0;
On 2015/02/03 19:40:02, floitsch wrote:
> :/
> Can this be emitted with the tear-off function?

In a separate CL to do it for both emitters.

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:603: //
TODO(herhut): Share parser with instance methods.
On 2015/02/03 19:40:02, floitsch wrote:
> I'm not sure that's worth the effort, but I agree that we should at least try.

Acknowledged.

https://codereview.chromium.org/895083002/diff/60001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/js_lib/js_helper.dart (right):

https://codereview.chromium.org/895083002/diff/60001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/js_lib/js_helper.dart:1966: var reflectionInfo,
On 2015/02/03 19:40:02, floitsch wrote:
> Add comments to the function with the information we have.

Done.

[herhut, 2015-02-03 21:32:42]

Committed patchset #2 (id:80001) manually as 43429 (presubmit successful).

[floitsch, 2015-02-03 21:44:17]

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart (right):

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:529: //
TODO(floitsch): can there be anything else than a StaticDartMethod?
On 2015/02/03 21:18:58, herhut wrote:
> On 2015/02/03 19:40:02, floitsch wrote:
> > Well now there can be, since you run it over every method.
> 
> Doesn't this still only run over static methods?

Never mind...

https://codereview.chromium.org/895083002/diff/60001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:560: var
functionCounter = 0;
On 2015/02/03 21:18:58, herhut wrote:
> On 2015/02/03 19:40:02, floitsch wrote:
> > :/
> > Can this be emitted with the tear-off function?
> 
> In a separate CL to do it for both emitters.

Acknowledged.

[sra1, 2015-02-04 19:00:06]

sra@google.com changed reviewers:
+ sra@google.com

[sra1, 2015-02-04 19:00:07]

The generated code has this wierdness:

closureFromTearOff: function(receiver, functions, reflectionInfo, isStatic,
jsArguments, $name) {
    var t1;
    functions.fixed$length = Array;
    if (!!J.getInterceptor(reflectionInfo).$isList) {
      reflectionInfo.fixed$length = Array;
      t1 = reflectionInfo;
    } else
      t1 = reflectionInfo;
    return H.Closure_fromTearOff(receiver, functions, t1, !!isStatic,
jsArguments, $name);
  },

Is it possible to avoid the getInterceptor call here, since the then-branch
assumes an JS Array.

[floitsch, 2015-02-05 16:35:13]

On 2015/02/04 19:00:07, sra1 wrote:
> The generated code has this wierdness:
> 
> closureFromTearOff: function(receiver, functions, reflectionInfo, isStatic,
> jsArguments, $name) {
>     var t1;
>     functions.fixed$length = Array;
>     if (!!J.getInterceptor(reflectionInfo).$isList) {
>       reflectionInfo.fixed$length = Array;
>       t1 = reflectionInfo;
>     } else
>       t1 = reflectionInfo;
>     return H.Closure_fromTearOff(receiver, functions, t1, !!isStatic,
> jsArguments, $name);
>   },
> 
> Is it possible to avoid the getInterceptor call here, since the then-branch
> assumes an JS Array.

Would you do it using JS magic, or with a different "is" check, or a whole
different approach?