Fix parentheses removal in nullish-coalescing operation#11014
Fix parentheses removal in nullish-coalescing operation#11014nicolo-ribaudo merged 4 commits intobabel:masterfrom
Conversation
|
@nicolo-ribaudo Made a draft PR for working on #10966 . I tried adding precedence to |
|
@nicolo-ribaudo Its working. The problem was that I was not running Also, I have assigned precedence 0 to it since https://tc39.es/ecma262/#sec-binary-logical-operators mentions |
| @@ -2,6 +2,7 @@ import * as t from "@babel/types"; | |||
|
|
|||
| const PRECEDENCE = { | |||
| "||": 0, | |||
There was a problem hiding this comment.
While it's unobservable now, ?? should actually be the lowest precedence of all tokens. Meaning we'll need to increase every other token's precedence.
There was a problem hiding this comment.
Ok, so I am assigning a precedence of 0 to ?? and will increment that of others by 1. BTW, I am still not clear as to how to get to know the precedence that we follow here.
There was a problem hiding this comment.
Oh, actually, you're right. SortCircuitExpression is both CoalesceExpression and LogicalORExpression, so they have the same precedence. I was remember when we had the spec using grammar flags, and there it was CoalesceExpression -> LogicalORExpression.
There was a problem hiding this comment.
Oh, I had already made the changes. 😅 Reverting them.
| @@ -0,0 +1,2 @@ | |||
| const foo = 'test'; | |||
| console.log(foo ?? '' == ''); No newline at end of file | |||
There was a problem hiding this comment.
Don't forget to update the test
There was a problem hiding this comment.
Oops. My bad. Making the changes.
38bf8f0 to
70da59b
Compare
|
@jridgewell Made the changes. PTAL. Thanks. |
There was a problem hiding this comment.
Thanks for this PR!
BTW, I am still not clear as to how to get to know the precedence that we follow here.
You can check the correct precedence in the spec.
Consider this example (directly from the https://tc39.es/ecma262).
AdditiveExpression:
MultiplicativeExpression
AdditiveExpression + MultiplicativeExpression
AdditiveExpression - MultiplicativeExpression
MultiplicativeExpression:
ExponentiationExpression
MultiplicativeExpression MultiplicativeOperator ExponentiationExpression
MultiplicativeOperator: one of
* / %
ExponentiationExpression:
UnaryExpression
UpdateExpression ** ExponentiationExpression
You have to trust me about this: the spec starts "parsing" from AdditiveExpression. If you want, you can look for the Expression production in the spec and try to understand why.
Now, suppose that we are trying to parse the following expression: a * b + c + d * e.
We first look for an AdditiveExpression. It can start with another AdditiveExpression, but it will ultimately go down and look for a MultiplicativeExpression.
We start parsing the nested MultiplicativeExpression, and go down to ExponentiationExpression, which then goes down to UnaryExpression. Trust me or try to read the spec, but both UnaryExpression and UpdateExpression go down to Identfier and parse a.
At this point, we start going "up" and go back to ExponentiationExpression: we already have something matched by either UnaryExpression or UpdateExpression. We check if there is **: there isn't, so we "return" a to MultiplicativeExpression. Then, MultiplicativeExpression either parses one of *, /, or %, or "returns up" to AdditiveExpression. There is the * character, so it consumes it and then parser another ExponentiationExpression: we now have a * b.
Now, we have a MultiplicativeExpression: it can either be the left operand of another MultiplicativeExpression, or "return up" to AdditiveExpression. We don't have one of *, /, or % anymore, so it returns.
The MultiplicativeExpression is now used as the left operand of the AdditiveExpression, because there is the + token. We have MultiplicativeExpression + ((a * b) + ), and need to look for its right operand which is specified as a MultiplicativeExpression.
We go down and parse the c identifier, and then go back in the grammar productions up to MultiplicativeExpression. We don't have one of * / %, so it "returns" c as the right operand of the AdditiveExpression: we now have (a * b) + c.
This AdditiveExpression can either be finished or be the left operand of another AdditiveExpression. The parser finds the + character, so it falls into the second case. We now have ((a * b) + c) +, and start parsing a MultiplicativeExpression: following the same logic as before, the MultiplicativeExpression is (d * e).
(d * e) is returned to the main AdditiveExpression: we now have ((a * b) + c) + (d * e).
You can see that * has an higher precedence than +, because when there aren't parentheses * is parsed exactly as if it was wrapped.
I also copied the ExponentiationExpression production from the spec because it gives us the opportunity to see another property of binary operators: their associativity. In this case, AdditiveExpression and MultiplicativeExpression are left-associative, while ExponentiationExpression is right-associative.
I leave to you to understand why 😉
|
@nicolo-ribaudo Thanks a lot for such a detailed explanation. I was confused earlier but not now. And yes, I fully trust your words, no need to read the spec. 😉 |
Uh oh!
There was an error while loading. Please reload this page.