Fix wrong order of execution in compound assignment nodes (located by Kevin).

Fix wrong order of execution in compound assignment nodes (located by Kevin). 
Committed: https://code.google.com/p/dart/source/detail?r=7173

[hausner, 2012-05-01 00:31:13]

LGTM but I think this code is really hard to understand. I don't have an obvious
improvement though.

https://chromiumcodereview.appspot.com/10270028/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://chromiumcodereview.appspot.com/10270028/diff/1/runtime/vm/parser.cc#n...
runtime/vm/parser.cc:6035: // to control sideeffects, the store part stores any
sideffect creating inputs
inputs with potential side effects

https://chromiumcodereview.appspot.com/10270028/diff/1/runtime/vm/parser.cc#n...
runtime/vm/parser.cc:6096: // Compound assignment: store inputs with sideffects
into temporary locals.
side effect

[srdjan, 2012-05-01 00:35:23]

https://chromiumcodereview.appspot.com/10270028/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://chromiumcodereview.appspot.com/10270028/diff/1/runtime/vm/parser.cc#n...
runtime/vm/parser.cc:6035: // to control sideeffects, the store part stores any
sideffect creating inputs
On 2012/05/01 00:31:14, hausner wrote:
> inputs with potential side effects

Done.

https://chromiumcodereview.appspot.com/10270028/diff/1/runtime/vm/parser.cc#n...
runtime/vm/parser.cc:6096: // Compound assignment: store inputs with sideffects
into temporary locals.
On 2012/05/01 00:31:14, hausner wrote:
> side effect

Done.

[srdjan, 2012-05-01 00:36:30]

On 2012/05/01 00:31:13, hausner wrote:
> LGTM but I think this code is really hard to understand. I don't have an
obvious
> improvement though.

Me neither. The alternative was to add  compound nodes and we did not like that
neither.

> 
> https://chromiumcodereview.appspot.com/10270028/diff/1/runtime/vm/parser.cc
> File runtime/vm/parser.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10270028/diff/1/runtime/vm/parser.cc#n...
> runtime/vm/parser.cc:6035: // to control sideeffects, the store part stores
any
> sideffect creating inputs
> inputs with potential side effects
> 
>
https://chromiumcodereview.appspot.com/10270028/diff/1/runtime/vm/parser.cc#n...
> runtime/vm/parser.cc:6096: // Compound assignment: store inputs with
sideffects
> into temporary locals.
> side effect

[Kevin Millikin (Google), 2012-05-01 08:40:13]

https://chromiumcodereview.appspot.com/10270028/diff/1003/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://chromiumcodereview.appspot.com/10270028/diff/1003/runtime/vm/parser.c...
runtime/vm/parser.cc:6099: expr = ModifyCompoundAssignmentLoad(left_expr);
Cute.

ModifyCompoundAssignmentLoad is only safe if the assignments found in the
subexpressions of left_expr are the ones introduced by AsSideEffectFreeNode, and
not ones written by the programmer.

We could force that by combining the two functions with a combined API like:

left_expr = Modify...(&expr)

or else

expr = Modify...(&left_expr).

[srdjan, 2012-05-01 16:27:55]

https://chromiumcodereview.appspot.com/10270028/diff/1003/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://chromiumcodereview.appspot.com/10270028/diff/1003/runtime/vm/parser.c...
runtime/vm/parser.cc:6099: expr = ModifyCompoundAssignmentLoad(left_expr);
On 2012/05/01 08:40:13, kmillikin wrote:
> Cute.
> 
> ModifyCompoundAssignmentLoad is only safe if the assignments found in the
> subexpressions of left_expr are the ones introduced by AsSideEffectFreeNode,
and
> not ones written by the programmer.
> 
> We could force that by combining the two functions with a combined API like:
> 
> left_expr = Modify...(&expr)
> 
> or else
> 
> expr = Modify...(&left_expr).

Good point, will combine them.