remove FieldKind from DSL v2
#687
Conversation
|
OmarTawfik
force-pushed
the
remove-field-kinds-from-dsl
branch
from
18b0386 to
8018cea
Compare
Makes the grammar a lot simpler, since it is no longer needed after enums were simplified in NomicFoundation#610
OmarTawfik
force-pushed
the
remove-field-kinds-from-dsl
branch
from
8018cea to
427b8b8
Compare
...testing/snapshots/cst_output/ContractMembers/separated_recovery/generated/0.4.11-failure.yml
Outdated
Show resolved
Hide resolved
| )] | ||
| operators = [ | ||
| PrecedenceOperator( | ||
| model = BinaryLeftAssociative, |
There was a problem hiding this comment.
We introduced UsingOperator above for cases like these (and below). Should we do it here as well?
There was a problem hiding this comment.
UsingOperator is used in a comma separated list, and it means something on its own.
The others here are part of a parent expression, and I didn't want to wrap it in an unnecessary parent. It cannot exist in any other place in the grammar.
WDYT?
There was a problem hiding this comment.
Do you mean UsingDeconstructionSymbol? UsingOperator was introduced in this PR IIUC and only used in UsingAlias.
I agree with the unnecessary wrapping argument though. The win in this PR was to remove the redundant Required/Optional elsewhere but in cases like these we're replacing 15 lines with 50. Do you think we could simplify this somehow in the future?
There was a problem hiding this comment.
My bad. I mixed the two. I think this change made the syntax more coincise in the 98% of the cases, and worth it to sacrifice a few extra lines in the remaining 2%. As far as the output is concerned, it will eventually be identical (once migration is complete).
But happy to reconsider if you feel otherwise.
| )] | ||
| operators = [ | ||
| PrecedenceOperator( | ||
| model = BinaryLeftAssociative, |
There was a problem hiding this comment.
Do you mean UsingDeconstructionSymbol? UsingOperator was introduced in this PR IIUC and only used in UsingAlias.
I agree with the unnecessary wrapping argument though. The win in this PR was to remove the redundant Required/Optional elsewhere but in cases like these we're replacing 15 lines with 50. Do you think we could simplify this somehow in the future?
Part of #638 This PR: - Removes the `PrecedenceExpression::rule_name` in favour of using the name directly for its rule/node kind - Deduplicates the individual precedence expressions in the parser codegen (regressed in #687 (comment)) - Allows to parse individual precedence expressions and adds rule kinds for them - ... so we can remove `ProductionKind` in favour of the now-complete `RuleKind` I can split this up but since all of this is interconnected, it made sense to work on them at the same time and submit work that solves all of the aforementioned issues. --------- Co-authored-by: Omar Tawfik <[email protected]>
Makes the grammar a lot simpler, since it is no longer needed after enums were simplified in #610