chumsky icon indicating copy to clipboard operation
chumsky copied to clipboard

Add non-associative infix operators

Open NomAnor opened this issue 7 months ago • 4 comments

I tried to mess with the code and got a working solution. I'm new to rust, so this might not be the best code.

I had to disallow a precedence of zero, which might be a breaking change. Zero is used as a start marker and all implementations I've seen use precedences > 0 when defining the operators.

I also switched the check for associtivity with the operator parsing, which might change the performance?

Now the test pratt::tests::with_pre_and_postfix_ops fails because it contains an ambiguity between the § and + operator, both have a binding power of 4. The comment in that test case indicates that such ambiguity is not a good thing, so maybe this new behaviour is okay? Then the check could just be changed. The old behaviour bound infix operators tighter than prefix operators of the same precedence.

This is a new attempt, the other ones are #600 and #728.

NomAnor avatar May 22 '25 18:05 NomAnor

Hmm, looks like there are CI failures too.

zesterer avatar May 22 '25 19:05 zesterer

Additionally, I'm wondering whether we could avoid breaking binding powers of 0 by casting the u16 up to a i32 and using i32::MIN as the starting marker?

I changed it, a precedence of zero is now ok.

Hmm, looks like there are CI failures too.

The check failures are somwhere else in the code, not sure why they fail. The semver check seems to file because I didn't bump the version, but should this not be done on release? The test failure is because of the pratt::tests::with_pre_and_postfix_ops test.

I made the new result type more generic so every operator can use it. I don't know how to handle error reporting properly. The infix parse returns a fatal error if it finds an operator (non-associative) with the same precedence. This is an ambigious input and should ideally result an an error like "Ambigous operator order, use () to fix it" or something like that. But these error message should be provided by the user and I don't see how to allow that.

The above test fails because the prefix operator "§" and the infix operator "+" have the same binding_power, but "+" is non-associative. I think this should not cause an error because associativity should onyl matter between infix operators. I have to think how to fix that.

NomAnor avatar May 25 '25 10:05 NomAnor

I have to think how to fix that.

Got it, just need to check if the operator is actually non-associative.

What do you thing about the error handling/message?

NomAnor avatar May 25 '25 10:05 NomAnor

Got it, just need to check if the operator is actually non-associative.

Great!

What do you thing about the error handling/message?

I think this is fine for an initial implementation. That said, chumsky supports non-fatal errors. It should be possible for the pratt parser to successfully generate an output while also emitting an error about the use of non-associative operators. You can find an example of this being doing in the validate combinator. You can probably add a new method to LabelError (with a default implementation) for non-associative errors: maybe it could take the spans of the two operators or something?

What do you think?

zesterer avatar May 25 '25 15:05 zesterer

I haven't wrapped my head around the error handling. If I need to use it in my code, I might come back to it. For now I think the base functionality is there.

NomAnor avatar Jun 15 '25 12:06 NomAnor

Thanks! I may make some minor modifications to this post-merge, but I really appreciate all of the work you've put into this!

zesterer avatar Jun 17 '25 19:06 zesterer