-
Notifications
You must be signed in to change notification settings - Fork 677
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Join assignment #226
base: master
Are you sure you want to change the base?
Join assignment #226
Conversation
return [ this[0], op, walk(lvalue), walk(rvalue) ]; | ||
rvalue = walk(rvalue) | ||
lvalue = walk(lvalue) | ||
return [ this[0], op, lvalue, rvalue ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you explain this change to me? Why do you have to walk the rvalue
before the lvalue
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michaelficarra consider this example:
a.b = 3;
c.d = 3;
If I'd walk lvalue first, c.d
would prevent me from joining those two because my code would (incorrectly) assume that c.d
gets evaluated before the second 3
and therefore could trigger a setter that alters state or so. My code depends on the walkers going through the code in the order the code would be evaluated in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, alright. Thanks.
This one is buggy: // problem 1: this.foo is dropped
this.foo = 0;
this.bar = 0;
this.baz = 0;
// problem 2: b -= b += 5?
if (a < 0) b += 5;
else b -= 5; ===> uglified: this.baz = this.bar = 0, a < 0 && (b -= b += 5); |
Fixed one easy bug, only simple assignments ( |
See #223 - basically, this is about joining assignment statements into expressions when appropriate: