diff options
author | Kevin Newton <kddnewton@gmail.com> | 2023-11-28 15:49:04 -0500 |
---|---|---|
committer | git <svn-admin@ruby-lang.org> | 2023-11-28 22:33:50 +0000 |
commit | 49383901772f874569bbdc992867bd02b4b597a8 (patch) | |
tree | abd79930a7511b6504a6314f4aab6a0869d86e66 | |
parent | ea3e17e430b74b4e58535a707319025e008cc123 (diff) | |
download | ruby-49383901772f874569bbdc992867bd02b4b597a8.tar.gz |
[ruby/prism] Implicit rest nodes
Fundamentally, `foo { |bar,| }` is different from `foo { |bar, *| }`
because of arity checks. This PR introduces a new node to handle
that, `ImplicitRestNode`, which goes in the `rest` slot of parameter
nodes instead of `RestParameterNode` instances.
This is also used in a couple of other places, namely:
* pattern matching: `foo in [bar,]`
* multi target: `for foo, in bar do end`
* multi write: `foo, = bar`
Now the only splat nodes with a `NULL` value are when you're
forwarding, as in: `def foo(*) = bar(*)`.
https://github.com/ruby/prism/commit/dba2a3b652
-rw-r--r-- | lib/prism/debug.rb | 2 | ||||
-rw-r--r-- | prism/config.yml | 16 | ||||
-rw-r--r-- | prism/prism.c | 67 | ||||
-rw-r--r-- | test/prism/location_test.rb | 16 | ||||
-rw-r--r-- | test/prism/snapshots/blocks.txt | 5 | ||||
-rw-r--r-- | test/prism/snapshots/seattlerb/bug236.txt | 5 | ||||
-rw-r--r-- | test/prism/snapshots/seattlerb/masgn_command_call.txt | 4 | ||||
-rw-r--r-- | test/prism/snapshots/seattlerb/parse_pattern_051.txt | 3 | ||||
-rw-r--r-- | test/prism/snapshots/unparser/corpus/literal/assignment.txt | 8 | ||||
-rw-r--r-- | test/prism/snapshots/unparser/corpus/literal/block.txt | 10 | ||||
-rw-r--r-- | test/prism/snapshots/unparser/corpus/literal/pattern.txt | 3 | ||||
-rw-r--r-- | test/prism/snapshots/variables.txt | 4 | ||||
-rw-r--r-- | test/prism/snapshots/whitequark/blockargs.txt | 10 | ||||
-rw-r--r-- | test/prism/snapshots/whitequark/masgn_nested.txt | 4 |
14 files changed, 91 insertions, 66 deletions
diff --git a/lib/prism/debug.rb b/lib/prism/debug.rb index d0469adc9a..adbc402f32 100644 --- a/lib/prism/debug.rb +++ b/lib/prism/debug.rb @@ -121,7 +121,7 @@ module Prism end end, *params.optionals.map(&:name), - *((params.rest.name || :*) if params.rest && params.rest.operator != ","), + *((params.rest.name || :*) if params.rest && !params.rest.is_a?(ImplicitRestNode)), *params.posts.map do |post| if post.is_a?(RequiredParameterNode) post.name diff --git a/prism/config.yml b/prism/config.yml index 353ad50c0d..4fd5c462f1 100644 --- a/prism/config.yml +++ b/prism/config.yml @@ -1455,6 +1455,21 @@ nodes: { Foo: } ^^^^ + - name: ImplicitRestNode + comment: | + Represents using a trailing comma to indicate an implicit rest parameter. + + foo { |bar,| } + ^ + + foo in [bar,] + ^ + + for foo, in bar do end + ^ + + foo, = bar + ^ - name: InNode fields: - name: pattern @@ -2071,7 +2086,6 @@ nodes: type: node[] - name: rest type: node? - kind: RestParameterNode - name: posts type: node[] - name: keywords diff --git a/prism/prism.c b/prism/prism.c index 05ecc6ad60..03b6204a78 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -1197,7 +1197,7 @@ pm_array_pattern_node_node_list_create(pm_parser_t *parser, pm_node_list_t *node for (size_t index = 0; index < nodes->size; index++) { pm_node_t *child = nodes->nodes[index]; - if (!found_rest && PM_NODE_TYPE_P(child, PM_SPLAT_NODE)) { + if (!found_rest && (PM_NODE_TYPE_P(child, PM_SPLAT_NODE) || PM_NODE_TYPE_P(child, PM_IMPLICIT_REST_NODE))) { node->rest = child; found_rest = true; } else if (found_rest) { @@ -3425,6 +3425,25 @@ pm_implicit_node_create(pm_parser_t *parser, pm_node_t *value) { } /** + * Allocate and initialize a new ImplicitRestNode node. + */ +static pm_implicit_rest_node_t * +pm_implicit_rest_node_create(pm_parser_t *parser, const pm_token_t *token) { + assert(token->type == PM_TOKEN_COMMA); + + pm_implicit_rest_node_t *node = PM_ALLOC_NODE(parser, pm_implicit_rest_node_t); + + *node = (pm_implicit_rest_node_t) { + { + .type = PM_IMPLICIT_REST_NODE, + .location = PM_LOCATION_TOKEN_VALUE(token) + } + }; + + return node; +} + +/** * Allocate and initialize a new IntegerNode node. */ static pm_integer_node_t * @@ -4285,7 +4304,7 @@ pm_multi_target_node_create(pm_parser_t *parser) { */ static void pm_multi_target_node_targets_append(pm_parser_t *parser, pm_multi_target_node_t *node, pm_node_t *target) { - if (PM_NODE_TYPE_P(target, PM_SPLAT_NODE)) { + if (PM_NODE_TYPE_P(target, PM_SPLAT_NODE) || PM_NODE_TYPE_P(target, PM_IMPLICIT_REST_NODE)) { if (node->rest == NULL) { node->rest = target; } else { @@ -4561,9 +4580,8 @@ pm_parameters_node_posts_append(pm_parameters_node_t *params, pm_node_t *param) * Set the rest parameter on a ParametersNode node. */ static void -pm_parameters_node_rest_set(pm_parameters_node_t *params, pm_rest_parameter_node_t *param) { - assert(params->rest == NULL); - pm_parameters_node_location_set(params, (pm_node_t *) param); +pm_parameters_node_rest_set(pm_parameters_node_t *params, pm_node_t *param) { + pm_parameters_node_location_set(params, param); params->rest = param; } @@ -10918,7 +10936,7 @@ parse_write(pm_parser_t *parser, pm_node_t *target, pm_token_t *operator, pm_nod */ static pm_node_t * parse_targets(pm_parser_t *parser, pm_node_t *first_target, pm_binding_power_t binding_power) { - bool has_splat = PM_NODE_TYPE_P(first_target, PM_SPLAT_NODE); + bool has_rest = PM_NODE_TYPE_P(first_target, PM_SPLAT_NODE); pm_multi_target_node_t *result = pm_multi_target_node_create(parser); pm_multi_target_node_targets_append(parser, result, parse_target(parser, first_target)); @@ -10928,7 +10946,7 @@ parse_targets(pm_parser_t *parser, pm_node_t *first_target, pm_binding_power_t b // Here we have a splat operator. It can have a name or be // anonymous. It can be the final target or be in the middle if // there haven't been any others yet. - if (has_splat) { + if (has_rest) { pm_parser_err_previous(parser, PM_ERR_MULTI_ASSIGN_MULTI_SPLATS); } @@ -10942,7 +10960,7 @@ parse_targets(pm_parser_t *parser, pm_node_t *first_target, pm_binding_power_t b pm_node_t *splat = (pm_node_t *) pm_splat_node_create(parser, &star_operator, name); pm_multi_target_node_targets_append(parser, result, splat); - has_splat = true; + has_rest = true; } else if (token_begins_expression_p(parser->current.type)) { pm_node_t *target = parse_expression(parser, binding_power, PM_ERR_EXPECT_EXPRESSION_AFTER_COMMA); target = parse_target(parser, target); @@ -10950,10 +10968,10 @@ parse_targets(pm_parser_t *parser, pm_node_t *first_target, pm_binding_power_t b pm_multi_target_node_targets_append(parser, result, target); } else if (!match1(parser, PM_TOKEN_EOF)) { // If we get here, then we have a trailing , in a multi target node. - // We need to indicate this somehow in the tree, so we'll add an - // anonymous splat. - pm_node_t *splat = (pm_node_t *) pm_splat_node_create(parser, &parser->previous, NULL); - pm_multi_target_node_targets_append(parser, result, splat); + // We'll set the implicit rest flag to indicate this. + pm_node_t *rest = (pm_node_t *) pm_implicit_rest_node_create(parser, &parser->previous); + pm_multi_target_node_targets_append(parser, result, rest); + has_rest = true; break; } } @@ -11382,11 +11400,14 @@ parse_required_destructured_parameter(pm_parser_t *parser) { do { pm_node_t *param; - // If we get here then we have a trailing comma. In this case we'll - // create an implicit splat node. + // If we get here then we have a trailing comma, which isn't allowed in + // the grammar. In other places, multi targets _do_ allow trailing + // commas, so here we'll assume this is a mistake of the user not + // knowing it's not allowed here. if (node->lefts.size > 0 && match1(parser, PM_TOKEN_PARENTHESIS_RIGHT)) { - param = (pm_node_t *) pm_splat_node_create(parser, &parser->previous, NULL); + param = (pm_node_t *) pm_implicit_rest_node_create(parser, &parser->previous); pm_multi_target_node_targets_append(parser, node, param); + pm_parser_err_current(parser, PM_ERR_PARAMETER_WILD_LOOSE_COMMA); break; } @@ -11726,12 +11747,12 @@ parse_parameters( } } - pm_rest_parameter_node_t *param = pm_rest_parameter_node_create(parser, &operator, &name); + pm_node_t *param = (pm_node_t *) pm_rest_parameter_node_create(parser, &operator, &name); if (params->rest == NULL) { pm_parameters_node_rest_set(params, param); } else { - pm_parser_err_node(parser, (pm_node_t *) param, PM_ERR_PARAMETER_SPLAT_MULTI); - pm_parameters_node_posts_append(params, (pm_node_t *) param); + pm_parser_err_node(parser, param, PM_ERR_PARAMETER_SPLAT_MULTI); + pm_parameters_node_posts_append(params, param); } break; @@ -11776,11 +11797,9 @@ parse_parameters( default: if (parser->previous.type == PM_TOKEN_COMMA) { if (allows_trailing_comma) { - // If we get here, then we have a trailing comma in a block - // parameter list. We need to create an anonymous rest parameter to - // represent it. - pm_token_t name = not_provided(parser); - pm_rest_parameter_node_t *param = pm_rest_parameter_node_create(parser, &parser->previous, &name); + // If we get here, then we have a trailing comma in a + // block parameter list. + pm_node_t *param = (pm_node_t *) pm_implicit_rest_node_create(parser, &parser->previous); if (params->rest == NULL) { pm_parameters_node_rest_set(params, param); @@ -13466,6 +13485,8 @@ parse_pattern(pm_parser_t *parser, bool top_pattern, pm_diagnostic_id_t diag_id) while (accept1(parser, PM_TOKEN_COMMA)) { // Break early here in case we have a trailing comma. if (match5(parser, PM_TOKEN_KEYWORD_THEN, PM_TOKEN_BRACE_RIGHT, PM_TOKEN_BRACKET_RIGHT, PM_TOKEN_NEWLINE, PM_TOKEN_SEMICOLON)) { + node = (pm_node_t *) pm_implicit_rest_node_create(parser, &parser->previous); + pm_node_list_append(&nodes, node); break; } diff --git a/test/prism/location_test.rb b/test/prism/location_test.rb index 302e6bd139..26415ab0c6 100644 --- a/test/prism/location_test.rb +++ b/test/prism/location_test.rb @@ -428,6 +428,22 @@ module Prism end end + def test_ImplicitRestNode + assert_location(ImplicitRestNode, "foo, = bar", 3..4, &:rest) + + assert_location(ImplicitRestNode, "for foo, in bar do end", 7..8) do |node| + node.index.rest + end + + assert_location(ImplicitRestNode, "foo { |bar,| }", 10..11) do |node| + node.block.parameters.parameters.rest + end + + assert_location(ImplicitRestNode, "foo in [bar,]", 11..12) do |node| + node.pattern.rest + end + end + def test_InNode assert_location(InNode, "case foo; in bar; end", 10...16) do |node| node.conditions.first diff --git a/test/prism/snapshots/blocks.txt b/test/prism/snapshots/blocks.txt index 955b087ed3..d3005d1c17 100644 --- a/test/prism/snapshots/blocks.txt +++ b/test/prism/snapshots/blocks.txt @@ -762,10 +762,7 @@ │ │ │ │ └── name: :bar │ │ │ ├── optionals: (length: 0) │ │ │ ├── rest: - │ │ │ │ @ RestParameterNode (location: (54,11)-(54,12)) - │ │ │ │ ├── name: ∅ - │ │ │ │ ├── name_loc: ∅ - │ │ │ │ └── operator_loc: (54,11)-(54,12) = "," + │ │ │ │ @ ImplicitRestNode (location: (54,11)-(54,12)) │ │ │ ├── posts: (length: 0) │ │ │ ├── keywords: (length: 0) │ │ │ ├── keyword_rest: ∅ diff --git a/test/prism/snapshots/seattlerb/bug236.txt b/test/prism/snapshots/seattlerb/bug236.txt index 0d9427bfff..a6ee09e21a 100644 --- a/test/prism/snapshots/seattlerb/bug236.txt +++ b/test/prism/snapshots/seattlerb/bug236.txt @@ -23,10 +23,7 @@ │ │ │ │ │ └── name: :a │ │ │ │ ├── optionals: (length: 0) │ │ │ │ ├── rest: - │ │ │ │ │ @ RestParameterNode (location: (1,4)-(1,5)) - │ │ │ │ │ ├── name: ∅ - │ │ │ │ │ ├── name_loc: ∅ - │ │ │ │ │ └── operator_loc: (1,4)-(1,5) = "," + │ │ │ │ │ @ ImplicitRestNode (location: (1,4)-(1,5)) │ │ │ │ ├── posts: (length: 0) │ │ │ │ ├── keywords: (length: 0) │ │ │ │ ├── keyword_rest: ∅ diff --git a/test/prism/snapshots/seattlerb/masgn_command_call.txt b/test/prism/snapshots/seattlerb/masgn_command_call.txt index ceb81c0500..7a7b26b9c3 100644 --- a/test/prism/snapshots/seattlerb/masgn_command_call.txt +++ b/test/prism/snapshots/seattlerb/masgn_command_call.txt @@ -9,9 +9,7 @@ │ ├── name: :a │ └── depth: 0 ├── rest: - │ @ SplatNode (location: (1,1)-(1,2)) - │ ├── operator_loc: (1,1)-(1,2) = "," - │ └── expression: ∅ + │ @ ImplicitRestNode (location: (1,1)-(1,2)) ├── rights: (length: 0) ├── lparen_loc: ∅ ├── rparen_loc: ∅ diff --git a/test/prism/snapshots/seattlerb/parse_pattern_051.txt b/test/prism/snapshots/seattlerb/parse_pattern_051.txt index fb9c3ee282..6067c109e1 100644 --- a/test/prism/snapshots/seattlerb/parse_pattern_051.txt +++ b/test/prism/snapshots/seattlerb/parse_pattern_051.txt @@ -26,7 +26,8 @@ │ │ │ │ └── flags: decimal │ │ │ └── @ IntegerNode (location: (2,7)-(2,8)) │ │ │ └── flags: decimal - │ │ ├── rest: ∅ + │ │ ├── rest: + │ │ │ @ ImplicitRestNode (location: (2,8)-(2,9)) │ │ ├── posts: (length: 0) │ │ ├── opening_loc: (2,3)-(2,4) = "[" │ │ └── closing_loc: (2,9)-(2,10) = "]" diff --git a/test/prism/snapshots/unparser/corpus/literal/assignment.txt b/test/prism/snapshots/unparser/corpus/literal/assignment.txt index ebb343a002..7890d308b2 100644 --- a/test/prism/snapshots/unparser/corpus/literal/assignment.txt +++ b/test/prism/snapshots/unparser/corpus/literal/assignment.txt @@ -39,9 +39,7 @@ │ │ │ │ ├── name: :a │ │ │ │ └── depth: 0 │ │ │ ├── rest: - │ │ │ │ @ SplatNode (location: (3,3)-(3,4)) - │ │ │ │ ├── operator_loc: (3,3)-(3,4) = "," - │ │ │ │ └── expression: ∅ + │ │ │ │ @ ImplicitRestNode (location: (3,3)-(3,4)) │ │ │ ├── rights: (length: 0) │ │ │ ├── lparen_loc: (3,1)-(3,2) = "(" │ │ │ └── rparen_loc: (3,4)-(3,5) = ")" @@ -274,9 +272,7 @@ │ │ ├── name: :a │ │ └── depth: 0 │ ├── rest: - │ │ @ SplatNode (location: (13,2)-(13,3)) - │ │ ├── operator_loc: (13,2)-(13,3) = "," - │ │ └── expression: ∅ + │ │ @ ImplicitRestNode (location: (13,2)-(13,3)) │ ├── rights: (length: 0) │ ├── lparen_loc: (13,0)-(13,1) = "(" │ ├── rparen_loc: (13,3)-(13,4) = ")" diff --git a/test/prism/snapshots/unparser/corpus/literal/block.txt b/test/prism/snapshots/unparser/corpus/literal/block.txt index 76d51d709e..11f92dd01b 100644 --- a/test/prism/snapshots/unparser/corpus/literal/block.txt +++ b/test/prism/snapshots/unparser/corpus/literal/block.txt @@ -72,10 +72,7 @@ │ │ │ │ │ └── name: :a │ │ │ │ ├── optionals: (length: 0) │ │ │ │ ├── rest: - │ │ │ │ │ @ RestParameterNode (location: (5,8)-(5,9)) - │ │ │ │ │ ├── name: ∅ - │ │ │ │ │ ├── name_loc: ∅ - │ │ │ │ │ └── operator_loc: (5,8)-(5,9) = "," + │ │ │ │ │ @ ImplicitRestNode (location: (5,8)-(5,9)) │ │ │ │ ├── posts: (length: 0) │ │ │ │ ├── keywords: (length: 0) │ │ │ │ ├── keyword_rest: ∅ @@ -108,10 +105,7 @@ │ │ │ │ │ └── name: :a │ │ │ │ ├── optionals: (length: 0) │ │ │ │ ├── rest: - │ │ │ │ │ @ RestParameterNode (location: (7,8)-(7,9)) - │ │ │ │ │ ├── name: ∅ - │ │ │ │ │ ├── name_loc: ∅ - │ │ │ │ │ └── operator_loc: (7,8)-(7,9) = "," + │ │ │ │ │ @ ImplicitRestNode (location: (7,8)-(7,9)) │ │ │ │ ├── posts: (length: 0) │ │ │ │ ├── keywords: (length: 0) │ │ │ │ ├── keyword_rest: ∅ diff --git a/test/prism/snapshots/unparser/corpus/literal/pattern.txt b/test/prism/snapshots/unparser/corpus/literal/pattern.txt index 88a191a334..983ec43050 100644 --- a/test/prism/snapshots/unparser/corpus/literal/pattern.txt +++ b/test/prism/snapshots/unparser/corpus/literal/pattern.txt @@ -54,7 +54,8 @@ │ │ │ │ │ │ └── flags: decimal │ │ │ │ │ └── @ IntegerNode (location: (4,7)-(4,8)) │ │ │ │ │ └── flags: decimal - │ │ │ │ ├── rest: ∅ + │ │ │ │ ├── rest: + │ │ │ │ │ @ ImplicitRestNode (location: (4,8)-(4,9)) │ │ │ │ ├── posts: (length: 0) │ │ │ │ ├── opening_loc: (4,3)-(4,4) = "[" │ │ │ │ └── closing_loc: (4,10)-(4,11) = "]" diff --git a/test/prism/snapshots/variables.txt b/test/prism/snapshots/variables.txt index 887b2a1b47..6930d43b25 100644 --- a/test/prism/snapshots/variables.txt +++ b/test/prism/snapshots/variables.txt @@ -199,9 +199,7 @@ │ │ ├── name: :foo │ │ └── depth: 0 │ ├── rest: - │ │ @ SplatNode (location: (35,3)-(35,4)) - │ │ ├── operator_loc: (35,3)-(35,4) = "," - │ │ └── expression: ∅ + │ │ @ ImplicitRestNode (location: (35,3)-(35,4)) │ ├── rights: (length: 0) │ ├── lparen_loc: ∅ │ ├── rparen_loc: ∅ diff --git a/test/prism/snapshots/whitequark/blockargs.txt b/test/prism/snapshots/whitequark/blockargs.txt index 150d02bf7a..9e579bee86 100644 --- a/test/prism/snapshots/whitequark/blockargs.txt +++ b/test/prism/snapshots/whitequark/blockargs.txt @@ -598,10 +598,7 @@ │ │ │ │ │ └── name: :b │ │ │ │ ├── optionals: (length: 0) │ │ │ │ ├── rest: - │ │ │ │ │ @ RestParameterNode (location: (37,8)-(37,9)) - │ │ │ │ │ ├── name: ∅ - │ │ │ │ │ ├── name_loc: ∅ - │ │ │ │ │ └── operator_loc: (37,8)-(37,9) = "," + │ │ │ │ │ @ ImplicitRestNode (location: (37,8)-(37,9)) │ │ │ │ ├── posts: (length: 0) │ │ │ │ ├── keywords: (length: 0) │ │ │ │ ├── keyword_rest: ∅ @@ -859,10 +856,7 @@ │ │ │ │ │ └── name: :a │ │ │ │ ├── optionals: (length: 0) │ │ │ │ ├── rest: - │ │ │ │ │ @ RestParameterNode (location: (49,5)-(49,6)) - │ │ │ │ │ ├── name: ∅ - │ │ │ │ │ ├── name_loc: ∅ - │ │ │ │ │ └── operator_loc: (49,5)-(49,6) = "," + │ │ │ │ │ @ ImplicitRestNode (location: (49,5)-(49,6)) │ │ │ │ ├── posts: (length: 0) │ │ │ │ ├── keywords: (length: 0) │ │ │ │ ├── keyword_rest: ∅ diff --git a/test/prism/snapshots/whitequark/masgn_nested.txt b/test/prism/snapshots/whitequark/masgn_nested.txt index e716762a98..d9513e5813 100644 --- a/test/prism/snapshots/whitequark/masgn_nested.txt +++ b/test/prism/snapshots/whitequark/masgn_nested.txt @@ -11,9 +11,7 @@ │ │ │ ├── name: :b │ │ │ └── depth: 0 │ │ ├── rest: - │ │ │ @ SplatNode (location: (1,3)-(1,4)) - │ │ │ ├── operator_loc: (1,3)-(1,4) = "," - │ │ │ └── expression: ∅ + │ │ │ @ ImplicitRestNode (location: (1,3)-(1,4)) │ │ ├── rights: (length: 0) │ │ ├── lparen_loc: (1,1)-(1,2) = "(" │ │ └── rparen_loc: (1,5)-(1,6) = ")" |