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

Added Non-Associative associativity for Pratt Parsers. #600

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 29 additions & 9 deletions src/pratt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
//! ['binding power'](https://matklad.github.io/2020/04/13/simple-but-powerful-pratt-parsing.html#From-Precedence-to-Binding-Power)
//! that determines how strongly operators should bind to the operands around them.
//!
//! Here is ['another approach'](https://www.engr.mun.ca/~theo/Misc/pratt_parsing.htm) that provides a more comprehensive perspective of handling multiple operators and patterns
//!
//! Pratt parsers are defined with the [`Parser::pratt`] method.
//!
//! When writing pratt parsers, it is necessary to first define an 'atomic' operand used by the parser for building up
Expand Down Expand Up @@ -131,6 +133,8 @@ pub enum Associativity {
Left(u16),
/// Specifies that the operator should be right-associative, with the given binding power (see [`right`]).
Right(u16),
/// Specifies that the operator should be non-associative, with the given binding power.
Non(u16),
}

/// Specifies a left [`Associativity`] with the given binding power.
Expand All @@ -149,18 +153,33 @@ pub fn right(binding_power: u16) -> Associativity {
Associativity::Right(binding_power)
}

/// Specifies a not-associative [`Associativity`] with the given binding power.
///
/// Not-associative operators are evaluated from the left-most terms, moving rightward.
pub fn non(binding_power: u16) -> Associativity {Associativity::Non(binding_power)}

impl Associativity {
fn left_power(&self) -> u32 {
match self {
Self::Left(x) => *x as u32 * 2,
Self::Right(x) => *x as u32 * 2 + 1,
Self::Left(x) => *x as u32 * 3,
Self::Right(x) => *x as u32 * 3,
Self::Non(x) => *x as u32 * 3,
}
}

fn right_power(&self) -> u32 {
match self {
Self::Left(x) => *x as u32 * 2 + 1,
Self::Right(x) => *x as u32 * 2,
Self::Left(x) => *x as u32 * 3 + 1,
Self::Right(x) => *x as u32 * 3,
Self::Non(x) => *x as u32 * 3 + 1,
}
}

fn next_power(&self) -> u32 {
match self {
Self::Left(x) => *x as u32 * 3,
Self::Right(x) => *x as u32 * 3,
Self::Non(x) => *x as u32 * 3 - 1,
Copy link
Owner

Choose a reason for hiding this comment

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

For this to be valid, all powers should have 1 added to them such that *x as u32 * 3 - 1 can never result in underflow.

Copy link

@TheOnlyTails TheOnlyTails Oct 20, 2024

Choose a reason for hiding this comment

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

Would this resolve the issue here?

            Self::Left(x) => *x as u32 * 3 + 1,
            Self::Right(x) => *x as u32 * 3 + 1,
            Self::Non(x) => *x as u32 * 3 - 1,

}
}
}
Expand Down Expand Up @@ -467,7 +486,8 @@ macro_rules! impl_pratt_for_tuple {
// Infix binary operators
$(
let assoc = $X.associativity();
if $X::IS_INFIX && assoc.left_power() >= min_power {
let mut upper_bound: u32 = u32::MAX;
if $X::IS_INFIX && assoc.left_power() >= min_power && assoc.left_power() <= upper_bound {
match $X.op_parser().go::<M>(inp) {
Ok(op) => match recursive::recurse(|| self.pratt_go::<M, _, _, _>(inp, assoc.right_power())) {
Ok(rhs) => {
Expand All @@ -478,6 +498,7 @@ macro_rules! impl_pratt_for_tuple {
$X.fold_infix(lhs, op, rhs, &mut MapExtra::new(pre_expr.offset(), inp))
},
);
upper_bound = assoc.next_power();
continue
},
Err(()) => inp.rewind(pre_op),
Expand Down Expand Up @@ -738,10 +759,9 @@ mod tests {
))
.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").has_errors(),
);
Comment on lines +762 to +764
Copy link
Owner

Choose a reason for hiding this comment

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

This change looks a little suspicious. Does this addition change the behaviour of existing parsers?

}

#[test]
Expand Down
Loading