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

Attempt At Non-Infix Pratt Parsing #728

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

Conversation

lancylot2004
Copy link

Along the same lines as #600.

Currently, the chumsky::pratt parser only supports binary infix operators with left or right associativity, but not non-infix ones. Other libraries and literature might term this as InfixN or non-associative. (Please correct me if I'm wrong~)

For example, comparison operators like > and == are typically non-associative, since expressions like 1 == 2 == 3 is invalid. (They cannot be chained.)

This PR adds a non option for associativity, and tries to follow the same article.

@lancylot2004
Copy link
Author

Important

The PR for now breaks one of the postfix tests - I wanted to open it just in case someone who knows more than me would want to take a look!

@zesterer
Copy link
Owner

zesterer commented Feb 9, 2025

Thanks for the PR, should be able to take a look at this soon.

Comment on lines +1311 to +1322
let parser = atom
.pratt((
// -- Infix
infix(left(1), just('+'), |l, _, r, _| i(Expr::Add, l, r)),
infix(non(2), just('*'), |l, _, r, _| i(Expr::Mul, l, r)),
))
.map(|x| x.to_string());
assert_eq!(
parser.parse("1+2*3").into_result(),
Ok("(1 + (2 * 3))".to_string())
);
assert!(parser.parse("1+2*3*3").has_errors());
Copy link
Owner

Choose a reason for hiding this comment

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

I think it would be a good idea to check that 1+2*3*5 fails here too. assert!(parser.parse(...).has_errors()) should be sufficient.

Copy link
Author

Choose a reason for hiding this comment

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

Added!

@lancylot2004
Copy link
Author

Do you have any idea why the current changes break the postfix test?

Comment on lines +499 to 503
fn right_power(&self) -> u32 {
match self {
Self::Left(x) => *x as u32 * 2,
Self::Right(x) => *x as u32 * 2 + 1,
&Self::Left(x) | &Self::Non(x) => x as u32 * 3 + 2,
&Self::Right(x) => x as u32 * 3 + 1,
}
Copy link
Owner

@zesterer zesterer Feb 10, 2025

Choose a reason for hiding this comment

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

This doesn't seem to match up with the table above it. Perhaps this is the cause of the test failure?

Copy link
Author

Choose a reason for hiding this comment

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

Apologies I should've changed the table - the values in the table don't work either!

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.

2 participants