Accept a multiple assignment in a rescue modifier value#4128
Draft
kai-matsudate wants to merge 2 commits into
Draft
Accept a multiple assignment in a rescue modifier value#4128kai-matsudate wants to merge 2 commits into
kai-matsudate wants to merge 2 commits into
Conversation
A rescue modifier value is a statement in the grammar, so it may be a
multiple assignment, which parse.y accepts but prism rejected:
a rescue b, c = 1 # parse.y: Syntax OK, prism: unexpected ','
It is parsed below the statement level where multiple assignment
detection starts, and lowering the binding power would absorb statement
modifiers too, so set a flag while reading the value instead.
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.
Fixes Bug #22095.
The value of a rescue modifier is a statement in the grammar and may itself be a multiple assignment (
a rescue b, c = 1). parse.y accepts this, but Prism rejected it. The cause is that multiple-assignment detection is only enabled at the statement level (PM_BINDING_POWER_STATEMENT), while the value of a rescue modifier is parsed with the rescue modifier's right binding power (PM_BINDING_POWER_COMPOSITION).Changes
Lowering the binding power so that the value is parsed as a statement would allow the parser to recognize the multiple assignment, but it would also include statement modifiers with lower precedence than
rescue(PM_BINDING_POWER_MODIFIER) in the value. Thena rescue b if cwould parse asa rescue (b if c)instead of(a rescue b) if c. parse.y keeps these concerns separate: precedence keeps statement modifiers out of the value, while a grammar rule allows a multiple assignment in the value. This PR follows the same split.This adds a flag,
PM_PARSE_ACCEPTS_MULTIPLE_ASSIGNMENT, and a predicate macro,PM_PARSE_MULTIPLE_ASSIGNMENT_P, which is true at the statement level or when the flag is set. The checks that decide whether a multiple assignment may start now use this macro: the prefix cases (identifier / instance, global, class variable / constant / leading splat), the infix cases (./[]/::), and the multi-value/splat checks inparse_assignment_values.The flag is set while reading the value of a rescue modifier. The binding power remains
PM_BINDING_POWER_COMPOSITION, which keeps statement modifiers out of the value; the flag enables multiple-assignment parsing for that value without lowering the binding power.The flag is set in two places:
a rescue b), it is passed unconditionally to theparse_expressionthat reads the value. The flag is set before parsing the value begins, so the rescue value can start with a multiple assignment target.parse_assignment_values(a rescue on the right-hand side of an assignment), it is set only when the binding power is that of a multiple assignment right-hand side (PM_BINDING_POWER_MULTI_ASSIGNMENT + 1).The latter is conditional because parse.y allows the rescue value to be a multiple assignment only on the right-hand side of a multiple assignment (
x, y = a rescue b, c = 1), not of a single assignment (x = a rescue b, c = 1). The grammar rule that allows a multiple assignment in a rescue value (mlhs '=' lex_ctxt mrhs_arg modifier_rescue ... stmt) is only reachable from the multiple-assignment left-hand side (mlhs). An endless method definition (def f = a rescue b, c = 1) reads its value through a separate path inparse_defand does not set the flag, so it is rejected as in parse.y as well.Out of scope
A parenthesized group as the first target (
a rescue (b, c), d = 1) is intentionally left out of this PR. Supporting it requires threading the flag throughparse_parentheses, but doing so changes how the opening parenthesis is classified (tLPAREN/tLPAREN2) in the whitequark/parser-compatible translation (lib/prism/translation/parser). That compatibility needs separate handling, so I plan to open a separate PR for it.Tests
test/prism/fixtures/rescue_modifier.txt.=case and the single-assignment right-hand side case totest/prism/errors/. I confirmed that parse.y also rejects these withruby --parser=parse.y -c(therefute_valid_syntaxintest/prism/errorschecks the parse.y-side rejection only when the default parser is parse.y).Note
The asymmetry where a multiple assignment in a rescue value is not allowed on a single-assignment right-hand side (
x = 1 rescue a, b = 1is rejected, butx, y = 1 rescue a, b = 1is accepted) seems to come from the parse.y grammar itself; I plan to file a separate issue for it.