-
Notifications
You must be signed in to change notification settings - Fork 866
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
Implement logical assigment operators ||=
and &&=
#1597
Implement logical assigment operators ||=
and &&=
#1597
Conversation
cd22dca
to
75c8b71
Compare
Great stuff! What about ??= through? That is also part of the same proposal, see #938 As for the 'regression': Can you create a case for that (assuming you don't (directly) go off and provide a PR for that one as well :-) |
BEsides the 'regression': seems not all I just looked at language/expressions/logical-assignment/lgcl-or-assignment-operator-lhs-before-rhs.js for example Aren't those fixable? |
I know, but IMHO that is better implemented after #1591 is merged. So, I'd prefer opening a new PR after this and that one are merged.
All of these problems happen also for existing operators, like |
I'm OK with merging this and opening new issues and PRs for the other features if others think this is a good PR. (looks good to me, BTW). Out of time today but it won't be long. Thanks! |
This PR looks good to me as well: have no clue how the limited set of changes actually made things work 🤔, but the relevant tests in test262 pass where they can, so apparently they work 🙂 so I'm all for merging this! |
Thanks for doing this! |
Well, very simply: we already have a lot of logic for operators such as |
Turned out to be pretty simple 🙂
I had to re-number all the entries in
Token
because the number for the new tokens has to land betweenFIRST_ASSIGN
andLAST_ASSIGN
Also, we get two regression in test262 (
lgcl-and-assignment-operator-non-simple-lhs.js
andlgcl-or-assignment-operator-non-simple-lhs.js
), where the tests does something like this:Before, the code was throwing a
SyntaxError
because it did not recognize the||=
operator. Now, it does, and it does not handle correctly the fact that there is a non-simple expression on the left-hand side. However, this behavior is shared with all other assignments operator (e.g.,test() += 32
does not throw aSyntaxError
either), so I don't think this is a real "regression".Partially resolves #936