Fix two bugs in the Dart VM's super-noSuchMethod invocation.

Fix two bugs in the Dart VM's super-noSuchMethod invocation.

1. The result value of an expression 
"super.someMissingSetter = val" was incorrect in case of a noSuchMethod call.

To fix this I refactored the BuildStaticNoSuchMethod function. It now no longer
creates AST nodes and visits them, because the last argument may be needed saved
as a result value.

2. The evaluation order of "super[e1] = e2" was wrong in case of a noSuchMethod call.

BUG=dart:8917, dart:10965
TEST=tests/language/super_operator_index7_test.dart, tests/language/super_operator_index8_test.dart
R=srdjan@google.com

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

[srdjan, 2013-05-30 17:42:30]

Thanks for fixing all these issues.

https://codereview.chromium.org/15979010/diff/1/runtime/vm/flow_graph_builder.cc
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/15979010/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:1845: bool return_definition = true)
WOuld it be better to have a subclass instead of the flag return_definition?

https://codereview.chromium.org/15979010/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:1878: bool return_definition_;
const

https://codereview.chromium.org/15979010/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:3382: if (save_last_arg && i ==
method_arguments->length() - 1) {
Add parenthesis.

[Florian Schneider, 2013-05-31 08:46:36]

https://codereview.chromium.org/15979010/diff/1/runtime/vm/flow_graph_builder.cc
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/15979010/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:1845: bool return_definition = true)
On 2013/05/30 17:42:30, srdjan wrote:
> WOuld it be better to have a subclass instead of the flag return_definition?

I reconsidered using the constructor/destructor of a scoped object, but decided
to replace it with Enter- and Exit-helper functions.

The scoped object does not provide enough flexibility for the more complex graph
builder functions, like for example the BuildNoSuchMethod helper.

I think the result is as easy to read and use.

https://codereview.chromium.org/15979010/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:1878: bool return_definition_;
On 2013/05/30 17:42:30, srdjan wrote:
> const

Replaced TempLocalScope with EnterScope/ExitScope functions.

[Florian Schneider, 2013-05-31 08:51:19]

https://codereview.chromium.org/15979010/diff/1/runtime/vm/flow_graph_builder.cc
File runtime/vm/flow_graph_builder.cc (right):

https://codereview.chromium.org/15979010/diff/1/runtime/vm/flow_graph_builder...
runtime/vm/flow_graph_builder.cc:3382: if (save_last_arg && i ==
method_arguments->length() - 1) {
On 2013/05/30 17:42:30, srdjan wrote:
> Add parenthesis.

Done.

[Florian Schneider, 2013-05-31 09:06:22]

Adding Slava.

[Florian Schneider, 2013-06-03 14:54:14]

Diff to previous patch-set:

* Simplified flow graph builder to use the parser's helper(
BuildNoSuchMethodArguments).

* Had to change parsed_function_ from a const-ref to a pointer to be able to
call EnsureExpressionTemp in FlowGraphBuilder.

* Some cleanup of expression temp var. (removing unused functions)

[srdjan, 2013-06-03 15:24:14]

https://codereview.chromium.org/15979010/diff/17001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/15979010/diff/17001/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:243: static bool
CompileParsedFunctionHelper(ParsedFunction* parsed_function,
Why this change (from const reference to pointer)? I assume that at some point
you need to modify parse_function, but I could not locate where.

https://codereview.chromium.org/15979010/diff/17001/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:244: bool optimized) {
You could add: 

const Function& function = parsed_function->function();, thus shortening some of
the code below.

[Florian Schneider, 2013-06-03 15:33:48]

https://codereview.chromium.org/15979010/diff/17001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/15979010/diff/17001/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:243: static bool
CompileParsedFunctionHelper(ParsedFunction* parsed_function,
On 2013/06/03 15:24:14, srdjan wrote:
> Why this change (from const reference to pointer)? I assume that at some point
> you need to modify parse_function, but I could not locate where.

I need to call ParsedFunction::EnsureExpressionTemp from the flow graph builder

We only had a const-reference there. Changing it in FlowGraphBuilder has this
ripple-effect on all the rest.

https://codereview.chromium.org/15979010/diff/17001/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:244: bool optimized) {
On 2013/06/03 15:24:14, srdjan wrote:
> You could add: 
> 
> const Function& function = parsed_function->function();, thus shortening some
of
> the code below.

Done.

[srdjan, 2013-06-03 16:56:15]

LGTM

https://codereview.chromium.org/15979010/diff/24003/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/15979010/diff/24003/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:276: Code::Handle(function.unoptimized_code());
Can this fit in one line now?

https://codereview.chromium.org/15979010/diff/24003/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:400:
(parsed_function->function().deoptimization_counter() <
s/parsed_function->function()/function/

https://codereview.chromium.org/15979010/diff/24003/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:489: const Function& function =
parsed_function->function();
Remove

https://codereview.chromium.org/15979010/diff/24003/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:766: ParsedFunction* parsed_function) {
One line?

[Florian Schneider, 2013-06-03 17:01:36]

https://codereview.chromium.org/15979010/diff/24003/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/15979010/diff/24003/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:276: Code::Handle(function.unoptimized_code());
On 2013/06/03 16:56:15, srdjan wrote:
> Can this fit in one line now?

No, still not.

https://codereview.chromium.org/15979010/diff/24003/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:400:
(parsed_function->function().deoptimization_counter() <
On 2013/06/03 16:56:15, srdjan wrote:
> s/parsed_function->function()/function/

Done.

https://codereview.chromium.org/15979010/diff/24003/runtime/vm/compiler.cc#ne...
runtime/vm/compiler.cc:489: const Function& function =
parsed_function->function();
On 2013/06/03 16:56:15, srdjan wrote:
> Remove

Done.

[Florian Schneider, 2013-06-03 17:09:24]

Committed patchset #7 manually as r23553 (presubmit successful).