From 76a207e5420b9ecdcc62c0d571f869c022d5d36e Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Thu, 11 Jan 2024 15:47:15 -0500 Subject: [PATCH] [ruby/prism] Fix nested default value error https://github.com/ruby/prism/commit/ef26b283de --- prism/prism.c | 98 +++++++++++++++++++++++++---------- prism/util/pm_constant_pool.c | 23 ++++---- prism/util/pm_constant_pool.h | 7 +++ 3 files changed, 91 insertions(+), 37 deletions(-) diff --git a/prism/prism.c b/prism/prism.c index 96725b1ee0..570b86ab1d 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -5932,6 +5932,33 @@ pm_parser_scope_push(pm_parser_t *parser, bool closed) { return true; } +/** + * Save the current param name as the return value and set it to the given + * constant id. + */ +static inline pm_constant_id_t +pm_parser_current_param_name_set(pm_parser_t *parser, pm_constant_id_t current_param_name) { + pm_constant_id_t saved_param_name = parser->current_param_name; + parser->current_param_name = current_param_name; + return saved_param_name; +} + +/** + * Save the current param name as the return value and clear it. + */ +static inline pm_constant_id_t +pm_parser_current_param_name_unset(pm_parser_t *parser) { + return pm_parser_current_param_name_set(parser, PM_CONSTANT_ID_UNSET); +} + +/** + * Restore the current param name from the given value. + */ +static inline void +pm_parser_current_param_name_restore(pm_parser_t *parser, pm_constant_id_t saved_param_name) { + parser->current_param_name = saved_param_name; +} + /** * Check if any of the currently visible scopes contain a local variable * described by the given constant id. @@ -11715,8 +11742,8 @@ parse_parameters( if (accept1(parser, PM_TOKEN_EQUAL)) { pm_token_t operator = parser->previous; context_push(parser, PM_CONTEXT_DEFAULT_PARAMS); - pm_constant_id_t old_param_name = parser->current_param_name; - parser->current_param_name = pm_parser_constant_id_token(parser, &name); + + pm_constant_id_t saved_param_name = pm_parser_current_param_name_set(parser, pm_parser_constant_id_token(parser, &name)); pm_node_t *value = parse_value_expression(parser, binding_power, false, PM_ERR_PARAMETER_NO_DEFAULT); pm_optional_parameter_node_t *param = pm_optional_parameter_node_create(parser, &name, &operator, value); @@ -11725,7 +11752,7 @@ parse_parameters( } pm_parameters_node_optionals_append(params, param); - parser->current_param_name = old_param_name; + pm_parser_current_param_name_restore(parser, saved_param_name); context_pop(parser); // If parsing the value of the parameter resulted in error recovery, @@ -11793,11 +11820,13 @@ parse_parameters( if (token_begins_expression_p(parser->current.type)) { context_push(parser, PM_CONTEXT_DEFAULT_PARAMS); - pm_constant_id_t old_param_name = parser->current_param_name; - parser->current_param_name = pm_parser_constant_id_token(parser, &local); + + pm_constant_id_t saved_param_name = pm_parser_current_param_name_set(parser, pm_parser_constant_id_token(parser, &local)); pm_node_t *value = parse_value_expression(parser, binding_power, false, PM_ERR_PARAMETER_NO_DEFAULT_KW); - parser->current_param_name = old_param_name; + + pm_parser_current_param_name_restore(parser, saved_param_name); context_pop(parser); + param = (pm_node_t *) pm_optional_keyword_parameter_node_create(parser, &name, value); } else { @@ -12142,8 +12171,10 @@ parse_block(pm_parser_t *parser) { pm_token_t opening = parser->previous; accept1(parser, PM_TOKEN_NEWLINE); + pm_constant_id_t saved_param_name = pm_parser_current_param_name_unset(parser); pm_accepts_block_stack_push(parser, true); pm_parser_scope_push(parser, false); + pm_block_parameters_node_t *block_parameters = NULL; if (accept1(parser, PM_TOKEN_PIPE)) { @@ -12207,6 +12238,8 @@ parse_block(pm_parser_t *parser) { pm_constant_id_list_t locals = parser->current_scope->locals; pm_parser_scope_pop(parser); pm_accepts_block_stack_pop(parser); + pm_parser_current_param_name_restore(parser, saved_param_name); + return pm_block_node_create(parser, &locals, locals_body_index, &opening, parameters, statements, &parser->previous); } @@ -14910,8 +14943,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b pm_token_t operator = parser->previous; pm_node_t *expression = parse_value_expression(parser, PM_BINDING_POWER_NOT, true, PM_ERR_EXPECT_EXPRESSION_AFTER_LESS_LESS); - pm_constant_id_t old_param_name = parser->current_param_name; - parser->current_param_name = 0; + pm_constant_id_t saved_param_name = pm_parser_current_param_name_unset(parser); pm_parser_scope_push(parser, true); accept2(parser, PM_TOKEN_NEWLINE, PM_TOKEN_SEMICOLON); @@ -14928,11 +14960,12 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b } expect1(parser, PM_TOKEN_KEYWORD_END, PM_ERR_CLASS_TERM); - pm_constant_id_list_t locals = parser->current_scope->locals; + pm_parser_scope_pop(parser); - parser->current_param_name = old_param_name; pm_do_loop_stack_pop(parser); + pm_parser_current_param_name_restore(parser, saved_param_name); + return (pm_node_t *) pm_singleton_class_node_create(parser, &locals, &class_keyword, &operator, expression, statements, &parser->previous); } @@ -14958,9 +14991,9 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b superclass = NULL; } - pm_constant_id_t old_param_name = parser->current_param_name; - parser->current_param_name = 0; + pm_constant_id_t saved_param_name = pm_parser_current_param_name_unset(parser); pm_parser_scope_push(parser, true); + if (inheritance_operator.type != PM_TOKEN_NOT_PROVIDED) { expect2(parser, PM_TOKEN_NEWLINE, PM_TOKEN_SEMICOLON, PM_ERR_CLASS_UNEXPECTED_END); } else { @@ -14986,9 +15019,10 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b } pm_constant_id_list_t locals = parser->current_scope->locals; + pm_parser_scope_pop(parser); - parser->current_param_name = old_param_name; pm_do_loop_stack_pop(parser); + pm_parser_current_param_name_restore(parser, saved_param_name); if (!PM_NODE_TYPE_P(constant_path, PM_CONSTANT_PATH_NODE) && !(PM_NODE_TYPE_P(constant_path, PM_CONSTANT_READ_NODE))) { pm_parser_err_node(parser, constant_path, PM_ERR_CLASS_NAME); @@ -15003,18 +15037,21 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b pm_token_t operator = not_provided(parser); pm_token_t name = (pm_token_t) { .type = PM_TOKEN_MISSING, .start = def_keyword.end, .end = def_keyword.end }; - // This context is necessary for lexing `...` in a bare params correctly. - // It must be pushed before lexing the first param, so it is here. + // This context is necessary for lexing `...` in a bare params + // correctly. It must be pushed before lexing the first param, so it + // is here. context_push(parser, PM_CONTEXT_DEF_PARAMS); + pm_constant_id_t saved_param_name; + parser_lex(parser); - pm_constant_id_t old_param_name = parser->current_param_name; switch (parser->current.type) { case PM_CASE_OPERATOR: + saved_param_name = pm_parser_current_param_name_unset(parser); pm_parser_scope_push(parser, true); - parser->current_param_name = 0; lex_state_set(parser, PM_LEX_STATE_ENDFN); parser_lex(parser); + name = parser->previous; break; case PM_TOKEN_IDENTIFIER: { @@ -15023,17 +15060,18 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b if (match2(parser, PM_TOKEN_DOT, PM_TOKEN_COLON_COLON)) { receiver = parse_variable_call(parser); + saved_param_name = pm_parser_current_param_name_unset(parser); pm_parser_scope_push(parser, true); - parser->current_param_name = 0; lex_state_set(parser, PM_LEX_STATE_FNAME); parser_lex(parser); operator = parser->previous; name = parse_method_definition_name(parser); } else { + saved_param_name = pm_parser_current_param_name_unset(parser); pm_refute_numbered_parameter(parser, parser->previous.start, parser->previous.end); pm_parser_scope_push(parser, true); - parser->current_param_name = 0; + name = parser->previous; } @@ -15050,9 +15088,10 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b case PM_TOKEN_KEYWORD___FILE__: case PM_TOKEN_KEYWORD___LINE__: case PM_TOKEN_KEYWORD___ENCODING__: { + saved_param_name = pm_parser_current_param_name_unset(parser); pm_parser_scope_push(parser, true); - parser->current_param_name = 0; parser_lex(parser); + pm_token_t identifier = parser->previous; if (match2(parser, PM_TOKEN_DOT, PM_TOKEN_COLON_COLON)) { @@ -15124,8 +15163,8 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b operator = parser->previous; receiver = (pm_node_t *) pm_parentheses_node_create(parser, &lparen, expression, &rparen); + saved_param_name = pm_parser_current_param_name_unset(parser); pm_parser_scope_push(parser, true); - parser->current_param_name = 0; // To push `PM_CONTEXT_DEF_PARAMS` again is for the same reason as described the above. context_push(parser, PM_CONTEXT_DEF_PARAMS); @@ -15133,8 +15172,9 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b break; } default: + saved_param_name = pm_parser_current_param_name_unset(parser); pm_parser_scope_push(parser, true); - parser->current_param_name = 0; + name = parse_method_definition_name(parser); break; } @@ -15249,8 +15289,9 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b } pm_constant_id_list_t locals = parser->current_scope->locals; - parser->current_param_name = old_param_name; + pm_parser_scope_pop(parser); + pm_parser_current_param_name_restore(parser, saved_param_name); return (pm_node_t *) pm_def_node_create( parser, @@ -15478,9 +15519,9 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b pm_parser_err_token(parser, &name, PM_ERR_MODULE_NAME); } - pm_constant_id_t old_param_name = parser->current_param_name; - parser->current_param_name = 0; + pm_constant_id_t saved_param_name = pm_parser_current_param_name_unset(parser); pm_parser_scope_push(parser, true); + accept2(parser, PM_TOKEN_SEMICOLON, PM_TOKEN_NEWLINE); pm_node_t *statements = NULL; @@ -15497,7 +15538,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b pm_constant_id_list_t locals = parser->current_scope->locals; pm_parser_scope_pop(parser); - parser->current_param_name = old_param_name; + pm_parser_current_param_name_restore(parser, saved_param_name); expect1(parser, PM_TOKEN_KEYWORD_END, PM_ERR_MODULE_TERM); @@ -16164,7 +16205,9 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b parser_lex(parser); pm_token_t operator = parser->previous; + pm_constant_id_t saved_param_name = pm_parser_current_param_name_unset(parser); pm_parser_scope_push(parser, false); + pm_block_parameters_node_t *block_parameters; switch (parser->current.type) { @@ -16243,8 +16286,11 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, b } pm_constant_id_list_t locals = parser->current_scope->locals; + pm_parser_scope_pop(parser); pm_accepts_block_stack_pop(parser); + pm_parser_current_param_name_restore(parser, saved_param_name); + return (pm_node_t *) pm_lambda_node_create(parser, &locals, locals_body_index, &operator, &opening, &parser->previous, parameters, body); } case PM_TOKEN_UPLUS: { diff --git a/prism/util/pm_constant_pool.c b/prism/util/pm_constant_pool.c index e06682eb48..7b8ed0654d 100644 --- a/prism/util/pm_constant_pool.c +++ b/prism/util/pm_constant_pool.c @@ -124,13 +124,13 @@ pm_constant_pool_resize(pm_constant_pool_t *pool) { // If an id is set on this constant, then we know we have content here. // In this case we need to insert it into the next constant pool. - if (bucket->id != 0) { + if (bucket->id != PM_CONSTANT_ID_UNSET) { uint32_t next_index = bucket->hash & mask; // This implements linear scanning to find the next available slot // in case this index is already taken. We don't need to bother // comparing the values since we know that the hash is unique. - while (next_buckets[next_index].id != 0) { + while (next_buckets[next_index].id != PM_CONSTANT_ID_UNSET) { next_index = (next_index + 1) & mask; } @@ -177,7 +177,7 @@ pm_constant_pool_init(pm_constant_pool_t *pool, uint32_t capacity) { */ pm_constant_t * pm_constant_pool_id_to_constant(const pm_constant_pool_t *pool, pm_constant_id_t constant_id) { - assert(constant_id > 0 && constant_id <= pool->size); + assert(constant_id != PM_CONSTANT_ID_UNSET && constant_id <= pool->size); return &pool->constants[constant_id - 1]; } @@ -187,7 +187,7 @@ pm_constant_pool_id_to_constant(const pm_constant_pool_t *pool, pm_constant_id_t static inline pm_constant_id_t pm_constant_pool_insert(pm_constant_pool_t *pool, const uint8_t *start, size_t length, pm_constant_pool_bucket_type_t type) { if (pool->size >= (pool->capacity / 4 * 3)) { - if (!pm_constant_pool_resize(pool)) return 0; + if (!pm_constant_pool_resize(pool)) return PM_CONSTANT_ID_UNSET; } assert(is_power_of_two(pool->capacity)); @@ -197,7 +197,7 @@ pm_constant_pool_insert(pm_constant_pool_t *pool, const uint8_t *start, size_t l uint32_t index = hash & mask; pm_constant_pool_bucket_t *bucket; - while (bucket = &pool->buckets[index], bucket->id != 0) { + while (bucket = &pool->buckets[index], bucket->id != PM_CONSTANT_ID_UNSET) { // If there is a collision, then we need to check if the content is the // same as the content we are trying to insert. If it is, then we can // return the id of the existing constant. @@ -248,8 +248,8 @@ pm_constant_pool_insert(pm_constant_pool_t *pool, const uint8_t *start, size_t l } /** - * Insert a constant into a constant pool. Returns the id of the constant, or 0 - * if any potential calls to resize fail. + * Insert a constant into a constant pool. Returns the id of the constant, or + * PM_CONSTANT_ID_UNSET if any potential calls to resize fail. */ pm_constant_id_t pm_constant_pool_insert_shared(pm_constant_pool_t *pool, const uint8_t *start, size_t length) { @@ -258,8 +258,8 @@ pm_constant_pool_insert_shared(pm_constant_pool_t *pool, const uint8_t *start, s /** * Insert a constant into a constant pool from memory that is now owned by the - * constant pool. Returns the id of the constant, or 0 if any potential calls to - * resize fail. + * constant pool. Returns the id of the constant, or PM_CONSTANT_ID_UNSET if any + * potential calls to resize fail. */ pm_constant_id_t pm_constant_pool_insert_owned(pm_constant_pool_t *pool, const uint8_t *start, size_t length) { @@ -268,7 +268,8 @@ pm_constant_pool_insert_owned(pm_constant_pool_t *pool, const uint8_t *start, si /** * Insert a constant into a constant pool from memory that is constant. Returns - * the id of the constant, or 0 if any potential calls to resize fail. + * the id of the constant, or PM_CONSTANT_ID_UNSET if any potential calls to + * resize fail. */ pm_constant_id_t pm_constant_pool_insert_constant(pm_constant_pool_t *pool, const uint8_t *start, size_t length) { @@ -286,7 +287,7 @@ pm_constant_pool_free(pm_constant_pool_t *pool) { pm_constant_pool_bucket_t *bucket = &pool->buckets[index]; // If an id is set on this constant, then we know we have content here. - if (bucket->id != 0 && bucket->type == PM_CONSTANT_POOL_BUCKET_OWNED) { + if (bucket->id != PM_CONSTANT_ID_UNSET && bucket->type == PM_CONSTANT_POOL_BUCKET_OWNED) { pm_constant_t *constant = &pool->constants[bucket->id - 1]; free((void *) constant->start); } diff --git a/prism/util/pm_constant_pool.h b/prism/util/pm_constant_pool.h index ae5a88fbde..3743d9f58d 100644 --- a/prism/util/pm_constant_pool.h +++ b/prism/util/pm_constant_pool.h @@ -18,6 +18,13 @@ #include #include +/** + * When we allocate constants into the pool, we reserve 0 to mean that the slot + * is not yet filled. This constant is reused in other places to indicate the + * lack of a constant id. + */ +#define PM_CONSTANT_ID_UNSET 0 + /** * A constant id is a unique identifier for a constant in the constant pool. */