From fb1328e4676da4dfc174ccadc57899e2eac96a63 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 14 Sep 2023 11:19:20 -0400 Subject: [PATCH] [ruby/yarp] Fix multi target parentheses locations https://github.com/ruby/yarp/commit/7f71527522 --- test/yarp/location_test.rb | 2 + .../seattlerb/masgn_double_paren.txt | 10 ++--- .../snapshots/whitequark/masgn_nested.txt | 10 ++--- yarp/yarp.c | 45 +++++++++---------- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/test/yarp/location_test.rb b/test/yarp/location_test.rb index a452dfad11..d76fac5a80 100644 --- a/test/yarp/location_test.rb +++ b/test/yarp/location_test.rb @@ -556,6 +556,8 @@ module YARP def test_MultiWriteNode assert_location(MultiWriteNode, "foo, bar = baz") + assert_location(MultiWriteNode, "(foo, bar) = baz") + assert_location(MultiWriteNode, "((foo, bar)) = baz") end def test_NextNode diff --git a/test/yarp/snapshots/seattlerb/masgn_double_paren.txt b/test/yarp/snapshots/seattlerb/masgn_double_paren.txt index 3e4fc9af03..d2f8b358c9 100644 --- a/test/yarp/snapshots/seattlerb/masgn_double_paren.txt +++ b/test/yarp/snapshots/seattlerb/masgn_double_paren.txt @@ -1,9 +1,9 @@ -@ ProgramNode (location: (1...9)) +@ ProgramNode (location: (0...9)) ├── locals: [:a, :b] └── statements: - @ StatementsNode (location: (1...9)) + @ StatementsNode (location: (0...9)) └── body: (length: 1) - └── @ MultiWriteNode (location: (1...9)) + └── @ MultiWriteNode (location: (0...9)) ├── targets: (length: 1) │ └── @ MultiTargetNode (location: (1...6)) │ ├── targets: (length: 2) @@ -15,8 +15,8 @@ │ │ └── depth: 0 │ ├── lparen_loc: (1...2) = "(" │ └── rparen_loc: (5...6) = ")" - ├── lparen_loc: ∅ - ├── rparen_loc: ∅ + ├── lparen_loc: (0...1) = "(" + ├── rparen_loc: (6...7) = ")" ├── operator_loc: (7...8) = "=" └── value: @ CallNode (location: (8...9)) diff --git a/test/yarp/snapshots/whitequark/masgn_nested.txt b/test/yarp/snapshots/whitequark/masgn_nested.txt index 30982bf9b7..3a3f07d09b 100644 --- a/test/yarp/snapshots/whitequark/masgn_nested.txt +++ b/test/yarp/snapshots/whitequark/masgn_nested.txt @@ -1,9 +1,9 @@ -@ ProgramNode (location: (1...30)) +@ ProgramNode (location: (0...30)) ├── locals: [:b, :a, :c] └── statements: - @ StatementsNode (location: (1...30)) + @ StatementsNode (location: (0...30)) └── body: (length: 2) - ├── @ MultiWriteNode (location: (1...13)) + ├── @ MultiWriteNode (location: (0...13)) │ ├── targets: (length: 1) │ │ └── @ MultiTargetNode (location: (1...6)) │ │ ├── targets: (length: 2) @@ -15,8 +15,8 @@ │ │ │ └── expression: ∅ │ │ ├── lparen_loc: (1...2) = "(" │ │ └── rparen_loc: (5...6) = ")" - │ ├── lparen_loc: ∅ - │ ├── rparen_loc: ∅ + │ ├── lparen_loc: (0...1) = "(" + │ ├── rparen_loc: (6...7) = ")" │ ├── operator_loc: (8...9) = "=" │ └── value: │ @ CallNode (location: (10...13)) diff --git a/yarp/yarp.c b/yarp/yarp.c index 7c02967617..5f6eb0a48c 100644 --- a/yarp/yarp.c +++ b/yarp/yarp.c @@ -11298,28 +11298,27 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) { parser_lex(parser); yp_accepts_block_stack_pop(parser); - // If we have a single statement and are ending on a right parenthesis, - // then we need to check if this is possibly a multiple target node. + // If we have a single statement and are ending on a right + // parenthesis, then we need to check if this is possibly a + // multiple target node. if (binding_power == YP_BINDING_POWER_STATEMENT && YP_NODE_TYPE_P(statement, YP_MULTI_TARGET_NODE)) { - yp_node_t *target; - yp_multi_target_node_t *multi_target = (yp_multi_target_node_t *) statement; + yp_multi_target_node_t *multi_target; + if (((yp_multi_target_node_t *) statement)->lparen_loc.start == NULL) { + multi_target = (yp_multi_target_node_t *) statement; + } else { + multi_target = yp_multi_target_node_create(parser); + yp_multi_target_node_targets_append(multi_target, statement); + } yp_location_t lparen_loc = YP_LOCATION_TOKEN_VALUE(&opening); yp_location_t rparen_loc = YP_LOCATION_TOKEN_VALUE(&parser->previous); - if (multi_target->lparen_loc.start == NULL) { - multi_target->base.location.start = lparen_loc.start; - multi_target->base.location.end = rparen_loc.end; - multi_target->lparen_loc = lparen_loc; - multi_target->rparen_loc = rparen_loc; - target = (yp_node_t *) multi_target; - } else { - yp_multi_target_node_t *parent_target = yp_multi_target_node_create(parser); - yp_multi_target_node_targets_append(parent_target, (yp_node_t *) multi_target); - target = (yp_node_t *) parent_target; - } + multi_target->lparen_loc = lparen_loc; + multi_target->rparen_loc = rparen_loc; + multi_target->base.location.start = lparen_loc.start; + multi_target->base.location.end = rparen_loc.end; - return parse_targets(parser, target, YP_BINDING_POWER_INDEX); + return parse_targets(parser, (yp_node_t *) multi_target, YP_BINDING_POWER_INDEX); } // If we have a single statement and are ending on a right parenthesis @@ -11331,9 +11330,9 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) { return (yp_node_t *) yp_parentheses_node_create(parser, &opening, (yp_node_t *) statements, &parser->previous); } - // If we have more than one statement in the set of parentheses, then we - // are going to parse all of them as a list of statements. We'll do that - // here. + // If we have more than one statement in the set of parentheses, + // then we are going to parse all of them as a list of statements. + // We'll do that here. context_push(parser, YP_CONTEXT_PARENS); yp_statements_node_t *statements = yp_statements_node_create(parser); yp_statements_node_body_append(statements, statement); @@ -11345,11 +11344,11 @@ parse_expression_prefix(yp_parser_t *parser, yp_binding_power_t binding_power) { yp_node_t *node = parse_expression(parser, YP_BINDING_POWER_STATEMENT, YP_ERR_CANNOT_PARSE_EXPRESSION); yp_statements_node_body_append(statements, node); - // If we're recovering from a syntax error, then we need to stop parsing the - // statements now. + // If we're recovering from a syntax error, then we need to stop + // parsing the statements now. if (parser->recovering) { - // If this is the level of context where the recovery has happened, then - // we can mark the parser as done recovering. + // If this is the level of context where the recovery has + // happened, then we can mark the parser as done recovering. if (match1(parser, YP_TOKEN_PARENTHESIS_RIGHT)) parser->recovering = false; break; }