Implement super calls.

Implement super calls.

TEST=tests/leg_only/src/SuperCallTest.dart, ResolverTest.testSuperCalls.

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

[ngeoffray, 2012-01-10 16:52:50]

Please also update the SSA tracer.

http://codereview.chromium.org/9166010/diff/1/frog/leg/resolver.dart
File frog/leg/resolver.dart (right):

http://codereview.chromium.org/9166010/diff/1/frog/leg/resolver.dart#newcode340
frog/leg/resolver.dart:340: ClassElement thisElement =
enclosingElement.enclosingElement;
I think I'd prefer having a classElement field in ResolverVisitor. It looks like
this does not scale with closures.

http://codereview.chromium.org/9166010/diff/1/frog/leg/resolver.dart#newcode342
frog/leg/resolver.dart:342: if (superElement === null) {
Should we attach the objectElement to the superElement in the Element instead?

http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/builder.dart
File frog/leg/ssa/builder.dart (right):

http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/builder.dart#newco...
frog/leg/ssa/builder.dart:554: } else if (node.isSuper()) {
maybe add at the entry of this function assert(!node.isSuper())

http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/builder.dart#newco...
frog/leg/ssa/builder.dart:1003: inputs.add(context);
put target and context in the list literal.

http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/nodes.dart
File frog/leg/ssa/nodes.dart (right):

http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/nodes.dart#newcode...
frog/leg/ssa/nodes.dart:1023: class HInvokeSuper extends HInvoke {
Should that be an HInvokeStatic instead?

[karlklose, 2012-01-11 12:44:43]

On 2012/01/10 16:52:50, ngeoffray wrote:
> Please also update the SSA tracer.
> 
> http://codereview.chromium.org/9166010/diff/1/frog/leg/resolver.dart
> File frog/leg/resolver.dart (right):
> 
>
http://codereview.chromium.org/9166010/diff/1/frog/leg/resolver.dart#newcode340
> frog/leg/resolver.dart:340: ClassElement thisElement =
> enclosingElement.enclosingElement;
> I think I'd prefer having a classElement field in ResolverVisitor. It looks
like
> this does not scale with closures.
> 
>
http://codereview.chromium.org/9166010/diff/1/frog/leg/resolver.dart#newcode342
> frog/leg/resolver.dart:342: if (superElement === null) {
> Should we attach the objectElement to the superElement in the Element instead?
> 
> http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/builder.dart
> File frog/leg/ssa/builder.dart (right):
> 
>
http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/builder.dart#newco...
> frog/leg/ssa/builder.dart:554: } else if (node.isSuper()) {
> maybe add at the entry of this function assert(!node.isSuper())
> 
>
http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/builder.dart#newco...
> frog/leg/ssa/builder.dart:1003: inputs.add(context);
> put target and context in the list literal.
> 
> http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/nodes.dart
> File frog/leg/ssa/nodes.dart (right):
> 
>
http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/nodes.dart#newcode...
> frog/leg/ssa/nodes.dart:1023: class HInvokeSuper extends HInvoke {
> Should that be an HInvokeStatic instead?

PTAL.

[karlklose, 2012-01-11 12:45:17]

http://codereview.chromium.org/9166010/diff/1/frog/leg/resolver.dart
File frog/leg/resolver.dart (right):

http://codereview.chromium.org/9166010/diff/1/frog/leg/resolver.dart#newcode340
frog/leg/resolver.dart:340: ClassElement thisElement =
enclosingElement.enclosingElement;
Done, added field currentClass.

http://codereview.chromium.org/9166010/diff/1/frog/leg/resolver.dart#newcode342
frog/leg/resolver.dart:342: if (superElement === null) {
On 2012/01/10 16:52:50, ngeoffray wrote:
> Should we attach the objectElement to the superElement in the Element instead?

Done.

http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/builder.dart
File frog/leg/ssa/builder.dart (right):

http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/builder.dart#newco...
frog/leg/ssa/builder.dart:554: } else if (node.isSuper()) {
I generate an internal error.

http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/builder.dart#newco...
frog/leg/ssa/builder.dart:1003: inputs.add(context);
On 2012/01/10 16:52:50, ngeoffray wrote:
> put target and context in the list literal.

Done.

http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/nodes.dart
File frog/leg/ssa/nodes.dart (right):

http://codereview.chromium.org/9166010/diff/1/frog/leg/ssa/nodes.dart#newcode...
frog/leg/ssa/nodes.dart:1023: class HInvokeSuper extends HInvoke {
On 2012/01/10 16:52:50, ngeoffray wrote:
> Should that be an HInvokeStatic instead?

Done.

[ngeoffray, 2012-01-11 13:00:48]

LGTM!

http://codereview.chromium.org/9166010/diff/4001/frog/leg/resolver.dart
File frog/leg/resolver.dart (right):

http://codereview.chromium.org/9166010/diff/4001/frog/leg/resolver.dart#newco...
frog/leg/resolver.dart:344: print(currentClass);
Remove debugging code

http://codereview.chromium.org/9166010/diff/4001/frog/leg/resolver.dart#newco...
frog/leg/resolver.dart:355: error(node, MessageKind.METHOD_NOT_FOUND,
[superName, name]);
I think I'd prefer two kinds of errors, one for the case of currentClass is null
and for the one where it is not null. For the one that is null, we already had
an error message. So I'm not sure what kind of error messages we can provide
again.

http://codereview.chromium.org/9166010/diff/4001/frog/leg/ssa/builder.dart
File frog/leg/ssa/builder.dart (right):

http://codereview.chromium.org/9166010/diff/4001/frog/leg/ssa/builder.dart#ne...
frog/leg/ssa/builder.dart:1027: node.receiver.asIdentifier().isSuper()) {
I think we can add a Send.isSuperCall to the Send class.

http://codereview.chromium.org/9166010/diff/4001/frog/leg/ssa/nodes.dart
File frog/leg/ssa/nodes.dart (right):

http://codereview.chromium.org/9166010/diff/4001/frog/leg/ssa/nodes.dart#newc...
frog/leg/ssa/nodes.dart:1026: accept(HVisitor visitor) =>
visitor.visitInvokeSuper(this);
The two methods below are already defined in HInvokeStatic.

http://codereview.chromium.org/9166010/diff/4001/frog/leg/ssa/tracer.dart
File frog/leg/ssa/tracer.dart (right):

http://codereview.chromium.org/9166010/diff/4001/frog/leg/ssa/tracer.dart#new...
frog/leg/ssa/tracer.dart:274: print(target);
Remove print.

[karlklose, 2012-01-11 13:46:12]

Thanks for the review, Nicolas and Florian.

http://codereview.chromium.org/9166010/diff/4001/frog/leg/resolver.dart
File frog/leg/resolver.dart (right):

http://codereview.chromium.org/9166010/diff/4001/frog/leg/resolver.dart#newco...
frog/leg/resolver.dart:344: print(currentClass);
On 2012/01/11 13:00:48, ngeoffray wrote:
> Remove debugging code

Done.

http://codereview.chromium.org/9166010/diff/4001/frog/leg/resolver.dart#newco...
frog/leg/resolver.dart:355: error(node, MessageKind.METHOD_NOT_FOUND,
[superName, name]);
Yes we already report an error if we encounter super in a non-instance context.

http://codereview.chromium.org/9166010/diff/4001/frog/leg/ssa/builder.dart
File frog/leg/ssa/builder.dart (right):

http://codereview.chromium.org/9166010/diff/4001/frog/leg/ssa/builder.dart#ne...
frog/leg/ssa/builder.dart:1027: node.receiver.asIdentifier().isSuper()) {
On 2012/01/11 13:00:48, ngeoffray wrote:
> I think we can add a Send.isSuperCall to the Send class.

Done.

http://codereview.chromium.org/9166010/diff/4001/frog/leg/ssa/nodes.dart
File frog/leg/ssa/nodes.dart (right):

http://codereview.chromium.org/9166010/diff/4001/frog/leg/ssa/nodes.dart#newc...
frog/leg/ssa/nodes.dart:1026: accept(HVisitor visitor) =>
visitor.visitInvokeSuper(this);
Done, removed.

http://codereview.chromium.org/9166010/diff/4001/frog/leg/ssa/tracer.dart
File frog/leg/ssa/tracer.dart (right):

http://codereview.chromium.org/9166010/diff/4001/frog/leg/ssa/tracer.dart#new...
frog/leg/ssa/tracer.dart:274: print(target);
On 2012/01/11 13:00:48, ngeoffray wrote:
> Remove print.

Done.

[ahe, 2012-01-11 13:55:58]

DBC

http://codereview.chromium.org/9166010/diff/9002/frog/leg/resolver.dart
File frog/leg/resolver.dart (right):

http://codereview.chromium.org/9166010/diff/9002/frog/leg/resolver.dart#newco...
frog/leg/resolver.dart:368: ClassElement superElement = currentClass.superClass;
It is called "superclass", but an element should not have a superclass. It has a
supertype.