-
Notifications
You must be signed in to change notification settings - Fork 20
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
Use DSL v2 for parser codegen #650
Conversation
|
664f1ad
to
08a20b6
Compare
...ots/cst_output/ContractDefinition/member_constructor_definition/generated/0.4.22-success.yml
Outdated
Show resolved
Hide resolved
...ty/testing/snapshots/cst_output/Expression/function_call_options/generated/0.6.2-success.yml
Outdated
Show resolved
Hide resolved
...dity/testing/snapshots/cst_output/SourceUnit/end_of_file_trivia/generated/0.8.22-failure.yml
Outdated
Show resolved
Hide resolved
With b5c51ae it's now possible to specify an optional (inline) repeated item, so this removes one more CST difference and solves the last Prettier error, incidentally. |
Part of #652 - Fixes versioning for `UsingDirectiveSymbol` in DSL v1 - Fixes event parameter definition in DSL v2 - Fixes `ElseBranch` sequence fields to be required themselves in DSL v2 - Syncs ordered choice for `(Yul)Statement` across DSLs (helps minimize diff for #650) I'll probably bundle remaining renames and restructures for DSL v2 in #650 already, but wanted to land this here since it fixes some tests and definitions separately from migrating the parser to DSL v2.
a2896e0
to
6db11ac
Compare
Split from #650 We talked about this recently at our stand-up. These are never reserved in the default context and are only used in the (lexical) context of pragmas, so let's reflect that in the spec. Not updating v1 in this context since #650 is about to obsolete that and v0 does not have lexical contexts that might be impacted.
Part of #652 Helps with #650 These make more sense, given that these are the names used in the official grammar (fixed-bytes, signed-integer-type, unsigned-integer-type) and "XXXKeyword" name implies a single atom and not a more complex rule (e.g. looking at the `fixed` definition...). I think we should revert the accidental name change in v2 and later decide if we want to change these. EDIT: This now does the opposite and backports the name change to v0/v1.
0fe06db
to
386c560
Compare
Current codegen relies on the rule to be defined in the same lexical context that the referred to keyword in order for it to be included in this context's trie, so we move the assembly statement up to the default context, so that the `assembly` keyword is picked up.
386c560
to
52f522b
Compare
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.
Looks great! Thank you!
Thanks a lot for the reviews! Here goes nothing... |
Part of #638
Outline
The new definition is used to construct the grammar definition model from the DSL v1, which is then used by the codegen to create the parser and rule/token kinds, like before.
The rough translation done here is to translate every struct/enum/repeated/separated/precedence expression as a rule/non-terminal kind (with the exception of choice nodes, which are inlined, as per #650 (comment)); the rest (i.e. trivia, fragment, token, keywords) are translated as "old" tokens/terminals.
More context
In general, this requires more polish and decision which rules are we comfortable with leaving and fixing leftover issues such as explicit
PrecedenceExpression#rule_name
added to fit the old DSL v1 model/codegen better.I specifically didn't want to touch the v1 codegen/model yet, because it helped being able to incrementally migrate to a new definition and I wanted to limit changes done in a single PR, so that it's easier to review. We can modify the codegen/model to fit the v2 model better once we fully migrate and remove the old DSL v1.
The translation/construction is done at build "run-time" rather than inside the newly introduced definition "compiler" proc macro. There's not much explicit reason for that, other than the fact that it was quicker for me to work on (quicker dev cycle) and the logic is just plain Rust code. This can be moved inside the proc macro later on, if we decide that it's better that way.
Differences
Because the DSL v2 takes a more named and structured approach (as it's designed to be a model for the strongly typed CST/AST), translating it 1-1 to our current parser structure is impossible or even desired.
There are some rules that were introduced and it doesn't make much sense for some to replicate the old behaviour.
TupleValue
(used inTupleValuesList
)TupleMemberDeconstruction
(used inTupleMembersList
)IndexAccessEnd
ImportAlias
UsingAlias
VariableDeclarationValue
Previously inlined sequence parsers now have to be named(fixed with Retrofit{Hex,Decimal}NumberExpression
from v2 into v0/v1 spec #657)-
NumericExpression
is now split into two, now named, choices:HexNumberExpression
andDecimalNumberExpression
Moreover, the following was done to bring back the old CST shape as much as possible:
List
suffix againArgumentsDeclaration
is outlined (as in v0/v1) as wellFunctionCallOptions
is outlined again as a(NamedArgs NamedArgs*)
rather thanNamedArgs | NamedArgs+
to better match the old CST in the singular case (but differs from the old CST in the multiple case)This was done to unify the definitions where possible and to help with reviewing the CST changes by minimizing the shape differences. Once we move off v0 (#637) and v1 (#638), we will be free to change the final shape, since the old definitions will stop tying us down quite a bit.
Outstanding issues
PrecedenceExpression#rule_name
and adapt the codegen model to fit the v2 definition better