Change how we generate bound closures to handler boud closures due to super getters: we need to pas…

Change how we generate bound closures to handler bound closures due to super getters: we need to pass the exact function to call instead of fetching it from the receiver.

R=ahe@google.com, kasperl@google.com

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

[kasperl, 2013-09-06 10:10:15]

LGTM!

[ngeoffray, 2013-09-07 13:10:00]

Thanks Kasper. PTAL, I had to also fix the hashCode computation of the
BoundClosure class.

[kasperl, 2013-09-09 06:40:40]

Still LGTM if you address my one comment:

https://codereview.chromium.org/23567019/diff/5001/sdk/lib/_internal/lib/js_h...
File sdk/lib/_internal/lib/js_helper.dart (right):

https://codereview.chromium.org/23567019/diff/5001/sdk/lib/_internal/lib/js_h...
sdk/lib/_internal/lib/js_helper.dart:1517: JS('String', '#.toString().length',
_target),
The type 'String' seems very wrong here. Also, the hash code here probably isn't
very good, but maybe that's okay for now.

[ngeoffray, 2013-09-09 07:08:12]

https://codereview.chromium.org/23567019/diff/5001/sdk/lib/_internal/lib/js_h...
File sdk/lib/_internal/lib/js_helper.dart (right):

https://codereview.chromium.org/23567019/diff/5001/sdk/lib/_internal/lib/js_h...
sdk/lib/_internal/lib/js_helper.dart:1517: JS('String', '#.toString().length',
_target),
On 2013/09/09 06:40:40, kasperl wrote:
> The type 'String' seems very wrong here. Also, the hash code here probably
isn't
> very good, but maybe that's okay for now.

Well spotted for the return type. I initially thought about just using
String.hashCode, but that's probably expensive on full function bodies. Which
one do you prefer?

[ngeoffray, 2013-09-09 10:22:06]

https://codereview.chromium.org/23567019/diff/5001/sdk/lib/_internal/lib/js_h...
File sdk/lib/_internal/lib/js_helper.dart (right):

https://codereview.chromium.org/23567019/diff/5001/sdk/lib/_internal/lib/js_h...
sdk/lib/_internal/lib/js_helper.dart:1517: JS('String', '#.toString().length',
_target),
On 2013/09/09 07:08:13, ngeoffray wrote:
> On 2013/09/09 06:40:40, kasperl wrote:
> > The type 'String' seems very wrong here. Also, the hash code here probably
> isn't
> > very good, but maybe that's okay for now.
> 
> Well spotted for the return type. I initially thought about just using
> String.hashCode, but that's probably expensive on full function bodies. Which
> one do you prefer?

Opened https://code.google.com/p/dart/issues/detail?id=13177.

[ngeoffray, 2013-09-09 11:18:36]

+ahe

Adding Peter since he convinced me the hashCode implementation was pretty bad
(also in terms of correctness) to keep it as is.

[ahe, 2013-09-11 12:16:03]

LGTM, but please test ClosureMirror.function.

[ngeoffray, 2013-09-13 08:38:51]

On 2013/09/11 12:16:03, ahe wrote:
> LGTM, but please test ClosureMirror.function.

Done, but PTAL to make sure the mirror changes make sense.

[ahe, 2013-09-16 12:26:40]

LGTM!

https://codereview.chromium.org/23567019/diff/35001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/23567019/diff/35001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2603: String
className = backend.namer.isolateAccess(classElement);
Use elementAccess, not isolateAccess.

https://codereview.chromium.org/23567019/diff/35001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2604:
jsAst.VariableUse classReference = new jsAst.VariableUse(className);
And then don't wrap it in VariableUse.

https://codereview.chromium.org/23567019/diff/35001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2608: new
jsAst.PropertyAccess.field(prototype, targetName);
The above lines can be written as:

jsAst.PropertyAccess method =
    backend.namer.elementAccess(classElement)['prototype'][targetName];

[ngeoffray, 2013-09-16 12:34:35]

Thanks Peter.

https://codereview.chromium.org/23567019/diff/35001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/23567019/diff/35001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2603: String
className = backend.namer.isolateAccess(classElement);
On 2013/09/16 12:26:40, ahe wrote:
> Use elementAccess, not isolateAccess.

Done.

https://codereview.chromium.org/23567019/diff/35001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2604:
jsAst.VariableUse classReference = new jsAst.VariableUse(className);
On 2013/09/16 12:26:40, ahe wrote:
> And then don't wrap it in VariableUse.

Done.

https://codereview.chromium.org/23567019/diff/35001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2608: new
jsAst.PropertyAccess.field(prototype, targetName);
On 2013/09/16 12:26:40, ahe wrote:
> The above lines can be written as:
> 
> jsAst.PropertyAccess method =
>     backend.namer.elementAccess(classElement)['prototype'][targetName];

Done.

[ngeoffray, 2013-09-16 13:09:24]

Committed patchset #12 manually as r27519 (presubmit successful).