Allow tuples with a single element #888
Conversation
|
The grammar shows that a keySetExpression is basically a comma-separated list of one or more simpleKeySetExpressions. tupleKeySetExpression has parentheses around it. It seems somewhat odd that parentheses are optional. Maybe p4c checks in certain contexts that they must be present, and must be absent in others? I haven't dug into that very far. |
|
I don't remember why we haven't merged this one. |
| @@ -178,14 +178,22 @@ keysetExpression | |||
| ; | |||
|
|
|||
| tupleKeysetExpression | |||
| : '(' simpleKeysetExpression ',' simpleExpressionList ')' | |||
| : "(" simpleKeysetExpression "," simpleExpressionList ")" | |||
| | "(" reducedSimpleKeysetExpression ")" | |||
There was a problem hiding this comment.
Why not use simpleKeysetExpression here? What is the need for introducing reducedSimpleKeysetExpression?
There was a problem hiding this comment.
reducedKeysetExpression is missing a case, expression
| | DEFAULT | ||
| | "_" | ||
| ; | ||
|
|
There was a problem hiding this comment.
If there is a reason to introduce reducedSimpleKeysetExpression, it seems reasonable for humans reviewing and understanding this to make its definition as similar to simpleKeysetExpression as possible, e.g. something like "DEFAULT | DONTCARE | expression MASK expression | expression RANGE expression"
|
We discussed at the March 1st, 2021 LDWG meeting. @jafingerhut agreed to shepherd it and @mbudiu-vmw will check consistency with the |
|
I can confirm that this grammar matches the compiler; there is some duplication for avoiding conflicts. |
|
@mbudiu-vmw I have carefully compared this modified grammar with the latest p4c, and agree that it matches in regards to the changes here. I will resolve all of my comments/questions on this PR, and approve it for merging. |
|
@mbudiu-vmw Rather, I thought I was going to resolve my comments, but I don't see the button for Github to allow me to do that. I have earlier approved this PR, so if there is anything else you would like me to do to approve this PR, let me know. |
|
Do you have 2 distinct github accounts? |
|
I do, @jfingerh and @jafingerhut, which I unfortunately confuse myself with, as well as others. Sorry about that. |
See p4lang/p4c#2518 and p4lang/p4c#2514