Fix super getter/setter

Fix super getter/setter

Fix handling of super getters and setters. In particular, parser
now correctly converts super getters into super setters for
compound assignment, and pre- and postfix increment operators.

co 19 tests still fail because super index operator is broken.
Will work on this next.
Committed: https://code.google.com/p/dart/source/detail?r=10413

[srdjan, 2012-08-08 16:44:33]

LGTM with comments

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.cc
File runtime/vm/ast.cc (right):

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.cc#newcode327
runtime/vm/ast.cc:327: if (is_super_getter_) {
Add brief comment what is going on.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.cc#newcode366
runtime/vm/ast.cc:366: // instanse setter cannot be found either).
s/instanse/instance/

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.h
File runtime/vm/ast.h (right):

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.h#newcode1241
runtime/vm/ast.h:1241: AstNode* receiver,  // Null if called from static
context.
Change comment that it is maybe non-NULL even in static context.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.h#newcode1261
runtime/vm/ast.h:1261: const bool is_super_getter() const { return
is_super_getter_; }
s/const bool/bool/

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.h#newcode1275
runtime/vm/ast.h:1275: bool is_super_getter_;
const

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.h#newcode1284
runtime/vm/ast.h:1284: AstNode* receiver,
In a brief comment explain the need for receiver.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/flow_graph_build...
File runtime/vm/flow_graph_builder.cc (right):

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/flow_graph_build...
runtime/vm/flow_graph_builder.cc:1797: if (!node->is_super_getter()) {
Revert, use first the positive clause, is_super_getter.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:1424: const Class& super_class =
Class::ZoneHandle(current_class().SuperClass());
Why ZoneHandle, does the 'super_class' handle escape this scope?

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:6667: false,
add comment what false means.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:6777: false,
ditto

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:6817: false,
ditto

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:7218: false,
ditto

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:7270: false,
ditto

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:7406: false,
ditto and all other places.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/resolver.h
File runtime/vm/resolver.h (right):

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/resolver.h#newco...
runtime/vm/resolver.h:37: static RawFunction* ResolveDynamicAnyParams(
AnyArguments? The API above mentions arguments not parameters.

[hausner, 2012-08-08 17:42:02]

Thanks. If you insist in the annotations in parser.c, I'll add them later.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.cc
File runtime/vm/ast.cc (right):

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.cc#newcode327
runtime/vm/ast.cc:327: if (is_super_getter_) {
On 2012/08/08 16:44:34, srdjan wrote:
> Add brief comment what is going on.

Done.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.cc#newcode366
runtime/vm/ast.cc:366: // instanse setter cannot be found either).
On 2012/08/08 16:44:34, srdjan wrote:
> s/instanse/instance/

Done.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.h
File runtime/vm/ast.h (right):

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.h#newcode1241
runtime/vm/ast.h:1241: AstNode* receiver,  // Null if called from static
context.
On 2012/08/08 16:44:34, srdjan wrote:
> Change comment that it is maybe non-NULL even in static context.
super getters are dynamic functions, so it's not a static context. But I removed
the comment completely because it does not help much. Instead, see comment above
the field "receiver" below.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.h#newcode1261
runtime/vm/ast.h:1261: const bool is_super_getter() const { return
is_super_getter_; }
On 2012/08/08 16:44:34, srdjan wrote:
> s/const bool/bool/

Done.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.h#newcode1275
runtime/vm/ast.h:1275: bool is_super_getter_;
On 2012/08/08 16:44:34, srdjan wrote:
> const

Done.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/ast.h#newcode1284
runtime/vm/ast.h:1284: AstNode* receiver,
On 2012/08/08 16:44:34, srdjan wrote:
> In a brief comment explain the need for receiver.

Done.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/flow_graph_build...
File runtime/vm/flow_graph_builder.cc (right):

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/flow_graph_build...
runtime/vm/flow_graph_builder.cc:1797: if (!node->is_super_getter()) {
On 2012/08/08 16:44:34, srdjan wrote:
> Revert, use first the positive clause, is_super_getter.

Done.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:1424: const Class& super_class =
Class::ZoneHandle(current_class().SuperClass());
On 2012/08/08 16:44:34, srdjan wrote:
> Why ZoneHandle, does the 'super_class' handle escape this scope?
Yes, it is used in the static getter node in line 1451.

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/parser.cc#newcod...
runtime/vm/parser.cc:6667: false,
On 2012/08/08 16:44:34, srdjan wrote:
> add comment what false means.

I don't think that makes the program more readable. Why add the comment for
false, but not for NULL?

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/resolver.h
File runtime/vm/resolver.h (right):

http://codereview.chromium.org/10849004/diff/7001/runtime/vm/resolver.h#newco...
runtime/vm/resolver.h:37: static RawFunction* ResolveDynamicAnyParams(
On 2012/08/08 16:44:34, srdjan wrote:
> AnyArguments? The API above mentions arguments not parameters.

Done.