diff options
author | Kevin Newton <kddnewton@gmail.com> | 2024-05-22 14:31:38 -0400 |
---|---|---|
committer | Kevin Newton <kddnewton@gmail.com> | 2024-05-22 16:34:04 -0400 |
commit | e575954887a8e8ae26a4122c33c66b8cb82dfa36 (patch) | |
tree | f1f9a41a0fcaa57640c914dcc429b8643f675b59 | |
parent | 5613d6e95bc09c2e8d2794590bb18444e0a4051d (diff) | |
download | ruby-e575954887a8e8ae26a4122c33c66b8cb82dfa36.tar.gz |
[ruby/prism] Fix support for 'it' implicit local variable
https://github.com/ruby/prism/commit/53bbcfe513
-rw-r--r-- | lib/prism/translation/parser/compiler.rb | 15 | ||||
-rw-r--r-- | lib/prism/translation/ripper.rb | 14 | ||||
-rw-r--r-- | lib/prism/translation/ruby_parser.rb | 6 | ||||
-rw-r--r-- | prism/config.yml | 10 | ||||
-rw-r--r-- | prism/parser.h | 49 | ||||
-rw-r--r-- | prism/prism.c | 433 | ||||
-rw-r--r-- | prism/templates/src/diagnostic.c.erb | 6 | ||||
-rw-r--r-- | prism/templates/src/token_type.c.erb | 2 | ||||
-rw-r--r-- | test/prism/errors_test.rb | 8 | ||||
-rw-r--r-- | test/prism/location_test.rb | 32 |
10 files changed, 292 insertions, 283 deletions
diff --git a/lib/prism/translation/parser/compiler.rb b/lib/prism/translation/parser/compiler.rb index c3fa68231c..08e54a7bf3 100644 --- a/lib/prism/translation/parser/compiler.rb +++ b/lib/prism/translation/parser/compiler.rb @@ -1149,6 +1149,12 @@ module Prism end # -> { it } + # ^^ + def visit_it_local_variable_read_node(node) + builder.ident([:it, srange(node.location)]).updated(:lvar) + end + + # -> { it } # ^^^^^^^^^ def visit_it_parameters_node(node) builder.args(nil, [], nil, false) @@ -1201,14 +1207,7 @@ module Prism # foo # ^^^ def visit_local_variable_read_node(node) - name = node.name - - # This is just a guess. parser doesn't have support for the implicit - # `it` variable yet, so we'll probably have to visit this once it - # does. - name = :it if name == :"0it" - - builder.ident([name, srange(node.location)]).updated(:lvar) + builder.ident([node.name, srange(node.location)]).updated(:lvar) end # foo = 1 diff --git a/lib/prism/translation/ripper.rb b/lib/prism/translation/ripper.rb index 68f658565d..79ba0e7ab3 100644 --- a/lib/prism/translation/ripper.rb +++ b/lib/prism/translation/ripper.rb @@ -2218,6 +2218,13 @@ module Prism end # -> { it } + # ^^ + def visit_it_local_variable_read_node(node) + bounds(node.location) + on_vcall(on_ident(node.slice)) + end + + # -> { it } # ^^^^^^^^^ def visit_it_parameters_node(node) end @@ -2312,12 +2319,7 @@ module Prism # ^^^ def visit_local_variable_read_node(node) bounds(node.location) - - if node.name == :"0it" - on_vcall(on_ident(node.slice)) - else - on_var_ref(on_ident(node.slice)) - end + on_var_ref(on_ident(node.slice)) end # foo = 1 diff --git a/lib/prism/translation/ruby_parser.rb b/lib/prism/translation/ruby_parser.rb index 6e745a9041..fe08a493d6 100644 --- a/lib/prism/translation/ruby_parser.rb +++ b/lib/prism/translation/ruby_parser.rb @@ -916,6 +916,12 @@ module Prism end end + # -> { it } + # ^^ + def visit_it_local_variable_read_node(node) + s(node, :call, nil, :it) + end + # foo(bar: baz) # ^^^^^^^^ def visit_keyword_hash_node(node) diff --git a/prism/config.yml b/prism/config.yml index 08441ca4de..35daacf614 100644 --- a/prism/config.yml +++ b/prism/config.yml @@ -2570,6 +2570,12 @@ nodes: `foo #{bar} baz` ^^^^^^^^^^^^^^^^ + - name: ItLocalVariableReadNode + comment: | + Represents reading from the implicit `it` local variable. + + -> { it } + ^^ - name: ItParametersNode comment: | Represents an implicit set of parameters through the use of the `it` keyword within a block or lambda. @@ -2695,10 +2701,6 @@ nodes: _1 # name `:_1` - Finally, for the default `it` block parameter, the name is `0it`. This is to distinguish it from an `it` local variable that is explicitly declared. - - it # name `:0it` - - name: depth type: uint32 comment: | diff --git a/prism/parser.h b/prism/parser.h index ff1261841c..2bae4b0ac3 100644 --- a/prism/parser.h +++ b/prism/parser.h @@ -546,6 +546,17 @@ typedef struct pm_locals { pm_local_t *locals; } pm_locals_t; +/** The flags about scope parameters that can be set. */ +typedef uint8_t pm_scope_parameters_t; +static const pm_scope_parameters_t PM_SCOPE_PARAMETERS_NONE = 0x0; +static const pm_scope_parameters_t PM_SCOPE_PARAMETERS_FORWARDING_POSITIONALS = 0x1; +static const pm_scope_parameters_t PM_SCOPE_PARAMETERS_FORWARDING_KEYWORDS = 0x2; +static const pm_scope_parameters_t PM_SCOPE_PARAMETERS_FORWARDING_BLOCK = 0x4; +static const pm_scope_parameters_t PM_SCOPE_PARAMETERS_FORWARDING_ALL = 0x8; +static const pm_scope_parameters_t PM_SCOPE_PARAMETERS_IMPLICIT_DISALLOWED = 0x10; +static const pm_scope_parameters_t PM_SCOPE_PARAMETERS_NUMBERED_INNER = 0x20; +static const pm_scope_parameters_t PM_SCOPE_PARAMETERS_NUMBERED_FOUND = 0x40; + /** * This struct represents a node in a linked list of scopes. Some scopes can see * into their parent scopes, while others cannot. @@ -558,9 +569,18 @@ typedef struct pm_scope { pm_locals_t locals; /** + * This is a list of the implicit parameters contained within the block. + * These will be processed after the block is parsed to determine the kind + * of parameters node that should be used and to check if any errors need to + * be added. + */ + pm_node_list_t implicit_parameters; + + /** * This is a bitfield that indicates the parameters that are being used in - * this scope. It is a combination of the PM_SCOPE_PARAMS_* constants. There - * are three different kinds of parameters that can be used in a scope: + * this scope. It is a combination of the PM_SCOPE_PARAMETERS_* constants. + * There are three different kinds of parameters that can be used in a + * scope: * * - Ordinary parameters (e.g., def foo(bar); end) * - Numbered parameters (e.g., def foo; _1; end) @@ -575,15 +595,7 @@ typedef struct pm_scope { * - def foo(&); end * - def foo(...); end */ - uint8_t parameters; - - /** - * An integer indicating the number of numbered parameters on this scope. - * This is necessary to determine if child blocks are allowed to use - * numbered parameters, and to pass information to consumers of the AST - * about how many numbered parameters exist. - */ - int8_t numbered_parameters; + pm_scope_parameters_t parameters; /** * The current state of constant shareability for this scope. This is @@ -598,21 +610,6 @@ typedef struct pm_scope { bool closed; } pm_scope_t; -static const uint8_t PM_SCOPE_PARAMETERS_NONE = 0x0; -static const uint8_t PM_SCOPE_PARAMETERS_ORDINARY = 0x1; -static const uint8_t PM_SCOPE_PARAMETERS_NUMBERED = 0x2; -static const uint8_t PM_SCOPE_PARAMETERS_IT = 0x4; -static const uint8_t PM_SCOPE_PARAMETERS_TYPE_MASK = 0x7; - -static const uint8_t PM_SCOPE_PARAMETERS_FORWARDING_POSITIONALS = 0x8; -static const uint8_t PM_SCOPE_PARAMETERS_FORWARDING_KEYWORDS = 0x10; -static const uint8_t PM_SCOPE_PARAMETERS_FORWARDING_BLOCK = 0x20; -static const uint8_t PM_SCOPE_PARAMETERS_FORWARDING_ALL = 0x40; - -static const int8_t PM_SCOPE_NUMBERED_PARAMETERS_INNER = -2; -static const int8_t PM_SCOPE_NUMBERED_PARAMETERS_DISALLOWED = -1; -static const int8_t PM_SCOPE_NUMBERED_PARAMETERS_NONE = 0; - /** * A struct that represents a stack of boolean values. */ diff --git a/prism/prism.c b/prism/prism.c index a47aa08ce2..71cb4ad698 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -708,7 +708,7 @@ pm_parser_scope_push(pm_parser_t *parser, bool closed) { .previous = parser->current_scope, .locals = { 0 }, .parameters = PM_SCOPE_PARAMETERS_NONE, - .numbered_parameters = PM_SCOPE_NUMBERED_PARAMETERS_NONE, + .implicit_parameters = { 0 }, .shareable_constant = (closed || parser->current_scope == NULL) ? PM_SCOPE_SHAREABLE_CONSTANT_NONE : parser->current_scope->shareable_constant, .closed = closed }; @@ -4322,7 +4322,7 @@ pm_float_node_rational_create(pm_parser_t *parser, const pm_token_t *token) { const uint8_t *point = memchr(start, '.', length); assert(point && "should have a decimal point"); - uint8_t *digits = malloc(length - 1); + uint8_t *digits = malloc(length); if (digits == NULL) { fputs("[pm_float_node_rational_create] Failed to allocate memory", stderr); abort(); @@ -4333,7 +4333,7 @@ pm_float_node_rational_create(pm_parser_t *parser, const pm_token_t *token) { pm_integer_parse(&node->numerator, PM_INTEGER_BASE_DEFAULT, digits, digits + length - 1); digits[0] = '1'; - memset(digits + 1, '0', (size_t) (end - point - 1)); + if (end - point > 1) memset(digits + 1, '0', (size_t) (end - point - 1)); pm_integer_parse(&node->denominator, PM_INTEGER_BASE_DEFAULT, digits, digits + (end - point)); free(digits); @@ -5498,6 +5498,23 @@ pm_interpolated_xstring_node_closing_set(pm_interpolated_x_string_node_t *node, } /** + * Create a local variable read that is reading the implicit 'it' variable. + */ +static pm_it_local_variable_read_node_t * +pm_it_local_variable_read_node_create(pm_parser_t *parser, const pm_token_t *name) { + pm_it_local_variable_read_node_t *node = PM_ALLOC_NODE(parser, pm_it_local_variable_read_node_t); + + *node = (pm_it_local_variable_read_node_t) { + { + .type = PM_IT_LOCAL_VARIABLE_READ_NODE, + .location = PM_LOCATION_TOKEN_VALUE(name) + } + }; + + return node; +} + +/** * Allocate and initialize a new ItParametersNode node. */ static pm_it_parameters_node_t * @@ -5810,28 +5827,6 @@ pm_token_is_it(const uint8_t *start, const uint8_t *end) { } /** - * Returns true if the given node is `it` default parameter. - */ -static inline bool -pm_node_is_it(pm_parser_t *parser, pm_node_t *node) { - // Check if it's a local variable reference - if (node->type != PM_CALL_NODE) { - return false; - } - - // Check if it's a variable call - pm_call_node_t *call_node = (pm_call_node_t *) node; - if (!PM_NODE_FLAG_P(call_node, PM_CALL_NODE_FLAGS_VARIABLE_CALL)) { - return false; - } - - // Check if it's called `it` - pm_constant_id_t id = ((pm_call_node_t *)node)->name; - pm_constant_t *constant = pm_constant_pool_id_to_constant(&parser->constant_pool, id); - return pm_token_is_it(constant->start, constant->start + constant->length); -} - -/** * Returns true if the given bounds comprise a numbered parameter (i.e., they * are of the form /^_\d$/). */ @@ -7952,51 +7947,6 @@ pm_parser_local_add_constant(pm_parser_t *parser, const char *start, size_t leng } /** - * Create a local variable read that is reading the implicit 'it' variable. - */ -static pm_local_variable_read_node_t * -pm_local_variable_read_node_create_it(pm_parser_t *parser, const pm_token_t *name) { - if (parser->current_scope->parameters & PM_SCOPE_PARAMETERS_ORDINARY) { - pm_parser_err_token(parser, name, PM_ERR_IT_NOT_ALLOWED_ORDINARY); - return NULL; - } - - if (parser->current_scope->parameters & PM_SCOPE_PARAMETERS_NUMBERED) { - pm_parser_err_token(parser, name, PM_ERR_IT_NOT_ALLOWED_NUMBERED); - return NULL; - } - - parser->current_scope->parameters |= PM_SCOPE_PARAMETERS_IT; - - pm_constant_id_t name_id = pm_parser_constant_id_constant(parser, "0it", 3); - pm_parser_local_add(parser, name_id, name->start, name->end, 0); - - return pm_local_variable_read_node_create_constant_id(parser, name, name_id, 0, false); -} - -/** - * Convert a `it` variable call node to a node for `it` default parameter. - */ -static pm_node_t * -pm_node_check_it(pm_parser_t *parser, pm_node_t *node) { - if ( - (parser->version != PM_OPTIONS_VERSION_CRUBY_3_3) && - !parser->current_scope->closed && - (parser->current_scope->numbered_parameters != PM_SCOPE_NUMBERED_PARAMETERS_DISALLOWED) && - pm_node_is_it(parser, node) - ) { - pm_local_variable_read_node_t *read = pm_local_variable_read_node_create_it(parser, &parser->previous); - - if (read != NULL) { - pm_node_destroy(parser, node); - node = (pm_node_t *) read; - } - } - - return node; -} - -/** * Add a parameter name to the current scope and check whether the name of the * parameter is unique or not. * @@ -8031,6 +7981,7 @@ pm_parser_scope_pop(pm_parser_t *parser) { pm_scope_t *scope = parser->current_scope; parser->current_scope = scope->previous; pm_locals_free(&scope->locals); + pm_node_list_free(&scope->implicit_parameters); xfree(scope); } @@ -13188,6 +13139,30 @@ parse_unwriteable_target(pm_parser_t *parser, pm_node_t *target) { } /** + * When an implicit local variable is written to or targeted, it becomes a + * regular, named local variable. This function removes it from the list of + * implicit parameters when that happens. + */ +static void +parse_target_implicit_parameter(pm_parser_t *parser, pm_node_t *node) { + pm_node_list_t *implicit_parameters = &parser->current_scope->implicit_parameters; + + for (size_t index = 0; index < implicit_parameters->size; index++) { + if (implicit_parameters->nodes[index] == node) { + // If the node is not the last one in the list, we need to shift the + // remaining nodes down to fill the gap. This is extremely unlikely + // to happen. + if (index != implicit_parameters->size - 1) { + memcpy(&implicit_parameters->nodes[index], &implicit_parameters->nodes[index + 1], (implicit_parameters->size - index - 1) * sizeof(pm_node_t *)); + } + + implicit_parameters->size--; + break; + } + } +} + +/** * Convert the given node into a valid target node. */ static pm_node_t * @@ -13237,7 +13212,10 @@ parse_target(pm_parser_t *parser, pm_node_t *target, bool multiple) { target->type = PM_GLOBAL_VARIABLE_TARGET_NODE; return target; case PM_LOCAL_VARIABLE_READ_NODE: { - pm_refute_numbered_parameter(parser, target->location.start, target->location.end); + if (pm_token_is_numbered_parameter(target->location.start, target->location.end)) { + PM_PARSER_ERR_FORMAT(parser, target->location.start, target->location.end, PM_ERR_PARAMETER_NUMBERED_RESERVED, target->location.start); + parse_target_implicit_parameter(parser, target); + } const pm_local_variable_read_node_t *cast = (const pm_local_variable_read_node_t *) target; uint32_t name = cast->name; @@ -13249,6 +13227,15 @@ parse_target(pm_parser_t *parser, pm_node_t *target, bool multiple) { return target; } + case PM_IT_LOCAL_VARIABLE_READ_NODE: { + pm_constant_id_t name = pm_parser_local_add_constant(parser, "it", 2); + pm_node_t *node = (pm_node_t *) pm_local_variable_target_node_create(parser, &target->location, name, 0); + + parse_target_implicit_parameter(parser, target); + pm_node_destroy(parser, target); + + return node; + } case PM_INSTANCE_VARIABLE_READ_NODE: assert(sizeof(pm_instance_variable_target_node_t) == sizeof(pm_instance_variable_read_node_t)); target->type = PM_INSTANCE_VARIABLE_TARGET_NODE; @@ -13410,8 +13397,9 @@ parse_write(pm_parser_t *parser, pm_node_t *target, pm_token_t *operator, pm_nod pm_scope_t *scope = pm_parser_scope_find(parser, depth); if (pm_token_is_numbered_parameter(target->location.start, target->location.end)) { - pm_diagnostic_id_t diag_id = scope->parameters > PM_SCOPE_NUMBERED_PARAMETERS_NONE ? PM_ERR_EXPRESSION_NOT_WRITABLE_NUMBERED : PM_ERR_PARAMETER_NUMBERED_RESERVED; + pm_diagnostic_id_t diag_id = (scope->parameters & PM_SCOPE_PARAMETERS_NUMBERED_FOUND) ? PM_ERR_EXPRESSION_NOT_WRITABLE_NUMBERED : PM_ERR_PARAMETER_NUMBERED_RESERVED; PM_PARSER_ERR_FORMAT(parser, target->location.start, target->location.end, diag_id, target->location.start); + parse_target_implicit_parameter(parser, target); } pm_locals_unread(&scope->locals, name); @@ -13419,6 +13407,15 @@ parse_write(pm_parser_t *parser, pm_node_t *target, pm_token_t *operator, pm_nod return (pm_node_t *) pm_local_variable_write_node_create(parser, name, depth, value, &name_loc, operator); } + case PM_IT_LOCAL_VARIABLE_READ_NODE: { + pm_constant_id_t name = pm_parser_local_add_constant(parser, "it", 2); + pm_node_t *node = (pm_node_t *) pm_local_variable_write_node_create(parser, name, 0, value, &target->location, operator); + + parse_target_implicit_parameter(parser, target); + pm_node_destroy(parser, target); + + return node; + } case PM_INSTANCE_VARIABLE_READ_NODE: { pm_node_t *write_node = (pm_node_t *) pm_instance_variable_write_node_create(parser, (pm_instance_variable_read_node_t *) target, operator, value); pm_node_destroy(parser, target); @@ -14861,37 +14858,107 @@ parse_block_parameters( } /** + * Return true if any of the visible scopes to the current context are using + * numbered parameters. + */ +static bool +outer_scope_using_numbered_parameters_p(pm_parser_t *parser) { + for (pm_scope_t *scope = parser->current_scope->previous; scope != NULL && !scope->closed; scope = scope->previous) { + if (scope->parameters & PM_SCOPE_PARAMETERS_NUMBERED_FOUND) return true; + } + + return false; +} + +/** + * These are the names of the various numbered parameters. We have them here so + * that when we insert them into the constant pool we can use a constant string + * and not have to allocate. + */ +static const char * const pm_numbered_parameter_names[] = { + "_1", "_2", "_3", "_4", "_5", "_6", "_7", "_8", "_9" +}; + +/** * Return the node that should be used in the parameters field of a block-like * (block or lambda) node, depending on the kind of parameters that were * declared in the current scope. */ static pm_node_t * parse_blocklike_parameters(pm_parser_t *parser, pm_node_t *parameters, const pm_token_t *opening, const pm_token_t *closing) { - uint8_t masked = parser->current_scope->parameters & PM_SCOPE_PARAMETERS_TYPE_MASK; + pm_node_list_t *implicit_parameters = &parser->current_scope->implicit_parameters; + + // If we have ordinary parameters, then we will return them as the set of + // parameters. + if (parameters != NULL) { + // If we also have implicit parameters, then this is an error. + if (implicit_parameters->size > 0) { + pm_node_t *node = implicit_parameters->nodes[0]; + + if (PM_NODE_TYPE_P(node, PM_LOCAL_VARIABLE_READ_NODE)) { + pm_parser_err_node(parser, node, PM_ERR_NUMBERED_PARAMETER_ORDINARY); + } else if (PM_NODE_TYPE_P(node, PM_IT_LOCAL_VARIABLE_READ_NODE)) { + pm_parser_err_node(parser, node, PM_ERR_IT_NOT_ALLOWED_ORDINARY); + } else { + assert(false && "unreachable"); + } + } - if (masked == PM_SCOPE_PARAMETERS_NONE) { - assert(parameters == NULL); - return NULL; - } else if (masked == PM_SCOPE_PARAMETERS_ORDINARY) { - assert(parameters != NULL); return parameters; - } else if (masked == PM_SCOPE_PARAMETERS_NUMBERED) { - assert(parameters == NULL); + } - int8_t maximum = parser->current_scope->numbered_parameters; - if (maximum > 0) { - const pm_location_t location = { .start = opening->start, .end = closing->end }; - return (pm_node_t *) pm_numbered_parameters_node_create(parser, &location, (uint8_t) maximum); + // If we don't have any implicit parameters, then the set of parameters is + // NULL. + if (implicit_parameters->size == 0) { + return NULL; + } + + // If we don't have ordinary parameters, then we now must validate our set + // of implicit parameters. We can only have numbered parameters or it, but + // they cannot be mixed. + uint8_t numbered_parameter = 0; + bool it_parameter = false; + + for (size_t index = 0; index < implicit_parameters->size; index++) { + pm_node_t *node = implicit_parameters->nodes[index]; + + if (PM_NODE_TYPE_P(node, PM_LOCAL_VARIABLE_READ_NODE)) { + if (it_parameter) { + pm_parser_err_node(parser, node, PM_ERR_NUMBERED_PARAMETER_IT); + } else if (outer_scope_using_numbered_parameters_p(parser)) { + pm_parser_err_node(parser, node, PM_ERR_NUMBERED_PARAMETER_OUTER_BLOCK); + } else if (parser->current_scope->parameters & PM_SCOPE_PARAMETERS_NUMBERED_INNER) { + pm_parser_err_node(parser, node, PM_ERR_NUMBERED_PARAMETER_INNER_BLOCK); + } else if (pm_token_is_numbered_parameter(node->location.start, node->location.end)) { + numbered_parameter = MAX(numbered_parameter, (uint8_t) (node->location.start[1] - '0')); + } else { + assert(false && "unreachable"); + } + } else if (PM_NODE_TYPE_P(node, PM_IT_LOCAL_VARIABLE_READ_NODE)) { + if (numbered_parameter > 0) { + pm_parser_err_node(parser, node, PM_ERR_IT_NOT_ALLOWED_NUMBERED); + } else { + it_parameter = true; + } } + } - return NULL; - } else if (masked == PM_SCOPE_PARAMETERS_IT) { - assert(parameters == NULL); + if (numbered_parameter > 0) { + // Go through the parent scopes and mark them as being disallowed from + // using numbered parameters because this inner scope is using them. + for (pm_scope_t *scope = parser->current_scope->previous; scope != NULL && !scope->closed; scope = scope->previous) { + scope->parameters |= PM_SCOPE_PARAMETERS_NUMBERED_INNER; + } + + const pm_location_t location = { .start = opening->start, .end = closing->end }; + return (pm_node_t *) pm_numbered_parameters_node_create(parser, &location, numbered_parameter); + } + + if (it_parameter) { return (pm_node_t *) pm_it_parameters_node_create(parser, opening, closing); - } else { - assert(false && "unreachable"); - return NULL; } + + return NULL; } /** @@ -14908,9 +14975,6 @@ parse_block(pm_parser_t *parser) { pm_block_parameters_node_t *block_parameters = NULL; if (accept1(parser, PM_TOKEN_PIPE)) { - assert(parser->current_scope->parameters == PM_SCOPE_PARAMETERS_NONE); - parser->current_scope->parameters = PM_SCOPE_PARAMETERS_ORDINARY; - pm_token_t block_parameters_opening = parser->previous; if (match1(parser, PM_TOKEN_PIPE)) { block_parameters = pm_block_parameters_node_create(parser, NULL, &block_parameters_opening); @@ -15408,7 +15472,7 @@ parse_conditional(pm_parser_t *parser, pm_context_t context) { #define PM_CASE_WRITABLE PM_CLASS_VARIABLE_READ_NODE: case PM_CONSTANT_PATH_NODE: \ case PM_CONSTANT_READ_NODE: case PM_GLOBAL_VARIABLE_READ_NODE: case PM_LOCAL_VARIABLE_READ_NODE: \ case PM_INSTANCE_VARIABLE_READ_NODE: case PM_MULTI_TARGET_NODE: case PM_BACK_REFERENCE_READ_NODE: \ - case PM_NUMBERED_REFERENCE_READ_NODE + case PM_NUMBERED_REFERENCE_READ_NODE: case PM_IT_LOCAL_VARIABLE_READ_NODE // Assert here that the flags are the same so that we can safely switch the type // of the node without having to move the flags. @@ -15828,91 +15892,43 @@ parse_alias_argument(pm_parser_t *parser, bool first) { } /** - * Return true if any of the visible scopes to the current context are using - * numbered parameters. - */ -static bool -outer_scope_using_numbered_parameters_p(pm_parser_t *parser) { - for (pm_scope_t *scope = parser->current_scope->previous; scope != NULL && !scope->closed; scope = scope->previous) { - if (scope->numbered_parameters > 0) return true; - } - - return false; -} - -/** - * These are the names of the various numbered parameters. We have them here so - * that when we insert them into the constant pool we can use a constant string - * and not have to allocate. - */ -static const char * const pm_numbered_parameter_names[] = { - "_1", "_2", "_3", "_4", "_5", "_6", "_7", "_8", "_9" -}; - -/** * 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 * +static pm_node_t * parse_variable(pm_parser_t *parser) { + pm_constant_id_t name_id = pm_parser_constant_id_token(parser, &parser->previous); 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 ((depth = pm_parser_local_depth_constant_id(parser, name_id)) != -1) { + return (pm_node_t *) pm_local_variable_read_node_create_constant_id(parser, &parser->previous, name_id, (uint32_t) depth, false); } pm_scope_t *current_scope = parser->current_scope; - if (!current_scope->closed && current_scope->numbered_parameters != PM_SCOPE_NUMBERED_PARAMETERS_DISALLOWED && 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 (current_scope->parameters & PM_SCOPE_PARAMETERS_ORDINARY) { - pm_parser_err_previous(parser, PM_ERR_NUMBERED_PARAMETER_ORDINARY); - } else if (current_scope->parameters & PM_SCOPE_PARAMETERS_IT) { - pm_parser_err_previous(parser, PM_ERR_NUMBERED_PARAMETER_IT); - } else if (outer_scope_using_numbered_parameters_p(parser)) { - pm_parser_err_previous(parser, PM_ERR_NUMBERED_PARAMETER_OUTER_BLOCK); - } else if (current_scope->numbered_parameters == PM_SCOPE_NUMBERED_PARAMETERS_INNER) { - pm_parser_err_previous(parser, PM_ERR_NUMBERED_PARAMETER_INNER_BLOCK); - } else { - // Indicate that this scope is using numbered params so that child - // scopes cannot. We subtract the value for the character '0' to get - // the actual integer value of the number (only _1 through _9 are - // valid). - int8_t numbered_parameters = (int8_t) (parser->previous.start[1] - '0'); - - // If we're about to match an =, then this is an invalid use of - // numbered parameters. We'll create all of the necessary - // infrastructure around it, but not actually mark the scope as - // using numbered parameters so that we can get the right error - // message. - if (!match1(parser, PM_TOKEN_EQUAL)) { - current_scope->parameters |= PM_SCOPE_PARAMETERS_NUMBERED; - - if (numbered_parameters > current_scope->numbered_parameters) { - current_scope->numbered_parameters = numbered_parameters; - } - - // Go through the parent scopes and mark them as being - // disallowed from using numbered parameters because this inner - // scope is using them. - for (pm_scope_t *scope = current_scope->previous; scope != NULL && !scope->closed; scope = scope->previous) { - scope->numbered_parameters = PM_SCOPE_NUMBERED_PARAMETERS_INNER; - } + if (!current_scope->closed && !(current_scope->parameters & PM_SCOPE_PARAMETERS_IMPLICIT_DISALLOWED)) { + if (pm_token_is_numbered_parameter(parser->previous.start, parser->previous.end)) { + // 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 maximum = (uint8_t) (parser->previous.start[1] - '0'); + for (uint8_t number = 1; number <= maximum; number++) { + pm_parser_local_add_constant(parser, pm_numbered_parameter_names[number - 1], 2); } - // 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. - for (int8_t numbered_param = 1; numbered_param <= numbered_parameters - 1; numbered_param++) { - pm_parser_local_add_constant(parser, pm_numbered_parameter_names[numbered_param - 1], 2); + if (!match1(parser, PM_TOKEN_EQUAL)) { + parser->current_scope->parameters |= PM_SCOPE_PARAMETERS_NUMBERED_FOUND; } - // Finally we can create the local variable read node. - pm_constant_id_t name_id = pm_parser_local_add_constant(parser, pm_numbered_parameter_names[numbered_parameters - 1], 2); - return pm_local_variable_read_node_create_constant_id(parser, &parser->previous, name_id, 0, false); + pm_node_t *node = (pm_node_t *) pm_local_variable_read_node_create_constant_id(parser, &parser->previous, name_id, 0, false); + pm_node_list_append(¤t_scope->implicit_parameters, node); + + return node; + } else if ((parser->version != PM_OPTIONS_VERSION_CRUBY_3_3) && pm_token_is_it(parser->previous.start, parser->previous.end)) { + pm_node_t *node = (pm_node_t *) pm_it_local_variable_read_node_create(parser, &parser->previous); + pm_node_list_append(¤t_scope->implicit_parameters, node); + + return node; } } @@ -15927,8 +15943,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] != '?')) { - pm_local_variable_read_node_t *node = parse_variable(parser); - if (node != NULL) return (pm_node_t *) node; + pm_node_t *node = parse_variable(parser); + if (node != NULL) return node; flags |= PM_CALL_NODE_FLAGS_VARIABLE_CALL; } @@ -16616,19 +16632,8 @@ parse_pattern_primitive(pm_parser_t *parser, pm_constant_id_list_t *captures, pm pm_node_t *variable = (pm_node_t *) parse_variable(parser); if (variable == NULL) { - if ( - (parser->version != PM_OPTIONS_VERSION_CRUBY_3_3) && - !parser->current_scope->closed && - (parser->current_scope->numbered_parameters != PM_SCOPE_NUMBERED_PARAMETERS_DISALLOWED) && - pm_token_is_it(parser->previous.start, parser->previous.end) - ) { - pm_local_variable_read_node_t *read = pm_local_variable_read_node_create_it(parser, &parser->previous); - if (read == NULL) read = pm_local_variable_read_node_create(parser, &parser->previous, 0); - variable = (pm_node_t *) read; - } else { - PM_PARSER_ERR_TOKEN_FORMAT_CONTENT(parser, parser->previous, PM_ERR_NO_LOCAL_VARIABLE); - variable = (pm_node_t *) pm_local_variable_read_node_missing_create(parser, &parser->previous, 0); - } + PM_PARSER_ERR_TOKEN_FORMAT_CONTENT(parser, parser->previous, PM_ERR_NO_LOCAL_VARIABLE); + variable = (pm_node_t *) pm_local_variable_read_node_missing_create(parser, &parser->previous, 0); } return (pm_node_t *) pm_pinned_variable_node_create(parser, &operator, variable); @@ -17378,8 +17383,13 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b break; } - if (pm_array_node_size(array) != 0) { - expect1(parser, PM_TOKEN_COMMA, PM_ERR_ARRAY_SEPARATOR); + // Ensure that we have a comma between elements in the array. + if ((pm_array_node_size(array) != 0) && !accept1(parser, PM_TOKEN_COMMA)) { + const uint8_t *location = parser->previous.end; + PM_PARSER_ERR_FORMAT(parser, location, location, PM_ERR_ARRAY_SEPARATOR, pm_token_type_human(parser->current.type)); + + parser->previous.start = location; + parser->previous.type = PM_TOKEN_MISSING; } // If we have a right bracket immediately following a comma, @@ -17807,8 +17817,28 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b ) { pm_arguments_t arguments = { 0 }; parse_arguments_list(parser, &arguments, true, accepts_command_call); - pm_call_node_t *fcall = pm_call_node_fcall_create(parser, &identifier, &arguments); + + if (PM_NODE_TYPE_P(node, PM_IT_LOCAL_VARIABLE_READ_NODE)) { + // If we're about to convert an 'it' implicit local + // variable read into a method call, we need to remove + // it from the list of implicit local variables. + parse_target_implicit_parameter(parser, node); + } else { + // Otherwise, we're about to convert a regular local + // variable read into a method call, in which case we + // need to indicate that this was not a read for the + // purposes of warnings. + assert(PM_NODE_TYPE_P(node, PM_LOCAL_VARIABLE_READ_NODE)); + + if (pm_token_is_numbered_parameter(identifier.start, identifier.end)) { + parse_target_implicit_parameter(parser, node); + } else { + pm_local_variable_read_node_t *cast = (pm_local_variable_read_node_t *) node; + pm_locals_unread(&pm_parser_scope_find(parser, cast->depth)->locals, cast->name); + } + } + pm_node_destroy(parser, node); return (pm_node_t *) fcall; } @@ -17816,31 +17846,6 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b if ((binding_power == PM_BINDING_POWER_STATEMENT) && match1(parser, PM_TOKEN_COMMA)) { node = parse_targets_validate(parser, node, PM_BINDING_POWER_INDEX); - } else { - // Check if `it` is not going to be assigned. - switch (parser->current.type) { - case PM_TOKEN_AMPERSAND_AMPERSAND_EQUAL: - case PM_TOKEN_AMPERSAND_EQUAL: - case PM_TOKEN_CARET_EQUAL: - case PM_TOKEN_EQUAL: - case PM_TOKEN_GREATER_GREATER_EQUAL: - case PM_TOKEN_LESS_LESS_EQUAL: - case PM_TOKEN_MINUS_EQUAL: - case PM_TOKEN_PARENTHESIS_RIGHT: - case PM_TOKEN_PERCENT_EQUAL: - case PM_TOKEN_PIPE_EQUAL: - case PM_TOKEN_PIPE_PIPE_EQUAL: - case PM_TOKEN_PLUS_EQUAL: - case PM_TOKEN_SLASH_EQUAL: - case PM_TOKEN_STAR_EQUAL: - case PM_TOKEN_STAR_STAR_EQUAL: - break; - default: - // Once we know it's neither a method call nor an - // assignment, we can finally create `it` default - // parameter. - node = pm_node_check_it(parser, node); - } } return node; @@ -19702,9 +19707,6 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b switch (parser->current.type) { case PM_TOKEN_PARENTHESIS_LEFT: { - assert(parser->current_scope->parameters == PM_SCOPE_PARAMETERS_NONE); - parser->current_scope->parameters = PM_SCOPE_PARAMETERS_ORDINARY; - pm_token_t opening = parser->current; parser_lex(parser); @@ -19721,9 +19723,6 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b break; } case PM_CASE_PARAMETER: { - assert(parser->current_scope->parameters == PM_SCOPE_PARAMETERS_NONE); - parser->current_scope->parameters = PM_SCOPE_PARAMETERS_ORDINARY; - pm_accepts_block_stack_push(parser, false); pm_token_t opening = not_provided(parser); block_parameters = parse_block_parameters(parser, false, &opening, true); @@ -21258,7 +21257,7 @@ pm_parser_init(pm_parser_t *parser, const uint8_t *source, size_t size, const pm // Scopes given from the outside are not allowed to have numbered // parameters. - parser->current_scope->numbered_parameters = PM_SCOPE_NUMBERED_PARAMETERS_DISALLOWED; + parser->current_scope->parameters |= PM_SCOPE_PARAMETERS_IMPLICIT_DISALLOWED; for (size_t local_index = 0; local_index < scope->locals_count; local_index++) { const pm_string_t *local = pm_options_scope_local_get(scope, local_index); diff --git a/prism/templates/src/diagnostic.c.erb b/prism/templates/src/diagnostic.c.erb index 30d6b0ffed..715cbd6598 100644 --- a/prism/templates/src/diagnostic.c.erb +++ b/prism/templates/src/diagnostic.c.erb @@ -112,7 +112,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = { [PM_ERR_ARRAY_ELEMENT] = { "expected an element for the array", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_ARRAY_EXPRESSION] = { "expected an expression for the array element", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_ARRAY_EXPRESSION_AFTER_STAR] = { "expected an expression after `*` in the array", PM_ERROR_LEVEL_SYNTAX }, - [PM_ERR_ARRAY_SEPARATOR] = { "expected a `,` separator for the array elements", PM_ERROR_LEVEL_SYNTAX }, + [PM_ERR_ARRAY_SEPARATOR] = { "unexpected %s; expected a `,` separator for the array elements", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_ARRAY_TERM] = { "expected a `]` to close the array", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_BEGIN_LONELY_ELSE] = { "unexpected `else` in `begin` block; else without rescue is useless", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_BEGIN_TERM] = { "expected an `end` to close the `begin` statement", PM_ERROR_LEVEL_SYNTAX }, @@ -245,7 +245,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = { [PM_ERR_INVALID_VARIABLE_GLOBAL_3_3] = { "`%.*s' is not allowed as a global variable name", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_INVALID_VARIABLE_GLOBAL] = { "'%.*s' is not allowed as a global variable name", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_INVALID_YIELD] = { "Invalid yield", PM_ERROR_LEVEL_SYNTAX }, - [PM_ERR_IT_NOT_ALLOWED_NUMBERED] = { "`it` is not allowed when an numbered parameter is defined", PM_ERROR_LEVEL_SYNTAX }, + [PM_ERR_IT_NOT_ALLOWED_NUMBERED] = { "`it` is not allowed when a numbered parameter is already used", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_IT_NOT_ALLOWED_ORDINARY] = { "`it` is not allowed when an ordinary parameter is defined", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_LAMBDA_OPEN] = { "expected a `do` keyword or a `{` to open the lambda block", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_LAMBDA_TERM_BRACE] = { "expected a lambda block beginning with `{` to end with `}`", PM_ERROR_LEVEL_SYNTAX }, @@ -269,7 +269,7 @@ static const pm_diagnostic_data_t diagnostic_messages[PM_DIAGNOSTIC_ID_MAX] = { [PM_ERR_NO_LOCAL_VARIABLE] = { "%.*s: no such local variable", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_NUMBER_LITERAL_UNDERSCORE] = { "number literal ending with a `_`", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_NUMBERED_PARAMETER_INNER_BLOCK] = { "numbered parameter is already used in inner block", PM_ERROR_LEVEL_SYNTAX }, - [PM_ERR_NUMBERED_PARAMETER_IT] = { "numbered parameters are not allowed when an 'it' parameter is defined", PM_ERROR_LEVEL_SYNTAX }, + [PM_ERR_NUMBERED_PARAMETER_IT] = { "numbered parameters are not allowed when 'it' is already used", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_NUMBERED_PARAMETER_ORDINARY] = { "numbered parameters are not allowed when an ordinary parameter is defined", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_NUMBERED_PARAMETER_OUTER_BLOCK] = { "numbered parameter is already used in outer block", PM_ERROR_LEVEL_SYNTAX }, [PM_ERR_OPERATOR_MULTI_ASSIGN] = { "unexpected operator for a multiple assignment", PM_ERROR_LEVEL_SYNTAX }, diff --git a/prism/templates/src/token_type.c.erb b/prism/templates/src/token_type.c.erb index af6a2ad6fe..f196393ee1 100644 --- a/prism/templates/src/token_type.c.erb +++ b/prism/templates/src/token_type.c.erb @@ -352,7 +352,7 @@ pm_token_type_human(pm_token_type_t token_type) { case PM_TOKEN_USTAR: return "*"; case PM_TOKEN_USTAR_STAR: - return "'**'"; + return "**"; case PM_TOKEN_WORDS_SEP: return "string separator"; case PM_TOKEN___END__: diff --git a/test/prism/errors_test.rb b/test/prism/errors_test.rb index 6f926452fa..50417e476e 100644 --- a/test/prism/errors_test.rb +++ b/test/prism/errors_test.rb @@ -1356,14 +1356,14 @@ module Prism if RUBY_VERSION >= "3.0" def test_writing_numbered_parameter - assert_errors expression("-> { _1 = 0 }"), "-> { _1 = 0 }", [ - ["_1 is reserved for numbered parameters", 5..7] + assert_error_messages "-> { _1 = 0 }", [ + "_1 is reserved for numbered parameters" ] end def test_targeting_numbered_parameter - assert_errors expression("-> { _1, = 0 }"), "-> { _1, = 0 }", [ - ["_1 is reserved for numbered parameters", 5..7] + assert_error_messages "-> { _1, = 0 }", [ + "_1 is reserved for numbered parameters" ] end diff --git a/test/prism/location_test.rb b/test/prism/location_test.rb index 0724995671..256e5b41e4 100644 --- a/test/prism/location_test.rb +++ b/test/prism/location_test.rb @@ -175,14 +175,6 @@ module Prism assert_location(CallNode, "foo bar baz") assert_location(CallNode, "foo bar('baz')") - - assert_location(CallNode, "-> { it }", 5...7, version: "3.3.0") do |node| - node.body.body.first - end - - assert_location(LocalVariableReadNode, "-> { it }", 5...7, version: "3.4.0") do |node| - node.body.body.first - end end def test_CallAndWriteNode @@ -543,6 +535,24 @@ module Prism assert_location(InterpolatedXStringNode, '`foo #{bar} baz`') end + def test_ItLocalVariableReadNode + assert_location(ItLocalVariableReadNode, "-> { it }", 5...7) do |node| + node.body.body.first + end + + assert_location(ItLocalVariableReadNode, "foo { it }", 6...8) do |node| + node.block.body.body.first + end + + assert_location(CallNode, "-> { it }", 5...7, version: "3.3.0") do |node| + node.body.body.first + end + + assert_location(ItLocalVariableReadNode, "-> { it }", 5...7, version: "3.4.0") do |node| + node.body.body.first + end + end + def test_ItParametersNode assert_location(ItParametersNode, "-> { it }", &:parameters) end @@ -583,12 +593,6 @@ module Prism def test_LocalVariableReadNode assert_location(LocalVariableReadNode, "foo = 1; foo", 9...12) - assert_location(LocalVariableReadNode, "-> { it }", 5...7) do |node| - node.body.body.first - end - assert_location(LocalVariableReadNode, "foo { it }", 6...8) do |node| - node.block.body.body.first - end end def test_LocalVariableTargetNode |