Skip to content
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

Clean up precedence parser codegen since DSL v2 #699

Merged
merged 18 commits into from
Dec 9, 2023

Conversation

Xanewok
Copy link
Contributor

@Xanewok Xanewok commented Dec 6, 2023

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 remove FieldKind from DSL v2 #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.

Copy link

changeset-bot bot commented Dec 6, 2023

🦋 Changeset detected

Latest commit: 04a6e59

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@nomicfoundation/slang Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

This introduced individual rule kinds per each precedence expression and
we lose the grouping rules such as BinaryExpression. Not sure how
desirable they were in the first place but it seems better and easier to
handle specific expressions rather than using grouping rule kinds like
before.
Since it's the only place it's used and it's a small helper.
This also fixes a run-time regression introduced with duplicate operator
parsers when migrating to v2.

To support different associativity/models for precedence expressions
across multiple versions, the parser generator now:
- groups the operators by model for a given precedence expression
- defines a sub-parser for (expression, model) as a choice over the
  operators with a given model
- each of the precedence sub-parses reduces to the parent precedence
  expression rule kind.
This lets us completely get rid of the ProductionKind, as we support
parsing every `RuleKind` now.
@Xanewok Xanewok marked this pull request as ready for review December 8, 2023 21:58
@Xanewok Xanewok requested a review from a team as a code owner December 8, 2023 21:58
@OmarTawfik OmarTawfik mentioned this pull request Dec 8, 2023
7 tasks
Copy link
Collaborator

@OmarTawfik OmarTawfik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. Left a few suggestions/questions.

Copy link
Collaborator

@OmarTawfik OmarTawfik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One question remaining about snapshot tests.

@Xanewok Xanewok added this pull request to the merge queue Dec 9, 2023
Merged via the queue into NomicFoundation:main with commit ddfebfe Dec 9, 2023
1 check passed
@Xanewok Xanewok deleted the precedence-cleanup branch December 9, 2023 23:24
@github-actions github-actions bot mentioned this pull request Dec 9, 2023
github-merge-queue bot pushed a commit that referenced this pull request Dec 14, 2023
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and publish to npm
yourself or [setup this action to publish
automatically](https://github.com/changesets/action#with-publishing). If
you're not ready to do a release yet, that's fine, whenever you add more
changesets to main, this PR will be updated.


# Releases
## @nomicfoundation/slang@0.12.0

### Minor Changes

- [#699](#699)
[`ddfebfe9`](ddfebfe)
Thanks [@Xanewok](https://github.com/Xanewok)! - Remove `ProductionKind`
in favor of `RuleKind`

- [#699](#699)
[`ddfebfe9`](ddfebfe)
Thanks [@Xanewok](https://github.com/Xanewok)! - Allow parsing
individual precedence expressions, like `ShiftExpression`

- [#665](#665)
[`4b5f8b46`](4b5f8b4)
Thanks [@Xanewok](https://github.com/Xanewok)! - Remove the CST Visitor
API in favor of the Cursor API

- [#666](#666)
[`0434b68c`](0434b68)
Thanks [@Xanewok](https://github.com/Xanewok)! - Add `Node::unparse()`
that allows to reconstruct the source code from the CST node

- [#675](#675)
[`daea4b7f`](daea4b7)
Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - rename `Cursor`'s
`pathRuleNodes()` to `ancestors()` in the NodeJS API.

- [#676](#676)
[`b496d361`](b496d36)
Thanks [@OmarTawfik](https://github.com/OmarTawfik)! - Fix NAPI `cursor`
types and expose `cursor.depth`.

### Patch Changes

- [#685](#685)
[`b5fca94a`](b5fca94)
Thanks [@Xanewok](https://github.com/Xanewok)! - `bytes` is now properly
recognized as a reserved word

- [#660](#660)
[`97028991`](9702899)
Thanks [@Xanewok](https://github.com/Xanewok)! - Drop List suffix from
collection grammar rule names

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
OmarTawfik added a commit that referenced this pull request Dec 21, 2023
github-merge-queue bot pushed a commit that referenced this pull request Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants