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

Macro Constructors #151

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Macro Constructors #151

wants to merge 2 commits into from

Conversation

CrockAgile
Copy link
Collaborator

@CrockAgile CrockAgile commented Nov 26, 2024

What

Adds macro constructors for each of the base BNF types (terms/expressions/productions/grammars)

let dna_grammar = bnf::grammar! {
  <dna> ::= <base> | <base> <dna>;
  <base> ::= 'A' | 'C' | 'G' | 'T';
};

How

Via not very easy to read, and hard to test macro_rules 😅

Why

  • sometimes writing a macro is nice because it is shorter than <prod> ::= 'A'".parse()
  • it is definitely shorter than the *::from_parts constructors

Why Not

  • macro_rules are hard to reason about, debug, and test
  • maybe offering multiple ways to do the same thing is confusing to users
    • for example, because whitespace is not a token, semicolons are a hard requirement for parsing a Grammar via macros

Future Dreams

The main motivation for this was to see if maybe a const constructor would be possible. The current macros do not support that due to vector mutations, but I think it may be doable. Unfortunately, a full macro const constructor would require:

  • Terms to hold Cow<str>, not String
  • Grammar/Prod/Expression would need to hold Cow<[T]>, not Vec<T>

This is doable, but would definitely introduce many breaking changes.

Open Questions

  • is this more useful than confusing?
  • is the future dream of a const constructor worth breaking changes?

@coveralls
Copy link

coveralls commented Nov 26, 2024

Coverage Status

coverage: 98.283% (+0.06%) from 98.228%
when pulling bdde346 on macro-constructor
into 4b433e8 on main.

@@ -1,3 +1,5 @@
#![allow(clippy::vec_init_then_push)]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

maybe this warrants moving all macros to their own mod ?

@shnewto
Copy link
Owner

shnewto commented Nov 27, 2024

oh wow! first impression is that it's a very cool concept and the dream of a const constructor is one I'd like to see realized too ❤️
i appreciate the why nots laid out, requiring semi-colon's is probably what gives me the most pause... I sort of want to think about that bit some more in case there's something we could do to live without them... like maybe just make newlines optional in grammars altogether?

@shnewto
Copy link
Owner

shnewto commented Nov 27, 2024

also I wonder if something like like maud or inline-python are doing something interesting to parse significant whitespace?

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.

3 participants