dart2js: don't emit tear-off data if not needed.

dart2js: don't emit unneeded tear-off data in the new emitter.

R=floitsch@google.com

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

[zarah, 2015-02-09 11:17:43]

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

[floitsch, 2015-02-09 12:41:41]

LGTM with comments.

https://codereview.chromium.org/910723002/diff/1/pkg/compiler/lib/src/js_emit...
File pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart (right):

https://codereview.chromium.org/910723002/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:538: // Install
the alias for super calls on the prototype chain.
Move this code into right order.
First a check for desc[2]. Then for desc[3] and then starting at pos == 6.

https://codereview.chromium.org/910723002/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:585: if
(method.needsTearOff) {
You can only avoid adding these if there aren't any parameterStubs. Otherwise
your stubs are shifted and you miss some of them.

[zarah, 2015-02-09 15:14:37]

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

[zarah, 2015-02-09 15:27:16]

Patchset #2 (id:20001) has been deleted

[zarah, 2015-02-09 15:52:01]

PTAL

https://codereview.chromium.org/910723002/diff/1/pkg/compiler/lib/src/js_emit...
File pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart (right):

https://codereview.chromium.org/910723002/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:538: // Install
the alias for super calls on the prototype chain.
On 2015/02/09 12:41:41, floitsch wrote:
> Move this code into right order.
> First a check for desc[2]. Then for desc[3] and then starting at pos == 6.

Acknowledged.

https://codereview.chromium.org/910723002/diff/1/pkg/compiler/lib/src/js_emit...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:585: if
(method.needsTearOff) {
On 2015/02/09 12:41:41, floitsch wrote:
> You can only avoid adding these if there aren't any parameterStubs. Otherwise
> your stubs are shifted and you miss some of them.

Acknowledged.

[floitsch, 2015-02-09 16:04:25]

LGTM.

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

https://codereview.chromium.org/910723002/diff/80001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:513: ///
[JavaScriptBackend.isInterceptedMethod]
No need to drag "JavaScriptBackend" into this.
just "isInterceptedMethod".

https://codereview.chromium.org/910723002/diff/80001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:513: ///
[JavaScriptBackend.isInterceptedMethod]
That one is optional too, no?

https://codereview.chromium.org/910723002/diff/80001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:527: var f, pos =
-1;
// `pos` points to the last read entry.

https://codereview.chromium.org/910723002/diff/80001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:538:
funs[0].$callName = descriptor[++pos];
might as well use "f" directly here.

[zarah, 2015-02-10 09:05:10]

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

[zarah, 2015-02-10 09:07:18]

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

https://codereview.chromium.org/910723002/diff/80001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:513: ///
[JavaScriptBackend.isInterceptedMethod]
On 2015/02/09 16:04:24, floitsch wrote:
> That one is optional too, no?

yes, added a comment to the following two as well.

https://codereview.chromium.org/910723002/diff/80001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:513: ///
[JavaScriptBackend.isInterceptedMethod]
On 2015/02/09 16:04:24, floitsch wrote:
> No need to drag "JavaScriptBackend" into this.
> just "isInterceptedMethod".

Done.

https://codereview.chromium.org/910723002/diff/80001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:527: var f, pos =
-1;
On 2015/02/09 16:04:24, floitsch wrote:
> // `pos` points to the last read entry.

Done.

https://codereview.chromium.org/910723002/diff/80001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/new_emitter/model_emitter.dart:538:
funs[0].$callName = descriptor[++pos];
On 2015/02/09 16:04:24, floitsch wrote:
> might as well use "f" directly here.

Done.

[zarah, 2015-02-10 12:01:21]

Committed patchset #5 (id:100001) manually as 43639 (presubmit successful).