fix: parse % as unary only when not followed by a term#3505
Merged
josdejong merged 1 commit intojosdejong:v15from Jul 4, 2025
Merged
fix: parse % as unary only when not followed by a term#3505josdejong merged 1 commit intojosdejong:v15from
josdejong merged 1 commit intojosdejong:v15from
Conversation
This was referenced Jul 3, 2025
Closed
Owner
|
Thanks for working out a solution Glen, well explained. I think this approach makes sense. It is a bit of a pity that we need a try/catch in a "normal" flow. But well, let's stay pragmatic. Merging your fix now in the |
Owner
|
Published now in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes #3501 using the approach suggested in the discussion of that issue: when a % is encountered, the parse now looks to see if there is a valid term just past the %, using only operators with higher precedence than %. If so, it concludes the % must be binary modulus; if not, it interprets the % as unary percentage. It appears that this approach succeeds as hoped.
Note this change flips the interpretation of two prior unit tests:
3%+100was interpreted as(3%) + 100but it is now interpreted as3 mod +100; similarly,3%-100goes from(3%) - 100to3 mod -100. On the other hand, it now properly allows the expression3%~1, evaluating it as3 mod -2(since the value of the bitwise negation~1is -2; this expression was previously a syntax error.Further, this change modifies the syntax error produced by several examples containing
%. In particular, the parser can no longer say that a%operator was unexpected. Since the parser is fine with any two higher-precedence terms with a % in between, an expression like35%%is now no different than35%*--- simply a binary operation with a left-hand operand of35%and missing a right-hand operand. So the syntax error in this sort of case is now that there is a missing value, not an unexpected '%'.It also introduces some redundancy in parsing: when handling e.g.
3%+100, it parses the 3 as a Unary expression, then sees the %. To determine whether this is a UnaryPercentage, it attempts another parseUnary just after the %, which succeeds, as+100is also Unary expression. Since there is a valid right-hand operand, the parser concludes that this % is not a unary percentage operator, so it unwinds the parse, leaving the UnaryPercentage expression as just3. The % is later consumed as a binary modulo operator, after which the parser re-parses the+100, replicating the work done earlier. It was not at all clear to me in the present parsing framework how one could save the work that had been done in parsing+100the first time -- we are not keeping any kind of table as to which expression types start at which character positions, for example.If the modified syntax errors and occasional new parsing redundancy are acceptable, this PR should be ready for review to merge to the v15 branch.
Resolves #3501.