NoSuchMethod on different super accesses/invocations handled.

NoSuchMethod on different super accesses/invocations handled.

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

[ngeoffray, 2012-12-28 10:23:58]

LGTM!

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:394: if
(isAccessor) return const <String,dynamic>{};
Shouldn't that be <dynamic>{} ?

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3650: bool assignable
= true;
Maybe a Elements.isAssignable helper method could improve readability of this
method. You would then have to write:
if (!Elements.isResolved(element) || !Elements.isAssignable(element)) {
  return generateSuperNoSuchMethodSend(node);
}

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/nodes.dart (right):

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/nodes.dart:1432: final bool
isGetter;
Aren't these fields misnamed? Shouldn't they really be isSendSet and
isPropertyAccess? Otherwise you might think that the element this super is
refering to is a Dart setter or a Dart getter, and that is not what those fields
express.

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/nodes.dart:1433:
HInvokeSuper(inputs, {this.isSetter: false, this.isGetter: false}) :
super(inputs);
long line.

https://codereview.chromium.org/11688010/diff/1/tests/language/super_no_such_...
File tests/language/super_no_such_method_test.dart (right):

https://codereview.chromium.org/11688010/diff/1/tests/language/super_no_such_...
tests/language/super_no_such_method_test.dart:8: noSuchMethod(im) => true;
Too much indentation here and below.

https://codereview.chromium.org/11688010/diff/1/tests/language/super_no_such_...
tests/language/super_no_such_method_test.dart:26: 
Do we also have a test where we get the bound closure of a super method?

[Johnni Winther, 2012-12-28 22:32:31]

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:394: if
(isAccessor) return const <String,dynamic>{};
On 2012/12/28 10:24:03, ngeoffray wrote:
> Shouldn't that be <dynamic>{} ?

The syntax was changed to include the key type before M1.

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/builder.dart (right):

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/builder.dart:3650: bool assignable
= true;
On 2012/12/28 10:24:03, ngeoffray wrote:
> Maybe a Elements.isAssignable helper method could improve readability of this
> method. You would then have to write:
> if (!Elements.isResolved(element) || !Elements.isAssignable(element)) {
>   return generateSuperNoSuchMethodSend(node);
> }

Done.

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/ssa/nodes.dart (right):

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/nodes.dart:1432: final bool
isGetter;
On 2012/12/28 10:24:03, ngeoffray wrote:
> Aren't these fields misnamed? Shouldn't they really be isSendSet and
> isPropertyAccess? Otherwise you might think that the element this super is
> refering to is a Dart setter or a Dart getter, and that is not what those
fields
> express.

Done.

https://codereview.chromium.org/11688010/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/ssa/nodes.dart:1433:
HInvokeSuper(inputs, {this.isSetter: false, this.isGetter: false}) :
super(inputs);
On 2012/12/28 10:24:03, ngeoffray wrote:
> long line.

Done.

https://codereview.chromium.org/11688010/diff/1/tests/language/super_no_such_...
File tests/language/super_no_such_method_test.dart (right):

https://codereview.chromium.org/11688010/diff/1/tests/language/super_no_such_...
tests/language/super_no_such_method_test.dart:8: noSuchMethod(im) => true;
On 2012/12/28 10:24:03, ngeoffray wrote:
> Too much indentation here and below.

Done.

https://codereview.chromium.org/11688010/diff/1/tests/language/super_no_such_...
tests/language/super_no_such_method_test.dart:26: 
On 2012/12/28 10:24:03, ngeoffray wrote:
> Do we also have a test where we get the bound closure of a super method?

No, I'll add one.