-
-
Notifications
You must be signed in to change notification settings - Fork 157
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
Added support for pratt parsers with up to 676 operators #682
Conversation
Hmm, I think this is likely to have a non-trivial impact on compilation time. Remember that the macro system still needs to generate all of those impls and the type system needs to check them, even if they go unused. I'm happy to see the number of operators supported by |
Agreed, I tested it out and it crashed my editor when running clippy, not to mention having to increase the recursion limit for macro expansion. |
Maybe it could be supported with nested tuples? but that might be too hacky |
It might be a possibility, yes. |
I'll see if I can make that a reality. |
); | ||
|
||
#[allow(unused_variables, non_snake_case)] | ||
impl<'a, Atom, T, const N: usize> Pratt<Atom, [T; N]> { |
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.
I don't think this is doing what you think it's doing, unfortunately. This requires that all operators are of exactly the same time, which is almost useless. For this to be useful you'd need to implement Operator
for Box<dyn Operator>
, but then the code below like T::IS_PREFIX
and others would need to change to check what kind of operator it is at run time.
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.
Ah, that sucks. I tried looking at the choice
combinator to use the same tuple nesting logic, but unfortunately macro syntax still flies over my head.
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.
Yeah, there's a good reason I bounced off this problem the first time round. Maybe I'll give it another go soon.
Closing this since #685 added support for nested pratt operators. |
Adds support for 676-operator pratt parsers (AA..=ZZ)
(this took exactly 5 minutes using multi-cursors)
Closes #658