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

Adding a %grammar-kind declaration? #349

Open
ratmice opened this issue Sep 5, 2022 · 2 comments
Open

Adding a %grammar-kind declaration? #349

ratmice opened this issue Sep 5, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@ratmice
Copy link
Collaborator

ratmice commented Sep 5, 2022

Before I try and come up with a patch, I figured it would be good to discuss this in an issue,
I was considering potentially adding a declaration %grammar-kind Original(NoAction), etc

One of the problems with this is that it is likely that we want to parse the value by just using Deserialize on the YaccGrammarKind,
this would at least be the easiest way. But it brings about a few issues

  1. cfgrammar has Optional deserialization support, so if we deserialized that way %grammar-kind would only work with serde feature enabled. Alternately we could just implement this by hand instead of serde?
  2. Some declarations depend upon a specific %grammar-kind, we may have to move some checks from parse_declarations to ast.complete_and_validate.

But it could potentially reduce the number of places that YaccGrammarKind needs to be specified (build.rs, nimbleparse command line, etc). So it seemed like it might be worth considering.

@ratmice ratmice added the enhancement New feature or request label Sep 5, 2022
@ltratt
Copy link
Member

ltratt commented Jan 7, 2023

Oops, I missed this one entirely! I think I'm broadly in favour of this, though we'd have to define what happens if a user specifies YaccGrammarKind and the grammar specifices %grammar-kind. Probably "specifying YaccGrammarKind overrides %grammar-kind" is reasonable?

@ratmice
Copy link
Collaborator Author

ratmice commented Jan 7, 2023

Yeah, that seems like the logical/reasonable choice to me, I'll look into an exploratory patch for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants