More fixes for super[]

More fixes for super[]

Extend the LoadIndexedNode to also handle super[] and super[]=

Make sure super []= returns the value.
Handle side effects in index expression correctly.
Defer operator function resolution to the flow graph build phase.
Committed: https://code.google.com/p/dart/source/detail?r=14157

[hausner, 2012-10-26 18:41:10]

Finally a simple solution that works in all cases.

[regis, 2012-10-26 22:04:37]

LGTM

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

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/ast.h#newcode1126
runtime/vm/ast.h:1126: ASSERT(index_expr_ != NULL);
ASSERT(super_class_.IsZoneHandle());

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/ast.h#newcode1165
runtime/vm/ast.h:1165: ASSERT(value_ != NULL);
ASSERT(super_class_.IsZoneHandle());

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
File runtime/vm/flow_graph_builder.cc (right):

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
runtime/vm/flow_graph_builder.cc:2308: // Generate dynamic call to index
operator
period

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
runtime/vm/flow_graph_builder.cc:2405: name,
bad indentation

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
runtime/vm/flow_graph_builder.cc:2672: const Class& target_class,
indentation

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
runtime/vm/flow_graph_builder.cc:2697: 
It seems quite late to build ast nodes in the graph builder. I am concerned,
because this code will get more complicated once we support invocation mirrors.
I am not sure we will be able to properly handle additional calls here.
But it's OK to submit as is. We'll revisit as discussed.

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
File runtime/vm/flow_graph_builder.h (right):

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
runtime/vm/flow_graph_builder.h:260: 
Strange indentation.

[hausner, 2012-10-26 22:25:14]

Thank you for the review.

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

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/ast.h#newcode1126
runtime/vm/ast.h:1126: ASSERT(index_expr_ != NULL);
On 2012/10/26 22:04:37, regis wrote:
> ASSERT(super_class_.IsZoneHandle());

Done.

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/ast.h#newcode1165
runtime/vm/ast.h:1165: ASSERT(value_ != NULL);
On 2012/10/26 22:04:37, regis wrote:
> ASSERT(super_class_.IsZoneHandle());

Done.

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
File runtime/vm/flow_graph_builder.cc (right):

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
runtime/vm/flow_graph_builder.cc:2308: // Generate dynamic call to index
operator
On 2012/10/26 22:04:37, regis wrote:
> period

Done.

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
runtime/vm/flow_graph_builder.cc:2405: name,
On 2012/10/26 22:04:37, regis wrote:
> bad indentation

Done.

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
runtime/vm/flow_graph_builder.cc:2672: const Class& target_class,
On 2012/10/26 22:04:37, regis wrote:
> indentation
Better?

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
runtime/vm/flow_graph_builder.cc:2697: 
On 2012/10/26 22:04:37, regis wrote:
> It seems quite late to build ast nodes in the graph builder. I am concerned,
> because this code will get more complicated once we support invocation
mirrors.
> I am not sure we will be able to properly handle additional calls here.
> But it's OK to submit as is. We'll revisit as discussed.

As we discussed offline, this will be temporary until we introduce new stubs
that handle NoSuchMethod cases. In the meantime, this change fixes some bugs we
have with handling super operator[] and operator[]=.

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
File runtime/vm/flow_graph_builder.h (right):

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
runtime/vm/flow_graph_builder.h:260: 
On 2012/10/26 22:04:37, regis wrote:
> Strange indentation.

Done.

[regis, 2012-10-26 22:31:57]

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
File runtime/vm/flow_graph_builder.cc (right):

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
runtime/vm/flow_graph_builder.cc:2672: const Class& target_class,
On 2012/10/26 22:25:14, hausner wrote:
> On 2012/10/26 22:04:37, regis wrote:
> > indentation
> Better?

I think we use 4 spaces for continuation lines. You have 6 now :-)

[hausner, 2012-10-26 22:39:59]

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
File runtime/vm/flow_graph_builder.cc (right):

http://codereview.chromium.org/11267027/diff/11002/runtime/vm/flow_graph_buil...
runtime/vm/flow_graph_builder.cc:2672: const Class& target_class,
Will fix in a future checkin.

On 2012/10/26 22:31:57, regis wrote:
> On 2012/10/26 22:25:14, hausner wrote:
> > On 2012/10/26 22:04:37, regis wrote:
> > > indentation
> > Better?
> 
> I think we use 4 spaces for continuation lines. You have 6 now :-)