When += (and similar operations) are converted to = + ,

When += (and similar operations) are converted to = + ,
we may need to parenthesize the original RHS.

BUG=849
Committed: https://code.google.com/p/dart/source/detail?r=2478

[Jennifer Messerly, 2011-12-15 01:52:12]

lgtm

[jimhug, 2011-12-15 17:28:00]

Thanks for the fix!

In the future, could you be sure to run presubmit.py to generate an updated
minfrog for changes like this?  It's nice to see the changes in the compiler. 
For example, I was very curious about where we would or would not see any extra
parenthesis in the generated js for frog from your change.

[jimhug, 2011-12-15 17:43:18]

http://codereview.chromium.org/8947008/diff/6001/frog/gen.dart
File frog/gen.dart (right):

http://codereview.chromium.org/8947008/diff/6001/frog/gen.dart#newcode1876
frog/gen.dart:1876: TokenKind.infixPrecedence(assignKind)) {
I would prefer not to include this check.  My general rule is that if you need
to think about precedence very hard then it is better to include parens.  This
check is relying on js and dart precedences to be identical - which I would have
to verify before guaranteeing.  Does this change make a big difference to the
generated code?

[dgrove, 2011-12-15 17:51:45]

http://codereview.chromium.org/8947008/diff/6001/frog/gen.dart
File frog/gen.dart (right):

http://codereview.chromium.org/8947008/diff/6001/frog/gen.dart#newcode1876
frog/gen.dart:1876: TokenKind.infixPrecedence(assignKind)) {
It doesn't any changes in minfrog, for instance. It could make some generated JS
a bit less readable, but that could be a reasonable price to pay.

I'm going to take this out, and we can always make this smarter if we need to.
On 2011/12/15 17:43:18, jimhug wrote:
> I would prefer not to include this check.  My general rule is that if you need
> to think about precedence very hard then it is better to include parens.  This
> check is relying on js and dart precedences to be identical - which I would
have
> to verify before guaranteeing.  Does this change make a big difference to the
> generated code?

http://codereview.chromium.org/8947008/diff/6001/frog/gen.dart#newcode1876
frog/gen.dart:1876: TokenKind.infixPrecedence(assignKind)) {
On 2011/12/15 17:43:18, jimhug wrote:
> I would prefer not to include this check.  My general rule is that if you need
> to think about precedence very hard then it is better to include parens.  This
> check is relying on js and dart precedences to be identical - which I would
have
> to verify before guaranteeing.  Does this change make a big difference to the
> generated code?

Done.

[jimhug, 2011-12-15 17:58:03]

http://codereview.chromium.org/8947008/diff/3005/frog/gen.dart
File frog/gen.dart (right):

http://codereview.chromium.org/8947008/diff/3005/frog/gen.dart#newcode1873
frog/gen.dart:1873: } else if ((assignKind != 0) && (node.y is
BinaryExpression)){
Final comment: Please add a TODO or unify with parenthesis adding code in
visitLiteralExpression.  I think that some of those MAY be relevant here as well
- but more significantly, this is hard enough that we should be only doing it in
one place.

[dgrove, 2011-12-15 20:55:51]

http://codereview.chromium.org/8947008/diff/3005/frog/gen.dart
File frog/gen.dart (right):

http://codereview.chromium.org/8947008/diff/3005/frog/gen.dart#newcode1873
frog/gen.dart:1873: } else if ((assignKind != 0) && (node.y is
BinaryExpression)){
On 2011/12/15 17:58:10, jimhug wrote:
> Final comment: Please add a TODO or unify with parenthesis adding code in
> visitLiteralExpression.  I think that some of those MAY be relevant here as
well
> - but more significantly, this is hard enough that we should be only doing it
in
> one place.

Done.

[jimhug, 2011-12-16 16:41:37]

LGTM!