[V2] Changes to language definition #1516
Conversation
|
OmarTawfik
left a comment
OmarTawfik
left a comment
There was a problem hiding this comment.
Left a few questions/suggestions. Thanks!
| - `HexLiteral` and `YulHexLiteral` and `DecimalLiteral` and `YulDecimalLiteral`: | ||
| - It was illegal for them to be followed by `IdentifierStart`. Now we will produce two separate tokens rather than rejecting it. | ||
|
|
||
| ## Language Definition Changes |
There was a problem hiding this comment.
I suggest renaming this section to Grammar, since the rest of the doc also lists language definition changes:
## Grammar|
|
||
| ### IdentifierPath | ||
|
|
||
| Changed from a simple `Separated` list to a structured format to allow the reserved `address` keyword to appear in identifier paths (but not as the head): |
There was a problem hiding this comment.
I wonder if we are able to just use Separated(MemberAccessIdentifier, Period) for simplicity? We won't need the extra type, given how commmon IdentifierPath is used.
There was a problem hiding this comment.
Also, WDYT of IdentifierPathElement instead of MemberAccessIdentifier? the latter conflicts with the fact that one of its two variants is no longer an identifier.
| The cases where using empty tuples are still ambiguous, `(,,,) = ...` can still be a `TupleDeconstructionStatement` or a | ||
| an `AssignmentExpression` with a `TupleExpression` on the lhs. |
There was a problem hiding this comment.
can still be a
TupleDeconstructionStatementor a anAssignmentExpression
Which one? I wonder if we have existing cst_output tests for this case?
There was a problem hiding this comment.
Also, how about var (,,,,)? this is legal AFAICT.
|
|
||
| This makes certain cases that were allowed before disallowed in V2, in particular having untyped declarations (like `(a, bool b) = ...`) | ||
| or having typed together with `var` (like `var (a, bool b) = ...`). | ||
| The cases where using empty tuples are still ambiguous, `(,,,) = ...` can still be a `TupleDeconstructionStatement` or a |
There was a problem hiding this comment.
For another PR/discussion, we considered merging
TupleDeconstructionStatementandVariableDeclaration, to merge all variable declarations together, however I think this will look a bit artificial since their shape is quite different.
I think having this distinction (declaring new vars VS assigning to existing ones) is worth the destinction, both syntactically and semantically. WDYT of having the following structure, if it works with LALR?
- use
VariableDeclarationStatementfor any syntax that declares a new name:var x = ...already supportedint x = ...already supported- change
VariableDeclarationStatement::namefield to an enum with two variants:name: Identifier-> existingelements-> a struct holdingLeftParen+Separated(elements)+RightParen
- use
AssignmentExpressionfor any syntax that just assigns values to the LHS:x = ....(x, y) = ....(,,,) = ....
Changes to V2 language definition, the main reason is to facilitate creating an LR(1) parser. The more complex ones are:
TupleDeconstructionStatement, making it more strict and with a clear separation betweenvarstyle declarations and explicit ones.IdentifierPath, due to makingaddressa reserved keyword.For another PR/discussion, we considered merging
TupleDeconstructionStatementandVariableDeclaration, to merge all variable declarations together, however I think this will look a bit artificial since their shape is quite different. We can still force it if we consider there's value in it, but I think not worth it for now; they'll probably be joined in one of the passes simplifying the ast.