From a25329e0dab2a3ada395b1e4e0254a15cd498de2 Mon Sep 17 00:00:00 2001 From: TSUYUSATO Kitsune Date: Wed, 3 Jan 2024 13:30:23 +0900 Subject: [PATCH] [ruby/prism] Fix parsing pinned local variable pattern for numbered parameter Fix https://github.com/ruby/prism/pull/2094 The part of `parse_variable_call` for variables was split into a new function `parse_variable` and used it. https://github.com/ruby/prism/commit/4c5fd1a746 --- prism/prism.c | 124 ++++++++++++++++-------------- test/prism/fixtures/case.txt | 6 ++ test/prism/fixtures/patterns.txt | 2 + test/prism/snapshots/case.txt | 90 ++++++++++++++++------ test/prism/snapshots/patterns.txt | 105 ++++++++++++++++--------- 5 files changed, 210 insertions(+), 117 deletions(-) diff --git a/prism/prism.c b/prism/prism.c index 4aeef976c9..5cefb1a86a 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -12775,6 +12775,65 @@ outer_scope_using_numbered_parameters_p(pm_parser_t *parser) { return false; } +/** + * Parse an identifier into either a local variable read. If the local variable + * is not found, it returns NULL instead. + */ +static pm_local_variable_read_node_t * +parse_variable(pm_parser_t *parser) { + int depth; + if ((depth = pm_parser_local_depth(parser, &parser->previous)) != -1) { + return pm_local_variable_read_node_create(parser, &parser->previous, (uint32_t) depth); + } + + if (!parser->current_scope->closed && pm_token_is_numbered_parameter(parser->previous.start, parser->previous.end)) { + // Now that we know we have a numbered parameter, we need to check + // if it's allowed in this context. If it is, then we will create a + // local variable read. If it's not, then we'll create a normal call + // node but add an error. + if (parser->current_scope->explicit_params) { + pm_parser_err_previous(parser, PM_ERR_NUMBERED_PARAMETER_NOT_ALLOWED); + } else if (outer_scope_using_numbered_parameters_p(parser)) { + pm_parser_err_previous(parser, PM_ERR_NUMBERED_PARAMETER_OUTER_SCOPE); + } else { + // Indicate that this scope is using numbered params so that child + // scopes cannot. + uint8_t number = parser->previous.start[1]; + + // We subtract the value for the character '0' to get the actual + // integer value of the number (only _1 through _9 are valid) + uint8_t numbered_parameters = (uint8_t) (number - '0'); + if (numbered_parameters > parser->current_scope->numbered_parameters) { + parser->current_scope->numbered_parameters = numbered_parameters; + pm_parser_numbered_parameters_set(parser, numbered_parameters); + } + + // When you use a numbered parameter, it implies the existence + // of all of the locals that exist before it. For example, + // referencing _2 means that _1 must exist. Therefore here we + // loop through all of the possibilities and add them into the + // constant pool. + uint8_t current = '1'; + uint8_t *value; + + while (current < number) { + value = malloc(2); + value[0] = '_'; + value[1] = current++; + pm_parser_local_add_owned(parser, value, 2); + } + + // Now we can add the actual token that is being used. For + // this one we can add a shared version since it is directly + // referenced in the source. + pm_parser_local_add_token(parser, &parser->previous); + return pm_local_variable_read_node_create(parser, &parser->previous, 0); + } + } + + return NULL; +} + /** * Parse an identifier into either a local variable read or a call. */ @@ -12783,56 +12842,8 @@ parse_variable_call(pm_parser_t *parser) { pm_node_flags_t flags = 0; if (!match1(parser, PM_TOKEN_PARENTHESIS_LEFT) && (parser->previous.end[-1] != '!') && (parser->previous.end[-1] != '?')) { - int depth; - if ((depth = pm_parser_local_depth(parser, &parser->previous)) != -1) { - return (pm_node_t *) pm_local_variable_read_node_create(parser, &parser->previous, (uint32_t) depth); - } - - if (!parser->current_scope->closed && pm_token_is_numbered_parameter(parser->previous.start, parser->previous.end)) { - // Now that we know we have a numbered parameter, we need to check - // if it's allowed in this context. If it is, then we will create a - // local variable read. If it's not, then we'll create a normal call - // node but add an error. - if (parser->current_scope->explicit_params) { - pm_parser_err_previous(parser, PM_ERR_NUMBERED_PARAMETER_NOT_ALLOWED); - } else if (outer_scope_using_numbered_parameters_p(parser)) { - pm_parser_err_previous(parser, PM_ERR_NUMBERED_PARAMETER_OUTER_SCOPE); - } else { - // Indicate that this scope is using numbered params so that child - // scopes cannot. - uint8_t number = parser->previous.start[1]; - - // We subtract the value for the character '0' to get the actual - // integer value of the number (only _1 through _9 are valid) - uint8_t numbered_parameters = (uint8_t) (number - '0'); - if (numbered_parameters > parser->current_scope->numbered_parameters) { - parser->current_scope->numbered_parameters = numbered_parameters; - pm_parser_numbered_parameters_set(parser, numbered_parameters); - } - - // When you use a numbered parameter, it implies the existence - // of all of the locals that exist before it. For example, - // referencing _2 means that _1 must exist. Therefore here we - // loop through all of the possibilities and add them into the - // constant pool. - uint8_t current = '1'; - uint8_t *value; - - while (current < number) { - value = malloc(2); - value[0] = '_'; - value[1] = current++; - pm_parser_local_add_owned(parser, value, 2); - } - - // Now we can add the actual token that is being used. For - // this one we can add a shared version since it is directly - // referenced in the source. - pm_parser_local_add_token(parser, &parser->previous); - return (pm_node_t *) pm_local_variable_read_node_create(parser, &parser->previous, 0); - } - } - + pm_local_variable_read_node_t *node = parse_variable(parser); + if (node != NULL) return (pm_node_t *) node; flags |= PM_CALL_NODE_FLAGS_VARIABLE_CALL; } @@ -13391,15 +13402,12 @@ parse_pattern_primitive(pm_parser_t *parser, pm_diagnostic_id_t diag_id) { // expression to determine if it's a variable or an expression. switch (parser->current.type) { case PM_TOKEN_IDENTIFIER: { - int depth = pm_parser_local_depth(parser, &parser->current); - - if (depth == -1) { - depth = 0; - PM_PARSER_ERR_TOKEN_FORMAT(parser, parser->current, PM_ERR_NO_LOCAL_VARIABLE, (int) (parser->current.end - parser->current.start), parser->current.start); - } - - pm_node_t *variable = (pm_node_t *) pm_local_variable_read_node_create(parser, &parser->current, (uint32_t) depth); parser_lex(parser); + pm_node_t *variable = (pm_node_t *) parse_variable(parser); + if (variable == NULL) { + PM_PARSER_ERR_TOKEN_FORMAT(parser, parser->previous, PM_ERR_NO_LOCAL_VARIABLE, (int) (parser->previous.end - parser->previous.start), parser->previous.start); + variable = (pm_node_t *) pm_local_variable_read_node_create(parser, &parser->previous, 0); + } return (pm_node_t *) pm_pinned_variable_node_create(parser, &operator, variable); } diff --git a/test/prism/fixtures/case.txt b/test/prism/fixtures/case.txt index 9db91e8d0f..dea6424db3 100644 --- a/test/prism/fixtures/case.txt +++ b/test/prism/fixtures/case.txt @@ -42,3 +42,9 @@ in 3 end case 1 in 2; in 3; end + +1.then do + case 1 + in ^_1 + end +end diff --git a/test/prism/fixtures/patterns.txt b/test/prism/fixtures/patterns.txt index dcd6786d4b..41e8aad29e 100644 --- a/test/prism/fixtures/patterns.txt +++ b/test/prism/fixtures/patterns.txt @@ -200,3 +200,5 @@ foo do end foo => Object[{x:}] + +1.then { 1 in ^_1 } diff --git a/test/prism/snapshots/case.txt b/test/prism/snapshots/case.txt index f205771f42..44f438092d 100644 --- a/test/prism/snapshots/case.txt +++ b/test/prism/snapshots/case.txt @@ -1,8 +1,8 @@ -@ ProgramNode (location: (1,0)-(44,22)) +@ ProgramNode (location: (1,0)-(50,3)) ├── locals: [] └── statements: - @ StatementsNode (location: (1,0)-(44,22)) - └── body: (length: 13) + @ StatementsNode (location: (1,0)-(50,3)) + └── body: (length: 14) ├── @ CaseNode (location: (1,0)-(3,3)) │ ├── predicate: │ │ @ SymbolNode (location: (1,5)-(1,8)) @@ -335,24 +335,66 @@ │ ├── consequent: ∅ │ ├── case_keyword_loc: (40,0)-(40,4) = "case" │ └── end_keyword_loc: (42,0)-(42,3) = "end" - └── @ CaseMatchNode (location: (44,0)-(44,22)) - ├── predicate: - │ @ MatchPredicateNode (location: (44,5)-(44,11)) - │ ├── value: - │ │ @ IntegerNode (location: (44,5)-(44,6)) - │ │ └── flags: decimal - │ ├── pattern: - │ │ @ IntegerNode (location: (44,10)-(44,11)) - │ │ └── flags: decimal - │ └── operator_loc: (44,7)-(44,9) = "in" - ├── conditions: (length: 1) - │ └── @ InNode (location: (44,13)-(44,17)) - │ ├── pattern: - │ │ @ IntegerNode (location: (44,16)-(44,17)) - │ │ └── flags: decimal - │ ├── statements: ∅ - │ ├── in_loc: (44,13)-(44,15) = "in" - │ └── then_loc: ∅ - ├── consequent: ∅ - ├── case_keyword_loc: (44,0)-(44,4) = "case" - └── end_keyword_loc: (44,19)-(44,22) = "end" + ├── @ CaseMatchNode (location: (44,0)-(44,22)) + │ ├── predicate: + │ │ @ MatchPredicateNode (location: (44,5)-(44,11)) + │ │ ├── value: + │ │ │ @ IntegerNode (location: (44,5)-(44,6)) + │ │ │ └── flags: decimal + │ │ ├── pattern: + │ │ │ @ IntegerNode (location: (44,10)-(44,11)) + │ │ │ └── flags: decimal + │ │ └── operator_loc: (44,7)-(44,9) = "in" + │ ├── conditions: (length: 1) + │ │ └── @ InNode (location: (44,13)-(44,17)) + │ │ ├── pattern: + │ │ │ @ IntegerNode (location: (44,16)-(44,17)) + │ │ │ └── flags: decimal + │ │ ├── statements: ∅ + │ │ ├── in_loc: (44,13)-(44,15) = "in" + │ │ └── then_loc: ∅ + │ ├── consequent: ∅ + │ ├── case_keyword_loc: (44,0)-(44,4) = "case" + │ └── end_keyword_loc: (44,19)-(44,22) = "end" + └── @ CallNode (location: (46,0)-(50,3)) + ├── flags: ∅ + ├── receiver: + │ @ IntegerNode (location: (46,0)-(46,1)) + │ └── flags: decimal + ├── call_operator_loc: (46,1)-(46,2) = "." + ├── name: :then + ├── message_loc: (46,2)-(46,6) = "then" + ├── opening_loc: ∅ + ├── arguments: ∅ + ├── closing_loc: ∅ + └── block: + @ BlockNode (location: (46,7)-(50,3)) + ├── locals: [:_1] + ├── locals_body_index: 1 + ├── parameters: + │ @ NumberedParametersNode (location: (46,7)-(50,3)) + │ └── maximum: 1 + ├── body: + │ @ StatementsNode (location: (47,2)-(49,5)) + │ └── body: (length: 1) + │ └── @ CaseMatchNode (location: (47,2)-(49,5)) + │ ├── predicate: + │ │ @ IntegerNode (location: (47,7)-(47,8)) + │ │ └── flags: decimal + │ ├── conditions: (length: 1) + │ │ └── @ InNode (location: (48,2)-(48,8)) + │ │ ├── pattern: + │ │ │ @ PinnedVariableNode (location: (48,5)-(48,8)) + │ │ │ ├── variable: + │ │ │ │ @ LocalVariableReadNode (location: (48,6)-(48,8)) + │ │ │ │ ├── name: :_1 + │ │ │ │ └── depth: 0 + │ │ │ └── operator_loc: (48,5)-(48,6) = "^" + │ │ ├── statements: ∅ + │ │ ├── in_loc: (48,2)-(48,4) = "in" + │ │ └── then_loc: ∅ + │ ├── consequent: ∅ + │ ├── case_keyword_loc: (47,2)-(47,6) = "case" + │ └── end_keyword_loc: (49,2)-(49,5) = "end" + ├── opening_loc: (46,7)-(46,9) = "do" + └── closing_loc: (50,0)-(50,3) = "end" diff --git a/test/prism/snapshots/patterns.txt b/test/prism/snapshots/patterns.txt index f7601609c5..17bd04f3bc 100644 --- a/test/prism/snapshots/patterns.txt +++ b/test/prism/snapshots/patterns.txt @@ -1,8 +1,8 @@ -@ ProgramNode (location: (1,0)-(202,19)) +@ ProgramNode (location: (1,0)-(204,19)) ├── locals: [:bar, :baz, :qux, :b, :a, :foo, :x] └── statements: - @ StatementsNode (location: (1,0)-(202,19)) - └── body: (length: 176) + @ StatementsNode (location: (1,0)-(204,19)) + └── body: (length: 177) ├── @ MatchRequiredNode (location: (1,0)-(1,10)) │ ├── value: │ │ @ CallNode (location: (1,0)-(1,3)) @@ -4735,35 +4735,70 @@ │ │ └── operator_loc: (199,9)-(199,11) = "=>" │ ├── opening_loc: (198,4)-(198,6) = "do" │ └── closing_loc: (200,0)-(200,3) = "end" - └── @ MatchRequiredNode (location: (202,0)-(202,19)) - ├── value: - │ @ LocalVariableReadNode (location: (202,0)-(202,3)) - │ ├── name: :foo - │ └── depth: 0 - ├── pattern: - │ @ ArrayPatternNode (location: (202,7)-(202,19)) - │ ├── constant: - │ │ @ ConstantReadNode (location: (202,7)-(202,13)) - │ │ └── name: :Object - │ ├── requireds: (length: 1) - │ │ └── @ HashPatternNode (location: (202,14)-(202,18)) - │ │ ├── constant: ∅ - │ │ ├── elements: (length: 1) - │ │ │ └── @ AssocNode (location: (202,15)-(202,17)) - │ │ │ ├── key: - │ │ │ │ @ SymbolNode (location: (202,15)-(202,17)) - │ │ │ │ ├── flags: ∅ - │ │ │ │ ├── opening_loc: ∅ - │ │ │ │ ├── value_loc: (202,15)-(202,16) = "x" - │ │ │ │ ├── closing_loc: (202,16)-(202,17) = ":" - │ │ │ │ └── unescaped: "x" - │ │ │ ├── value: ∅ - │ │ │ └── operator_loc: ∅ - │ │ ├── rest: ∅ - │ │ ├── opening_loc: (202,14)-(202,15) = "{" - │ │ └── closing_loc: (202,17)-(202,18) = "}" - │ ├── rest: ∅ - │ ├── posts: (length: 0) - │ ├── opening_loc: (202,13)-(202,14) = "[" - │ └── closing_loc: (202,18)-(202,19) = "]" - └── operator_loc: (202,4)-(202,6) = "=>" + ├── @ MatchRequiredNode (location: (202,0)-(202,19)) + │ ├── value: + │ │ @ LocalVariableReadNode (location: (202,0)-(202,3)) + │ │ ├── name: :foo + │ │ └── depth: 0 + │ ├── pattern: + │ │ @ ArrayPatternNode (location: (202,7)-(202,19)) + │ │ ├── constant: + │ │ │ @ ConstantReadNode (location: (202,7)-(202,13)) + │ │ │ └── name: :Object + │ │ ├── requireds: (length: 1) + │ │ │ └── @ HashPatternNode (location: (202,14)-(202,18)) + │ │ │ ├── constant: ∅ + │ │ │ ├── elements: (length: 1) + │ │ │ │ └── @ AssocNode (location: (202,15)-(202,17)) + │ │ │ │ ├── key: + │ │ │ │ │ @ SymbolNode (location: (202,15)-(202,17)) + │ │ │ │ │ ├── flags: ∅ + │ │ │ │ │ ├── opening_loc: ∅ + │ │ │ │ │ ├── value_loc: (202,15)-(202,16) = "x" + │ │ │ │ │ ├── closing_loc: (202,16)-(202,17) = ":" + │ │ │ │ │ └── unescaped: "x" + │ │ │ │ ├── value: ∅ + │ │ │ │ └── operator_loc: ∅ + │ │ │ ├── rest: ∅ + │ │ │ ├── opening_loc: (202,14)-(202,15) = "{" + │ │ │ └── closing_loc: (202,17)-(202,18) = "}" + │ │ ├── rest: ∅ + │ │ ├── posts: (length: 0) + │ │ ├── opening_loc: (202,13)-(202,14) = "[" + │ │ └── closing_loc: (202,18)-(202,19) = "]" + │ └── operator_loc: (202,4)-(202,6) = "=>" + └── @ CallNode (location: (204,0)-(204,19)) + ├── flags: ∅ + ├── receiver: + │ @ IntegerNode (location: (204,0)-(204,1)) + │ └── flags: decimal + ├── call_operator_loc: (204,1)-(204,2) = "." + ├── name: :then + ├── message_loc: (204,2)-(204,6) = "then" + ├── opening_loc: ∅ + ├── arguments: ∅ + ├── closing_loc: ∅ + └── block: + @ BlockNode (location: (204,7)-(204,19)) + ├── locals: [:_1] + ├── locals_body_index: 1 + ├── parameters: + │ @ NumberedParametersNode (location: (204,7)-(204,19)) + │ └── maximum: 1 + ├── body: + │ @ StatementsNode (location: (204,9)-(204,17)) + │ └── body: (length: 1) + │ └── @ MatchPredicateNode (location: (204,9)-(204,17)) + │ ├── value: + │ │ @ IntegerNode (location: (204,9)-(204,10)) + │ │ └── flags: decimal + │ ├── pattern: + │ │ @ PinnedVariableNode (location: (204,14)-(204,17)) + │ │ ├── variable: + │ │ │ @ LocalVariableReadNode (location: (204,15)-(204,17)) + │ │ │ ├── name: :_1 + │ │ │ └── depth: 0 + │ │ └── operator_loc: (204,14)-(204,15) = "^" + │ └── operator_loc: (204,11)-(204,13) = "in" + ├── opening_loc: (204,7)-(204,8) = "{" + └── closing_loc: (204,18)-(204,19) = "}"