Skip to content

fix: parse % as unary only when not followed by a term#3505

Merged
josdejong merged 1 commit intojosdejong:v15from
gwhitney:fix_3501
Jul 4, 2025
Merged

fix: parse % as unary only when not followed by a term#3505
josdejong merged 1 commit intojosdejong:v15from
gwhitney:fix_3501

Conversation

@gwhitney
Copy link
Copy Markdown
Collaborator

@gwhitney gwhitney commented Jul 3, 2025

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%+100 was interpreted as (3%) + 100 but it is now interpreted as 3 mod +100; similarly, 3%-100 goes from (3%) - 100 to 3 mod -100. On the other hand, it now properly allows the expression 3%~1, evaluating it as 3 mod -2 (since the value of the bitwise negation ~1 is -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 like 35%% is now no different than 35%* --- simply a binary operation with a left-hand operand of 35% 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 +100 is 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 just 3. 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 +100 the 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.

@josdejong
Copy link
Copy Markdown
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 v15 branch.

@josdejong josdejong merged commit b44172b into josdejong:v15 Jul 4, 2025
8 checks passed
@josdejong
Copy link
Copy Markdown
Owner

Published now in v15.0.0.

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