diff --git a/compile.c b/compile.c index 3269645e3d8d85..1d762c485ef17b 100644 --- a/compile.c +++ b/compile.c @@ -1962,6 +1962,10 @@ iseq_set_arguments_keywords(rb_iseq_t *iseq, LINK_ANCHOR *const optargs, kw++; node = node->nd_next; } + if (kw > VM_CALL_KW_LEN_MAX) { + COMPILE_ERROR(ERROR_ARGS_AT(RNODE(args->kw_args)) "too many keyword parameters (%d, maximum is %d)", + kw, (int)VM_CALL_KW_LEN_MAX); + } arg_size += kw; keyword->bits_start = arg_size++; @@ -5075,6 +5079,12 @@ compile_keyword_arg(rb_iseq_t *iseq, LINK_ANCHOR *const ret, { int len = 0; VALUE key_index = node_hash_unique_key_index(iseq, RNODE_HASH(root_node), &len); + + if (len > VM_CALL_KW_LEN_MAX) { + COMPILE_ERROR(ERROR_ARGS_AT(root_node) "too many keyword arguments (%d, maximum is %d)", + len, (int)VM_CALL_KW_LEN_MAX); + } + struct rb_callinfo_kwarg *kw_arg = rb_xmalloc_mul_add(len, sizeof(VALUE), sizeof(struct rb_callinfo_kwarg)); VALUE *keywords = kw_arg->keywords; @@ -9600,7 +9610,7 @@ compile_call(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node, co if (type == NODE_CALL || type == NODE_OPCALL || type == NODE_QCALL) { int idx, level; - if (mid == idCall && + if ((mid == idCall || mid == idAREF || mid == idYield || mid == idEqq) && nd_type_p(get_nd_recv(node), NODE_LVAR) && iseq_block_param_id_p(iseq, RNODE_LVAR(get_nd_recv(node))->nd_vid, &idx, &level)) { ADD_INSN2(recv, get_nd_recv(node), getblockparamproxy, INT2FIX(idx + VM_ENV_DATA_SIZE - 1), INT2FIX(level)); diff --git a/defs/id.def b/defs/id.def index 344b072e7615a3..7848c797dcdf0b 100644 --- a/defs/id.def +++ b/defs/id.def @@ -48,6 +48,7 @@ firstline, predefined = __LINE__+1, %[\ bt bt_locations call + yield mesg exception locals diff --git a/internal/basic_operators.h b/internal/basic_operators.h index 493d2fa7f733d7..e28060944c01dc 100644 --- a/internal/basic_operators.h +++ b/internal/basic_operators.h @@ -40,6 +40,7 @@ enum ruby_basic_operators { BOP_DEFAULT, BOP_PACK, BOP_INCLUDE_P, + BOP_YIELD, BOP_LAST_ }; diff --git a/internal/object.h b/internal/object.h index 837dd7a457f6fd..7538d46b8b343e 100644 --- a/internal/object.h +++ b/internal/object.h @@ -20,8 +20,15 @@ VALUE rb_obj_dig(int argc, VALUE *argv, VALUE self, VALUE notfound); VALUE rb_obj_clone_setup(VALUE obj, VALUE clone, VALUE kwfreeze); VALUE rb_obj_dup_setup(VALUE obj, VALUE dup); VALUE rb_immutable_obj_clone(int, VALUE *, VALUE); -VALUE rb_check_convert_type_with_id(VALUE,int,const char*,ID); +VALUE rb_check_convert_type_with_id_slow(VALUE,int,const char*,ID); int rb_bool_expected(VALUE, const char *, int raise); + +static inline VALUE +rb_check_convert_type_with_id(VALUE val, int type, const char *tname, ID method) +{ + if (RB_TYPE_P(val, type) && type != T_DATA) return val; + return rb_check_convert_type_with_id_slow(val, type, tname, method); +} static inline void RBASIC_CLEAR_CLASS(VALUE obj); static inline void RBASIC_SET_CLASS_RAW(VALUE obj, VALUE klass); static inline void RBASIC_SET_CLASS(VALUE obj, VALUE klass); diff --git a/lib/prism/lex_compat.rb b/lib/prism/lex_compat.rb index 7aacec037da5e0..4d92842bda7515 100644 --- a/lib/prism/lex_compat.rb +++ b/lib/prism/lex_compat.rb @@ -141,6 +141,7 @@ def deconstruct_keys(keys) # :nodoc: KEYWORD_DEFINED: :on_kw, KEYWORD_DO: :on_kw, KEYWORD_DO_BLOCK: :on_kw, + KEYWORD_DO_LAMBDA: :on_kw, KEYWORD_DO_LOOP: :on_kw, KEYWORD_ELSE: :on_kw, KEYWORD_ELSIF: :on_kw, diff --git a/lib/prism/translation/parser/lexer.rb b/lib/prism/translation/parser/lexer.rb index dadc53d38e5d41..0b2f4b9da76cb1 100644 --- a/lib/prism/translation/parser/lexer.rb +++ b/lib/prism/translation/parser/lexer.rb @@ -88,6 +88,7 @@ class Lexer # :nodoc: KEYWORD_DEFINED: :kDEFINED, KEYWORD_DO: :kDO, KEYWORD_DO_BLOCK: :kDO_BLOCK, + KEYWORD_DO_LAMBDA: :kDO_LAMBDA, KEYWORD_DO_LOOP: :kDO_COND, KEYWORD_END: :kEND, KEYWORD_END_UPCASE: :klEND, @@ -192,14 +193,6 @@ class Lexer # :nodoc: EXPR_BEG = 0x1 EXPR_LABEL = 0x400 - # It is used to determine whether `do` is of the token type `kDO` or - # `kDO_LAMBDA`. - # - # NOTE: In edge cases like `-> (foo = -> (bar) {}) do end`, please note - # that `kDO` is still returned instead of `kDO_LAMBDA`, which is - # expected: https://github.com/ruby/prism/pull/3046 - LAMBDA_TOKEN_TYPES = Set.new([:kDO_LAMBDA, :tLAMBDA, :tLAMBEG]) - # The `PARENTHESIS_LEFT` token in Prism is classified as either # `tLPAREN` or `tLPAREN2` in the Parser gem. The following token types # are listed as those classified as `tLPAREN`. @@ -221,7 +214,7 @@ class Lexer # :nodoc: # Heredocs are complex and require us to keep track of a bit of info to refer to later HeredocData = Struct.new(:identifier, :common_whitespace, keyword_init: true) - private_constant :TYPES, :EXPR_BEG, :EXPR_LABEL, :LAMBDA_TOKEN_TYPES, :LPAREN_CONVERSION_TOKEN_TYPES, :HeredocData + private_constant :TYPES, :EXPR_BEG, :EXPR_LABEL, :LPAREN_CONVERSION_TOKEN_TYPES, :HeredocData # The Parser::Source::Buffer that the tokens were lexed from. attr_reader :source_buffer @@ -269,14 +262,6 @@ def to_a location = range(token.location.start_offset, token.location.end_offset) case type - when :kDO - nearest_lambda_token = tokens.reverse_each.find do |token| - LAMBDA_TOKEN_TYPES.include?(token.first) - end - - if nearest_lambda_token&.first == :tLAMBDA - type = :kDO_LAMBDA - end when :tCHARACTER value.delete_prefix!("?") # Character literals behave similar to double-quoted strings. We can use the same escaping mechanism. diff --git a/object.c b/object.c index 0552e7cf44019a..5a3203d4e1a567 100644 --- a/object.c +++ b/object.c @@ -3358,13 +3358,9 @@ rb_check_convert_type(VALUE val, int type, const char *tname, const char *method /*! \private */ VALUE -rb_check_convert_type_with_id(VALUE val, int type, const char *tname, ID method) +rb_check_convert_type_with_id_slow(VALUE val, int type, const char *tname, ID method) { - VALUE v; - - /* always convert T_DATA */ - if (TYPE(val) == type && type != T_DATA) return val; - v = convert_type_with_id(val, tname, method, FALSE, -1); + VALUE v = convert_type_with_id(val, tname, method, FALSE, -1); if (NIL_P(v)) return Qnil; if (TYPE(v) != type) { rb_cant_convert_invalid_return(val, tname, rb_id2name(method), v); diff --git a/prism/config.yml b/prism/config.yml index e22d2f2ef5835e..a27cc829b9e7fb 100644 --- a/prism/config.yml +++ b/prism/config.yml @@ -496,6 +496,8 @@ tokens: comment: "defined?" - name: KEYWORD_DO_BLOCK comment: "do keyword for a block attached to a command" + - name: KEYWORD_DO_LAMBDA + comment: "do keyword that opens the body of a lambda literal" - name: KEYWORD_DO_LOOP comment: "do keyword for a predicate in a while, until, or for loop" - name: KEYWORD_END_UPCASE diff --git a/prism/prism.c b/prism/prism.c index a7e661ca169eba..19af4f4a259805 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -7405,6 +7405,42 @@ pm_do_loop_stack_p(pm_parser_t *parser) { return pm_state_stack_p(&parser->do_loop_stack); } +/** + * When the lexer finds an opening delimiter (`(`, `[`, `{`, or `#{`) it pushes + * an enclosure frame onto both bit-stacks that gate how a subsequent `do` is + * lexed: the do-loop stack (so a `do` inside is not read as a `while`/`until` + * loop body) and the accepts-block stack (so a `do` inside is accepted as a + * block rather than binding to an enclosing command). The matching close pops + * both. This mirrors parse.y's paired `COND_PUSH(0); CMDARG_PUSH(0)`. Keep the + * following in mind before touching either stack: + * + * - The LEXER owns the frame for delimiter-bound stuff. Every token that opens + * a matched pair calls this on the open and `pm_enclosure_frame_pop` on the + * close. The exhaustive push sites are the `(`, `[`, `{` cases in + * `parser_lex` and the `#{` case in `lex_interpolation`; the pop sites are + * the matching `)`, `]`, `}` (including `}` as `EMBEXPR_END`). + * - The PARSER owns the frame for keyword-bounded blocks, where there is no + * delimiter token to hang it on: the `do`/`end` forms in `parse_block` and + * the lambda push `accepts_block` directly (and pop before consuming `end`). + * - The `parse_arguments_list` command-args branch juggles ONLY `accepts_block` + * (never through this helper), because the lexer has pushed a delimiter frame + * one token early and the command-args frame must be threaded beneath it. Its + * match lookahead sets must stay in sync with the lexer push sites above. + * This matches parse.y, whose `command_args` rule juggles CMDARG but not + * COND. + */ +static PRISM_INLINE void +pm_enclosure_frame_push(pm_parser_t *parser) { + pm_do_loop_stack_push(parser, false); + pm_accepts_block_stack_push(parser, true); +} + +static PRISM_INLINE void +pm_enclosure_frame_pop(pm_parser_t *parser) { + pm_do_loop_stack_pop(parser); + pm_accepts_block_stack_pop(parser); +} + /******************************************************************************/ /* Lexer check helpers */ /******************************************************************************/ @@ -8578,12 +8614,41 @@ lex_identifier(pm_parser_t *parser, bool previous_command_start) { if (parser->lex_state != PM_LEX_STATE_DOT) { pm_token_type_t type; + + /* The lex state from before lex_keyword transitions it, mirroring the + * `state = p->lex.state` capture in parse.y's keyword handling. */ + pm_lex_state_t previous_lex_state = parser->lex_state; + switch (width) { case 2: if (lex_keyword(parser, current_start, "do", width, PM_LEX_STATE_BEG, PM_TOKEN_KEYWORD_DO, PM_TOKEN_EOF) != PM_TOKEN_EOF) { - if (parser->enclosure_nesting == parser->lambda_enclosure_nesting) { + /* In FNAME position (a symbol like `:do` or a method name + * like `def do`), `do` is a plain name rather than a + * block, loop, or lambda opener, so none of the + * discrimination below applies. This mirrors parse.y, + * whose EXPR_FNAME early-return precedes all of the + * keyword_do special-casing (and never touches + * lpar_beg). */ + if (previous_lex_state & PM_LEX_STATE_FNAME) { return PM_TOKEN_KEYWORD_DO; } + if (parser->enclosure_nesting == parser->lambda_enclosure_nesting) { + // At the bare nesting level of a lambda literal (no + // delimiter opened since `->`), a `do` opens the lambda + // body. This is a distinct token so that a command in a + // parameter default cannot consume it as its own block + // (`-> a = foo do end` is `->(a = foo) do end`). It + // mirrors CRuby's keyword_do_LAMBDA. + // + // Clear the nesting so that no token within the + // `do`/`end` body is considered to be at the beginning + // of a lambda; the parser restores the enclosing value + // once the lambda has been fully parsed. This mirrors + // parse.y setting `p->lex.lpar_beg = -1` when lexing + // keyword_do_LAMBDA. + parser->lambda_enclosure_nesting = -1; + return PM_TOKEN_KEYWORD_DO_LAMBDA; + } if (pm_do_loop_stack_p(parser)) { return PM_TOKEN_KEYWORD_DO_LOOP; } @@ -8789,7 +8854,7 @@ lex_interpolation(pm_parser_t *parser, const uint8_t *pound) { lex_mode_push(parser, (pm_lex_mode_t) { .mode = PM_LEX_EMBEXPR }); parser->current.end = pound + 2; parser->command_start = true; - pm_do_loop_stack_push(parser, false); + pm_enclosure_frame_push(parser); return PM_TOKEN_EMBEXPR_BEGIN; default: // In this case we've hit a # that doesn't constitute interpolation. We'll @@ -10394,7 +10459,7 @@ parser_lex(pm_parser_t *parser) { parser->enclosure_nesting++; lex_state_set(parser, PM_LEX_STATE_BEG | PM_LEX_STATE_LABEL); - pm_do_loop_stack_push(parser, false); + pm_enclosure_frame_push(parser); LEX(type); } @@ -10402,7 +10467,7 @@ parser_lex(pm_parser_t *parser) { case ')': parser->enclosure_nesting--; lex_state_set(parser, PM_LEX_STATE_ENDFN); - pm_do_loop_stack_pop(parser); + pm_enclosure_frame_pop(parser); LEX(PM_TOKEN_PARENTHESIS_RIGHT); // ; @@ -10432,14 +10497,14 @@ parser_lex(pm_parser_t *parser) { } lex_state_set(parser, PM_LEX_STATE_BEG | PM_LEX_STATE_LABEL); - pm_do_loop_stack_push(parser, false); + pm_enclosure_frame_push(parser); LEX(type); // ] case ']': parser->enclosure_nesting--; lex_state_set(parser, PM_LEX_STATE_END); - pm_do_loop_stack_pop(parser); + pm_enclosure_frame_pop(parser); LEX(PM_TOKEN_BRACKET_RIGHT); // { @@ -10469,7 +10534,7 @@ parser_lex(pm_parser_t *parser) { parser->enclosure_nesting++; parser->brace_nesting++; - pm_do_loop_stack_push(parser, false); + pm_enclosure_frame_push(parser); LEX(type); } @@ -10477,7 +10542,7 @@ parser_lex(pm_parser_t *parser) { // } case '}': parser->enclosure_nesting--; - pm_do_loop_stack_pop(parser); + pm_enclosure_frame_pop(parser); if ((parser->lex_modes.current->mode == PM_LEX_EMBEXPR) && (parser->brace_nesting == 0)) { lex_mode_pop(parser); @@ -12824,6 +12889,7 @@ token_begins_expression_p(pm_token_type_t type) { case PM_TOKEN_LAMBDA_BEGIN: case PM_TOKEN_KEYWORD_DO: case PM_TOKEN_KEYWORD_DO_BLOCK: + case PM_TOKEN_KEYWORD_DO_LAMBDA: case PM_TOKEN_KEYWORD_DO_LOOP: case PM_TOKEN_KEYWORD_END: case PM_TOKEN_KEYWORD_ELSE: @@ -15264,7 +15330,12 @@ parse_block(pm_parser_t *parser, uint16_t depth) { pm_token_t opening = parser->previous; accept1(parser, PM_TOKEN_NEWLINE); - pm_accepts_block_stack_push(parser, true); + /* A brace block is delimited by `{`/`}`, whose block-accepting frame is + * managed by the lexer. A `do`/`end` block is delimited by keywords, so we + * push the frame here (covering the block parameters and body) and pop it + * before consuming `end`, mirroring parse.y's `do_body` rule. */ + bool do_block = opening.type != PM_TOKEN_BRACE_LEFT; + if (do_block) pm_accepts_block_stack_push(parser, true); pm_parser_scope_push(parser, false); pm_block_parameters_node_t *block_parameters = NULL; @@ -15293,19 +15364,11 @@ parse_block(pm_parser_t *parser, uint16_t depth) { statements = UP(parse_statements(parser, PM_CONTEXT_BLOCK_BRACES, (uint16_t) (depth + 1))); } - /* Pop before consuming the closing `}` so the following token (e.g. a - * `do`) is lexed in the enclosing context rather than as a block - * belonging to this block's interior. Otherwise a `do` block would - * wrongly bind to a command whose argument ends in a brace block, as in - * `foo(m a { } do end)`. */ - pm_accepts_block_stack_pop(parser); expect1_opening(parser, PM_TOKEN_BRACE_RIGHT, PM_ERR_BLOCK_TERM_BRACE, &opening); } else { if (!match1(parser, PM_TOKEN_KEYWORD_END)) { if (!match3(parser, PM_TOKEN_KEYWORD_RESCUE, PM_TOKEN_KEYWORD_ELSE, PM_TOKEN_KEYWORD_ENSURE)) { - pm_accepts_block_stack_push(parser, true); statements = UP(parse_statements(parser, PM_CONTEXT_BLOCK_KEYWORDS, (uint16_t) (depth + 1))); - pm_accepts_block_stack_pop(parser); } if (match2(parser, PM_TOKEN_KEYWORD_RESCUE, PM_TOKEN_KEYWORD_ENSURE)) { @@ -15314,7 +15377,8 @@ parse_block(pm_parser_t *parser, uint16_t depth) { } } - /* As with the brace case above, pop before consuming `end`. */ + /* Pop the `do`/`end` frame before consuming `end` so the token + * following the block is lexed in the enclosing context. */ pm_accepts_block_stack_pop(parser); expect1_opening(parser, PM_TOKEN_KEYWORD_END, PM_ERR_BLOCK_TERM_END, &opening); } @@ -15324,7 +15388,6 @@ parse_block(pm_parser_t *parser, uint16_t depth) { pm_node_t *parameters = parse_blocklike_parameters(parser, UP(block_parameters), &opening, &parser->previous); pm_parser_scope_pop(parser); - return pm_block_node_create(parser, &locals, &opening, parameters, statements, &parser->previous); } @@ -15361,7 +15424,6 @@ parse_arguments_list(pm_parser_t *parser, pm_arguments_t *arguments, bool full_a if (accept1(parser, PM_TOKEN_PARENTHESIS_RIGHT)) { arguments->closing_loc = TOK2LOC(parser, &parser->previous); } else { - pm_accepts_block_stack_push(parser, true); parse_arguments(parser, arguments, full_arguments, PM_TOKEN_PARENTHESIS_RIGHT, (uint8_t) (flags & ~PM_PARSE_ACCEPTS_DO_BLOCK), (uint16_t) (depth + 1)); // `yield` parses its arguments through the restricted `call_args` @@ -15380,13 +15442,24 @@ parse_arguments_list(pm_parser_t *parser, pm_arguments_t *arguments, bool full_a parser->previous.type = 0; } - pm_accepts_block_stack_pop(parser); arguments->closing_loc = TOK2LOC(parser, &parser->previous); } } else if ((flags & PM_PARSE_ACCEPTS_COMMAND_CALL) && (token_begins_expression_p(parser->current.type) || match3(parser, PM_TOKEN_USTAR, PM_TOKEN_USTAR_STAR, PM_TOKEN_UAMPERSAND)) && !match1(parser, PM_TOKEN_BRACE_LEFT)) { found |= true; parsed_command_args = true; + + /* The command-args frame does not accept blocks, so that a trailing + * `do` binds to this command rather than to an argument. Mirroring + * parse.y's `command_args` rule: when the first argument begins with an + * opening delimiter, the lexer has already pushed that delimiter's + * (block-accepting) frame. We must push the command-args frame beneath + * it, so pop the delimiter frame, push the command-args frame, and then + * restore the delimiter frame on top (the delimiter's closing token + * will pop it back off during argument parsing). */ + bool lookahead_delimiter = match4(parser, PM_TOKEN_PARENTHESIS_LEFT, PM_TOKEN_PARENTHESIS_LEFT_PARENTHESES, PM_TOKEN_BRACKET_LEFT, PM_TOKEN_BRACKET_LEFT_ARRAY); + if (lookahead_delimiter) pm_accepts_block_stack_pop(parser); pm_accepts_block_stack_push(parser, false); + if (lookahead_delimiter) pm_accepts_block_stack_push(parser, true); // If we get here, then the subsequent token cannot be used as an infix // operator. In this case we assume the subsequent token is part of an @@ -15400,7 +15473,15 @@ parse_arguments_list(pm_parser_t *parser, pm_arguments_t *arguments, bool full_a PM_PARSER_ERR_TOKEN_FORMAT(parser, &parser->previous, PM_ERR_EXPECT_ARGUMENT, pm_token_str(parser->current.type)); } + /* Symmetrically, if the command arguments are followed by a brace block + * (`m args { }`), the lexer has already pushed that block's frame. Pop + * it, pop the command-args frame beneath it, and restore the block + * frame so the block's `}` still pops it. This mirrors the `tLBRACE_ARG` + * lookahead handling in parse.y's `command_args` rule. */ + bool lookahead_brace = match1(parser, PM_TOKEN_BRACE_LEFT); + if (lookahead_brace) pm_accepts_block_stack_pop(parser); pm_accepts_block_stack_pop(parser); + if (lookahead_brace) pm_accepts_block_stack_push(parser, true); } // If we're at the end of the arguments, we can now check if there is a block @@ -15841,7 +15922,7 @@ parse_conditional(pm_parser_t *parser, pm_context_t context, size_t opening_newl #define PM_CASE_KEYWORD PM_TOKEN_KEYWORD___ENCODING__: case PM_TOKEN_KEYWORD___FILE__: case PM_TOKEN_KEYWORD___LINE__: \ case PM_TOKEN_KEYWORD_ALIAS: case PM_TOKEN_KEYWORD_AND: case PM_TOKEN_KEYWORD_BEGIN: case PM_TOKEN_KEYWORD_BEGIN_UPCASE: \ case PM_TOKEN_KEYWORD_BREAK: case PM_TOKEN_KEYWORD_CASE: case PM_TOKEN_KEYWORD_CLASS: case PM_TOKEN_KEYWORD_DEF: \ - case PM_TOKEN_KEYWORD_DEFINED: case PM_TOKEN_KEYWORD_DO: case PM_TOKEN_KEYWORD_DO_BLOCK: case PM_TOKEN_KEYWORD_DO_LOOP: case PM_TOKEN_KEYWORD_ELSE: \ + case PM_TOKEN_KEYWORD_DEFINED: case PM_TOKEN_KEYWORD_DO: case PM_TOKEN_KEYWORD_DO_BLOCK: case PM_TOKEN_KEYWORD_DO_LAMBDA: case PM_TOKEN_KEYWORD_DO_LOOP: case PM_TOKEN_KEYWORD_ELSE: \ case PM_TOKEN_KEYWORD_ELSIF: case PM_TOKEN_KEYWORD_END: case PM_TOKEN_KEYWORD_END_UPCASE: case PM_TOKEN_KEYWORD_ENSURE: \ case PM_TOKEN_KEYWORD_FALSE: case PM_TOKEN_KEYWORD_FOR: case PM_TOKEN_KEYWORD_IF: case PM_TOKEN_KEYWORD_IN: \ case PM_TOKEN_KEYWORD_MODULE: case PM_TOKEN_KEYWORD_NEXT: case PM_TOKEN_KEYWORD_NIL: case PM_TOKEN_KEYWORD_NOT: \ @@ -15964,9 +16045,7 @@ parse_string_part(pm_parser_t *parser, uint16_t depth) { pm_statements_node_t *statements = NULL; if (!match3(parser, PM_TOKEN_EMBEXPR_END, PM_TOKEN_HEREDOC_END, PM_TOKEN_EOF)) { - pm_accepts_block_stack_push(parser, true); statements = parse_statements(parser, PM_CONTEXT_EMBEXPR, (uint16_t) (depth + 1)); - pm_accepts_block_stack_pop(parser); } parser->brace_nesting = brace_nesting; @@ -18527,10 +18606,13 @@ parse_def(pm_parser_t *parser, pm_binding_power_t binding_power, uint8_t flags, * for primary-level constructs, not commands). During command argument * parsing, the stack is pushed to false, causing `do` to be lexed as * PM_TOKEN_KEYWORD_DO_BLOCK, which is not consumed inside the endless - * def body and instead left for the outer context. */ + * def body and instead left for the outer context. A method definition + * opens a fresh context all the way through its rescue modifier, so + * this frame spans the rescue modifier value as well: the `do` in + * `baz def f = a rescue z do end` lexes as a plain keyword that + * attaches to `z` rather than to `baz`. */ pm_accepts_block_stack_push(parser, true); pm_node_t *statement = parse_expression(parser, PM_BINDING_POWER_DEFINED + 1, allow_flags | PM_PARSE_IN_ENDLESS_DEF, PM_ERR_DEF_ENDLESS, (uint16_t) (depth + 1)); - pm_accepts_block_stack_pop(parser); /* If an unconsumed PM_TOKEN_KEYWORD_DO follows the body, it is an error * (e.g., `def f = 1 do end`). PM_TOKEN_KEYWORD_DO_BLOCK is @@ -18542,7 +18624,11 @@ parse_def(pm_parser_t *parser, pm_binding_power_t binding_power, uint8_t flags, pm_parser_err_node(parser, UP(block), PM_ERR_DEF_ENDLESS_DO_BLOCK); } - if (accept1(parser, PM_TOKEN_KEYWORD_RESCUE_MODIFIER)) { + /* Any number of rescue modifiers chain onto the body within the method + * definition itself, associating to the left: `def f = a rescue b + * rescue c` defines a method whose body is `(a rescue b) rescue c`, + * rather than a rescue modifier guarding the definition. */ + while (accept1(parser, PM_TOKEN_KEYWORD_RESCUE_MODIFIER)) { context_push(parser, PM_CONTEXT_RESCUE_MODIFIER); pm_token_t rescue_keyword = parser->previous; @@ -18555,6 +18641,8 @@ parse_def(pm_parser_t *parser, pm_binding_power_t binding_power, uint8_t flags, statement = UP(pm_rescue_modifier_node_create(parser, statement, &rescue_keyword, value)); } + pm_accepts_block_stack_pop(parser); + /* A nested endless def whose body is a command call (e.g., * `def f = def g = foo bar`) is a command assignment and cannot appear * as a def body. */ @@ -19031,7 +19119,6 @@ parse_parentheses(pm_parser_t *parser, pm_binding_power_t binding_power, uint8_t /* Otherwise, we're going to parse the first statement in the list of * statements within the parentheses. */ - pm_accepts_block_stack_push(parser, true); context_push(parser, PM_CONTEXT_PARENS); pm_node_t *statement = parse_expression(parser, PM_BINDING_POWER_STATEMENT, PM_PARSE_ACCEPTS_COMMAND_CALL | PM_PARSE_ACCEPTS_DO_BLOCK, PM_ERR_CANNOT_PARSE_EXPRESSION, (uint16_t) (depth + 1)); context_pop(parser); @@ -19065,10 +19152,6 @@ parse_parentheses(pm_parser_t *parser, pm_binding_power_t binding_power, uint8_t lex_state_set(parser, PM_LEX_STATE_ENDARG); } - /* Pop before consuming the closing `)` so the following token (e.g. a - * `do`) is lexed in the enclosing context rather than as a block - * belonging to the parenthesized expression. */ - pm_accepts_block_stack_pop(parser); parser_lex(parser); pop_block_exits(parser, previous_block_exits); @@ -19181,7 +19264,6 @@ parse_parentheses(pm_parser_t *parser, pm_binding_power_t binding_power, uint8_t } context_pop(parser); - pm_accepts_block_stack_pop(parser); expect1(parser, PM_TOKEN_PARENTHESIS_RIGHT, PM_ERR_EXPECT_RPAREN); /* When we're parsing multi targets, we allow them to be followed by a right @@ -19243,7 +19325,6 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, u parser_lex(parser); pm_array_node_t *array = pm_array_node_create(parser, &parser->previous); - pm_accepts_block_stack_push(parser, true); bool parsed_bare_hash = false; while (!match2(parser, PM_TOKEN_BRACKET_RIGHT, PM_TOKEN_EOF)) { @@ -19341,13 +19422,6 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, u accept1(parser, PM_TOKEN_NEWLINE); - /* Pop before consuming the closing `]` so the following token (e.g. - * a `do`) is lexed in the enclosing context rather than as a block - * belonging to the array's interior. Otherwise a `do` block would - * wrongly bind to a command with an array argument, as in - * `foo(m [] do end)`. */ - pm_accepts_block_stack_pop(parser); - if (!accept1(parser, PM_TOKEN_BRACKET_RIGHT)) { PM_PARSER_ERR_TOKEN_FORMAT(parser, &parser->current, PM_ERR_ARRAY_TERM, pm_token_str(parser->current.type)); parser->previous.start = parser->previous.end; @@ -19372,7 +19446,6 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, u pm_static_literals_t *current_hash_keys = parser->current_hash_keys; parser->current_hash_keys = NULL; - pm_accepts_block_stack_push(parser, true); parser_lex(parser); pm_token_t opening = parser->previous; @@ -19390,7 +19463,6 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, u accept1(parser, PM_TOKEN_NEWLINE); } - pm_accepts_block_stack_pop(parser); expect1_opening(parser, PM_TOKEN_BRACE_RIGHT, PM_ERR_HASH_TERM, &opening); pm_hash_node_closing_loc_set(parser, node, &parser->previous); @@ -20624,7 +20696,6 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, u parser->lambda_enclosure_nesting = parser->enclosure_nesting; size_t opening_newline_index = token_newline_index(parser); - pm_accepts_block_stack_push(parser, true); parser_lex(parser); pm_token_t operator = parser->previous; @@ -20650,9 +20721,7 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, u break; } case PM_CASE_PARAMETER: { - pm_accepts_block_stack_push(parser, false); block_parameters = parse_block_parameters(parser, false, NULL, true, false, (uint16_t) (depth + 1)); - pm_accepts_block_stack_pop(parser); break; } default: { @@ -20663,7 +20732,6 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, u pm_token_t opening; pm_node_t *body = NULL; - parser->lambda_enclosure_nesting = previous_lambda_enclosure_nesting; if (accept1(parser, PM_TOKEN_LAMBDA_BEGIN)) { opening = parser->previous; @@ -20674,15 +20742,31 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, u parser_warn_indentation_mismatch(parser, opening_newline_index, &operator, false, false); - /* Pop before consuming the closing `}` so the following token - * (e.g. a `do`) is lexed in the enclosing context rather than - * as a block belonging to the lambda's interior. */ - pm_accepts_block_stack_pop(parser); + /* Restore the enclosing lambda's nesting now that the body has + * been parsed, so that the token following the closing `}` is + * lexed in the enclosing context. During the body the nesting + * held this lambda's own level, which every token inside the + * braces sits above. This mirrors parse.y restoring + * `p->lex.lpar_beg` after `lambda_body`. */ + parser->lambda_enclosure_nesting = previous_lambda_enclosure_nesting; expect1_opening(parser, PM_TOKEN_BRACE_RIGHT, PM_ERR_LAMBDA_TERM_BRACE, &opening); } else { - expect1(parser, PM_TOKEN_KEYWORD_DO, PM_ERR_LAMBDA_OPEN); + /* A `-> { }` body is delimited by `{`/`}`, whose block-accepting + * frame the lexer manages. A `-> do end` body is delimited by + * keywords, so push the frame here and pop it before `end`. The + * push must precede consuming the `do`, which lexes the first + * token of the body; this matches parse.y's CMDARG_PUSH(0) + * before `lambda_body`. */ + pm_accepts_block_stack_push(parser, true); + expect1(parser, PM_TOKEN_KEYWORD_DO_LAMBDA, PM_ERR_LAMBDA_OPEN); opening = parser->previous; + /* The lexer cleared the nesting when it produced the `do`. If + * it was missing entirely, clear it here so that the body is + * recovered the same way it would have been parsed: no token + * within it sits at the beginning of a lambda. */ + parser->lambda_enclosure_nesting = -1; + if (!match3(parser, PM_TOKEN_KEYWORD_END, PM_TOKEN_KEYWORD_RESCUE, PM_TOKEN_KEYWORD_ENSURE)) { body = UP(parse_statements(parser, PM_CONTEXT_LAMBDA_DO_END, (uint16_t) (depth + 1))); } @@ -20694,8 +20778,12 @@ parse_expression_prefix(pm_parser_t *parser, pm_binding_power_t binding_power, u parser_warn_indentation_mismatch(parser, opening_newline_index, &operator, false, false); } - /* As with the brace case above, pop before consuming `end`. */ pm_accepts_block_stack_pop(parser); + + /* As with the brace branch above, restore the nesting before + * consuming the closing `end`, which lexes the token that + * follows it. */ + parser->lambda_enclosure_nesting = previous_lambda_enclosure_nesting; expect1_opening(parser, PM_TOKEN_KEYWORD_END, PM_ERR_LAMBDA_TERM_END, &operator); } @@ -22138,9 +22226,7 @@ parse_expression_infix(pm_parser_t *parser, pm_node_t *node, pm_binding_power_t arguments.opening_loc = TOK2LOC(parser, &parser->previous); if (!accept1(parser, PM_TOKEN_BRACKET_RIGHT)) { - pm_accepts_block_stack_push(parser, true); parse_arguments(parser, &arguments, false, PM_TOKEN_BRACKET_RIGHT, (uint8_t) (flags & ~PM_PARSE_ACCEPTS_DO_BLOCK), (uint16_t) (depth + 1)); - pm_accepts_block_stack_pop(parser); expect1(parser, PM_TOKEN_BRACKET_RIGHT, PM_ERR_EXPECT_RBRACKET); } @@ -22279,9 +22365,9 @@ parse_expression_terminator(pm_parser_t *parser, pm_node_t *node) { if (pm_command_call_value_p(parser, node)) { return left > PM_BINDING_POWER_COMPOSITION; } + /* A super carrying a do-block is a block call, so it may also be - * followed by call chaining (`.`, `::`, `&.`). - */ + * followed by call chaining (`.`, `::`, `&.`). */ if (pm_block_call_p(node)) { return left > PM_BINDING_POWER_COMPOSITION && left < PM_BINDING_POWER_CALL; } diff --git a/prism/templates/src/tokens.c.erb b/prism/templates/src/tokens.c.erb index 1e829547382526..472c82ea690939 100644 --- a/prism/templates/src/tokens.c.erb +++ b/prism/templates/src/tokens.c.erb @@ -169,6 +169,8 @@ pm_token_str(pm_token_type_t token_type) { return "'do'"; case PM_TOKEN_KEYWORD_DO_BLOCK: return "'do'"; + case PM_TOKEN_KEYWORD_DO_LAMBDA: + return "'do'"; case PM_TOKEN_KEYWORD_DO_LOOP: return "'do'"; case PM_TOKEN_KEYWORD_ELSE: diff --git a/prism_compile.c b/prism_compile.c index e70477a51c786e..6a0422c4f4daab 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -1776,6 +1776,11 @@ pm_setup_args_core(const pm_arguments_node_t *arguments_node, const pm_node_t *b size++; } + if (size > VM_CALL_KW_LEN_MAX) { + COMPILE_ERROR(iseq, node_location->line, "too many keyword arguments (%d, maximum is %d)", + (int) size, (int) VM_CALL_KW_LEN_MAX); + } + *kw_arg = rb_xmalloc_mul_add(size, sizeof(VALUE), sizeof(struct rb_callinfo_kwarg)); *flags |= VM_CALL_KWARG; @@ -6601,6 +6606,11 @@ pm_compile_scope_node(rb_iseq_t *iseq, pm_scope_node_t *scope_node, const pm_nod // ^^^^^^^^ // Keywords create an internal variable on the parse tree if (keywords_list && keywords_list->size) { + if (keywords_list->size > VM_CALL_KW_LEN_MAX) { + COMPILE_ERROR(iseq, node_location->line, "too many keyword parameters (%d, maximum is %d)", + (int) keywords_list->size, (int) VM_CALL_KW_LEN_MAX); + } + keyword = ZALLOC_N(struct rb_iseq_param_keyword, 1); keyword->num = (int) keywords_list->size; @@ -7500,7 +7510,7 @@ pm_compile_call_node(rb_iseq_t *iseq, const pm_call_node_t *node, LINK_ANCHOR *c PUSH_INSN(ret, location, putself); } else { - if (method_id == idCall && PM_NODE_TYPE_P(node->receiver, PM_LOCAL_VARIABLE_READ_NODE)) { + if ((method_id == idCall || method_id == idAREF || method_id == idYield || method_id == idEqq) && PM_NODE_TYPE_P(node->receiver, PM_LOCAL_VARIABLE_READ_NODE)) { const pm_local_variable_read_node_t *read_node_cast = (const pm_local_variable_read_node_t *) node->receiver; uint32_t node_id = node->receiver->node_id; int idx, level; diff --git a/test/prism/errors/do_block_after_command_paren_call.txt b/test/prism/errors/do_block_after_command_paren_call.txt new file mode 100644 index 00000000000000..9f6eaa370ad505 --- /dev/null +++ b/test/prism/errors/do_block_after_command_paren_call.txt @@ -0,0 +1,14 @@ +foo(m a(1) do end) + ^~ unexpected 'do'; expected a `)` to close the arguments + ^~ unexpected 'do', expecting end-of-input + ^~ unexpected 'do', ignoring it + ^~~ unexpected 'end', ignoring it + ^ unexpected ')', ignoring it + +foo(m a.b(1) do end) + ^~ unexpected 'do'; expected a `)` to close the arguments + ^~ unexpected 'do', expecting end-of-input + ^~ unexpected 'do', ignoring it + ^~~ unexpected 'end', ignoring it + ^ unexpected ')', ignoring it + diff --git a/test/prism/fixtures/endless_method_rescue_modifier.txt b/test/prism/fixtures/endless_method_rescue_modifier.txt new file mode 100644 index 00000000000000..f9229fd42fb696 --- /dev/null +++ b/test/prism/fixtures/endless_method_rescue_modifier.txt @@ -0,0 +1,11 @@ +def f = a rescue b + +def f = a rescue b rescue c + +baz def f = a rescue z do end + +bar(baz def f = a rescue z do end) + +foo(def f = x rescue y rescue y do end) + +baz def f = a rescue b rescue c { } diff --git a/test/prism/ruby/ruby_parser_test.rb b/test/prism/ruby/ruby_parser_test.rb index 0a89e784f623f4..600f97c80ba067 100644 --- a/test/prism/ruby/ruby_parser_test.rb +++ b/test/prism/ruby/ruby_parser_test.rb @@ -37,6 +37,7 @@ class RubyParserTest < TestCase "alias.txt", "dsym_str.txt", "dos_endings.txt", + "endless_method_rescue_modifier.txt", "heredoc_dedent_line_continuation.txt", "heredoc_percent_q_newline_delimiter.txt", "heredocs_with_fake_newlines.txt", diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb index c5236b065a1256..1fb19c61326a7f 100644 --- a/test/ruby/test_keyword.rb +++ b/test/ruby/test_keyword.rb @@ -4587,6 +4587,64 @@ def test_splat_arg_with_prohibited_keyword assert_equal([1, 2], m_bug20570(*[1, 2], **nil)) end + def test_keyword_only_method_rejects_positional_after_cache_primed_bug_22099 + c = Class.new do + def bar(a: nil, b: nil) + [a, b] + end + end + obj = c.new + + # Prime the per-class call cache with a valid keyword-only call. + assert_equal([1, 2], obj.bar(a: 1, b: 2)) + + # Different call sites with the same argc/flag but a positional argument + # must still raise, not reuse the cached keyword-setup fastpath and bind + # the positional value to the first keyword. + assert_raise(ArgumentError) { obj.bar(99, a: 0) } + assert_raise(ArgumentError) { obj.bar(99, b: 0) } + + # The cache must compare argc and keyword count separately, not their sum. + assert_raise(ArgumentError) { obj.bar(7, 8, a: 1) } + end + + def test_keyword_call_cache_resolves_keywords_by_name_bug_22099 + c = Class.new do + def bar(a: nil, b: nil, cc: nil) + [a, b, cc] + end + end + obj = c.new + + # Call sites sharing argc/flag/keyword-count must still bind each keyword + # by name (the cached fastpath reads the keyword names from the live call). + assert_equal([1, nil, nil], obj.bar(a: 1)) + assert_equal([nil, 2, nil], obj.bar(b: 2)) + assert_equal([nil, nil, 3], obj.bar(cc: 3)) + assert_equal([1, 2, nil], obj.bar(a: 1, b: 2)) + assert_equal([8, 9, nil], obj.bar(b: 9, a: 8)) + assert_raise(ArgumentError) { obj.bar(z: 1) } + end + + def test_too_many_keywords_rejected_at_compile_time + # The per-class call cache stores the keyword count in an unsigned short + # (rb_class_cc_entries_entry.kw_len), so a call site or method may not use + # more than that many keyword arguments. It is rejected when compiling. + max = 65535 + + over_def = "def m(" + Array.new(max + 1) {|i| "k#{i}:" }.join(", ") + "); end" + err = assert_raise(SyntaxError) { RubyVM::InstructionSequence.compile(over_def) } + assert_match(/too many keyword parameters/, err.message) + + over_call = "m(" + Array.new(max + 1) {|i| "k#{i}: 0" }.join(", ") + ")" + err = assert_raise(SyntaxError) { RubyVM::InstructionSequence.compile(over_call) } + assert_match(/too many keyword arguments/, err.message) + + # The maximum itself is still accepted. + ok_def = "def m(" + Array.new(max) {|i| "k#{i}:" }.join(", ") + "); end" + assert_nothing_raised(SyntaxError) { RubyVM::InstructionSequence.compile(ok_def) } + end + private def one 1 end diff --git a/test/ruby/test_optimization.rb b/test/ruby/test_optimization.rb index 4e9fd5cf0bdd27..5d26e16b44f43d 100644 --- a/test/ruby/test_optimization.rb +++ b/test/ruby/test_optimization.rb @@ -945,6 +945,82 @@ def foo &b END end + def bptest_call(&b) = b.call(1, 2) + def bptest_aref(&b) = b[1, 2] + def bptest_dot_yield(&b) = b.yield(1, 2) + def bptest_eqq(&b) = b === 2 + + def test_block_param_proxy_aref_yield_and_eqq + %i[bptest_call bptest_aref bptest_dot_yield bptest_eqq].each do |name| + assert_include(RubyVM::InstructionSequence.of(method(name)).disasm, + "getblockparamproxy", "#{name} should use the block param proxy") + end + assert_equal(3, bptest_call {|a, b| a + b}) + assert_equal(3, bptest_aref {|a, b| a + b}) + assert_equal(3, bptest_dot_yield {|a, b| a + b}) + assert_equal(3, bptest_eqq {|a| a + 1}) + end + + def test_block_param_proxy_should_not_create_objects + assert_separately [], <<-END + def viacall(&b) = b.call + def viaaref(&b) = b[] + def viayield(&b) = b.yield + def viaeqq(&b) = b === 1 + viacall{}; viaaref{}; viayield{}; viaeqq{} # warm up caches + h1 = {}; h2 = {} + ObjectSpace.count_objects(h1) + GC.start; GC.disable + ObjectSpace.count_objects(h1) + 1000.times { viacall{}; viaaref{}; viayield{}; viaeqq{} } + ObjectSpace.count_objects(h2) + + # The proxy avoids materializing a Proc (T_DATA), so allocations must + # not scale with the number of calls. + assert_operator(h2[:T_DATA] - h1[:T_DATA], :<, 1000) + END + end + + def test_block_param_proxy_honors_redefinition + assert_separately [], <<-END + $VERBOSE = nil # silence "method redefined" warnings + def viacall(&b) = b.call + def viaaref(&b) = b[] + def viayield(&b) = b.yield + def viaeqq(&b) = b === 1 + + assert_equal(:orig, viacall { :orig }) + assert_equal(:orig, viaaref { :orig }) + assert_equal(:orig, viayield { :orig }) + assert_equal(:orig, viaeqq { :orig }) + + class Proc + def [](*) = :redef_aref + end + assert_equal(:redef_aref, viaaref { :orig }, "redefining Proc#[] must deopt blk[]") + assert_equal(:orig, viacall { :orig }, "Proc#[] redefinition must not affect blk.call") + assert_equal(:orig, viayield { :orig }, "Proc#[] redefinition must not affect blk.yield") + assert_equal(:orig, viaeqq { :orig }, "Proc#[] redefinition must not affect blk === x") + + class Proc + def yield(*) = :redef_yield + end + assert_equal(:redef_yield, viayield { :orig }, "redefining Proc#yield must deopt blk.yield") + assert_equal(:orig, viacall { :orig }, "Proc#yield redefinition must not affect blk.call") + + class Proc + def ===(*) = :redef_eqq + end + assert_equal(:redef_eqq, viaeqq { :orig }, "redefining Proc#=== must deopt blk === x") + assert_equal(:orig, viacall { :orig }, "Proc#=== redefinition must not affect blk.call") + + class Proc + def call(*) = :redef_call + end + assert_equal(:redef_call, viacall { :orig }, "redefining Proc#call must deopt blk.call") + END + end + def test_peephole_optimization_without_trace assert_ruby_status [], <<-END RubyVM::InstructionSequence.compile_option = {trace_instruction: false} diff --git a/vm.c b/vm.c index c30e750d9fee00..82f595dc62d4dd 100644 --- a/vm.c +++ b/vm.c @@ -2462,6 +2462,7 @@ vm_init_redefined_flag(void) OP(Min, MIN), (C(Array)); OP(Hash, HASH), (C(Array)); OP(Call, CALL), (C(Proc)); + OP(Yield, YIELD), (C(Proc)); OP(And, AND), (C(Integer)); OP(Or, OR), (C(Integer)); OP(NilP, NIL_P), (C(NilClass)); @@ -2502,6 +2503,7 @@ vm_redefinition_bop_for_id(ID mid) OP(Min, MIN); OP(Hash, HASH); OP(Call, CALL); + OP(Yield, YIELD); OP(And, AND); OP(Or, OR); OP(NilP, NIL_P); @@ -4621,8 +4623,11 @@ Init_VM(void) vm_init_redefined_flag(); rb_block_param_proxy = rb_obj_alloc(rb_cObject); - rb_add_method_optimized(rb_singleton_class(rb_block_param_proxy), idCall, - OPTIMIZED_METHOD_TYPE_BLOCK_CALL, 0, METHOD_VISI_PUBLIC); + VALUE proxy_singleton = rb_singleton_class(rb_block_param_proxy); + rb_add_method_optimized(proxy_singleton, idCall, OPTIMIZED_METHOD_TYPE_BLOCK_CALL, 0, METHOD_VISI_PUBLIC); + rb_add_method_optimized(proxy_singleton, idAREF, OPTIMIZED_METHOD_TYPE_BLOCK_CALL, 0, METHOD_VISI_PUBLIC); + rb_add_method_optimized(proxy_singleton, idYield, OPTIMIZED_METHOD_TYPE_BLOCK_CALL, 0, METHOD_VISI_PUBLIC); + rb_add_method_optimized(proxy_singleton, idEqq, OPTIMIZED_METHOD_TYPE_BLOCK_CALL, 0, METHOD_VISI_PUBLIC); rb_obj_freeze(rb_block_param_proxy); rb_vm_register_global_object(rb_block_param_proxy); diff --git a/vm_callinfo.h b/vm_callinfo.h index 0ff0d89d1a5617..612f8024640303 100644 --- a/vm_callinfo.h +++ b/vm_callinfo.h @@ -601,12 +601,20 @@ struct rb_class_cc_entries { int len; const struct rb_callable_method_entry_struct *cme; struct rb_class_cc_entries_entry { - unsigned int argc; - unsigned int flag; const struct rb_callcache *cc; + unsigned int argc; + unsigned short flag; + unsigned short kw_len; } entries[FLEX_ARY_LEN]; }; +/* entries[].flag is an unsigned short, so every VM_CALL flag bit must fit in 16 bits. */ +STATIC_ASSERT(cc_entries_flag_fits_in_short, VM_CALL__END <= 16); + +/* entries[].kw_len is an unsigned short, so a call site cannot carry, nor a method + declare, more keyword arguments than this. Enforced at compile time. */ +#define VM_CALL_KW_LEN_MAX UINT16_MAX + static inline size_t vm_ccs_alloc_size(size_t capa) { diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 57a56e6a628d43..e6f23f27beb975 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1954,9 +1954,11 @@ vm_ccs_push(VALUE cc_tbl, ID mid, struct rb_class_cc_entries *ccs, const struct } VM_ASSERT(ccs->len < ccs->capa); + const struct rb_callinfo_kwarg *kwarg = vm_ci_kwarg(ci); const int pos = ccs->len++; ccs->entries[pos].argc = vm_ci_argc(ci); - ccs->entries[pos].flag = vm_ci_flag(ci); + ccs->entries[pos].flag = (unsigned short)vm_ci_flag(ci); + ccs->entries[pos].kw_len = (unsigned short)(kwarg ? kwarg->keyword_len : 0); RB_OBJ_WRITE(cc_tbl, &ccs->entries[pos].cc, cc); if (RB_DEBUG_COUNTER_SETMAX(ccs_maxlen, ccs->len)) { @@ -1971,9 +1973,10 @@ rb_vm_ccs_dump(struct rb_class_cc_entries *ccs) { ruby_debug_printf("ccs:%p (%d,%d)\n", (void *)ccs, ccs->len, ccs->capa); for (int i=0; ilen; i++) { - ruby_debug_printf("CCS CI ID:flag:%x argc:%u\n", + ruby_debug_printf("CCS CI ID:flag:%x argc:%u kw_len:%u\n", ccs->entries[i].flag, - ccs->entries[i].argc); + ccs->entries[i].argc, + ccs->entries[i].kw_len); rp(ccs->entries[i].cc); } } @@ -2118,19 +2121,25 @@ vm_lookup_cc(const VALUE klass, const struct rb_callinfo * const ci, ID mid) VM_ASSERT(vm_ccs_verify(ccs, mid, klass)); // We already know the method id is correct because we had - // to look up the ccs_data by method id. All we need to - // compare is argc and flag + // to look up the ccs_data by method id. We compare argc and + // flag, plus the keyword argument count: two call sites with + // the same argc and flag can still differ in how many of the + // arguments are keywords (e.g. `m(a: 1, b: 2)` vs `m(1, b: 2)`), + // which selects a different argument setup fastpath. + const struct rb_callinfo_kwarg *kwarg = vm_ci_kwarg(ci); unsigned int argc = vm_ci_argc(ci); unsigned int flag = vm_ci_flag(ci); + unsigned int kw_len = kwarg ? kwarg->keyword_len : 0; for (int i=0; ientries[i].argc; unsigned int ccs_ci_flag = ccs->entries[i].flag; + unsigned int ccs_ci_kw_len = ccs->entries[i].kw_len; const struct rb_callcache *ccs_cc = ccs->entries[i].cc; VM_ASSERT(IMEMO_TYPE_P(ccs_cc, imemo_callcache)); - if (ccs_ci_argc == argc && ccs_ci_flag == flag) { + if (ccs_ci_argc == argc && ccs_ci_flag == flag && ccs_ci_kw_len == kw_len) { RB_DEBUG_COUNTER_INC(cc_found_in_ccs); VM_ASSERT(vm_cc_cme(ccs_cc)->called_id == mid); @@ -4652,7 +4661,15 @@ vm_call_opt_block_call(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, VALUE block_handler = VM_ENV_BLOCK_HANDLER(VM_CF_LEP(reg_cfp)); const struct rb_callinfo *ci = calling->cd->ci; - if (BASIC_OP_UNREDEFINED_P(BOP_CALL, PROC_REDEFINED_OP_FLAG)) { + enum ruby_basic_operators bop; + switch (vm_ci_mid(ci)) { + case idAREF: bop = BOP_AREF; break; + case idYield: bop = BOP_YIELD; break; + case idEqq: bop = BOP_EQQ; break; + default: bop = BOP_CALL; break; + } + + if (BASIC_OP_UNREDEFINED_P(bop, PROC_REDEFINED_OP_FLAG)) { return vm_invoke_block_opt_call(ec, reg_cfp, calling, ci, block_handler); } else { diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 2b31f5cddd1b06..336d5cd559c795 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -337,7 +337,8 @@ pub const BOP_CMP: ruby_basic_operators = 31; pub const BOP_DEFAULT: ruby_basic_operators = 32; pub const BOP_PACK: ruby_basic_operators = 33; pub const BOP_INCLUDE_P: ruby_basic_operators = 34; -pub const BOP_LAST_: ruby_basic_operators = 35; +pub const BOP_YIELD: ruby_basic_operators = 35; +pub const BOP_LAST_: ruby_basic_operators = 36; pub type ruby_basic_operators = u32; pub type rb_serial_t = ::std::os::raw::c_ulonglong; pub const imemo_env: imemo_type = 0; diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index fdd88e5001126a..5c5c5cd3fa87f4 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -110,7 +110,9 @@ fn emit_jmp_ptr_with_invalidation(cb: &mut CodeBlock, dst_ptr: CodePtr) { let start = cb.get_write_ptr(); emit_jmp_ptr(cb, dst_ptr, true); let end = cb.get_write_ptr(); - unsafe { rb_jit_icache_invalidate(start.raw_ptr(cb) as _, end.raw_ptr(cb) as _) }; + trace_compile_phase("invalidate_icache", || { + unsafe { rb_jit_icache_invalidate(start.raw_ptr(cb) as _, end.raw_ptr(cb) as _) }; + }); } fn emit_jmp_ptr(cb: &mut CodeBlock, dst_ptr: CodePtr, padding: bool) { @@ -1729,8 +1731,10 @@ impl Assembler { cb.link_labels().or(Err(CompileError::LabelLinkingFailure))?; - // Invalidate icache for newly written out region so we don't run stale code. - unsafe { rb_jit_icache_invalidate(start_ptr.raw_ptr(cb) as _, cb.get_write_ptr().raw_ptr(cb) as _) }; + trace_compile_phase("invalidate_icache", || { + // Invalidate icache for newly written out region so we don't run stale code. + unsafe { rb_jit_icache_invalidate(start_ptr.raw_ptr(cb) as _, cb.get_write_ptr().raw_ptr(cb) as _) }; + }); Ok((start_ptr, gc_offsets)) }) diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 69e5a2b6fe0e01..3a321e04062360 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -439,7 +439,8 @@ pub const BOP_CMP: ruby_basic_operators = 31; pub const BOP_DEFAULT: ruby_basic_operators = 32; pub const BOP_PACK: ruby_basic_operators = 33; pub const BOP_INCLUDE_P: ruby_basic_operators = 34; -pub const BOP_LAST_: ruby_basic_operators = 35; +pub const BOP_YIELD: ruby_basic_operators = 35; +pub const BOP_LAST_: ruby_basic_operators = 36; pub type ruby_basic_operators = u32; pub type rb_serial_t = ::std::os::raw::c_ulonglong; #[repr(C)] diff --git a/zjit/src/options.rs b/zjit/src/options.rs index 2bb65cf526128a..55c9728a1944f1 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -336,6 +336,9 @@ macro_rules! get_option { ($option_name:ident) => { unsafe { crate::options::OPTIONS.as_ref() }.unwrap().$option_name }; + ($option_name:ident, $default:expr) => { + unsafe { crate::options::OPTIONS.as_ref() }.map(|opts| opts.$option_name).unwrap_or($default) + }; } pub(crate) use get_option; diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index f6457dbf2a6547..ae8fd26c47b7b1 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -1024,7 +1024,7 @@ pub fn zjit_alloc_bytes() -> usize { /// Record a Perfetto duration event spanning the execution of `func`. /// Uses Begin/End pairs so nested calls produce properly nested slices. pub fn trace_compile_phase(name: &str, func: F) -> R where F: FnOnce() -> R { - if !get_option!(trace_compiles) { + if !get_option!(trace_compiles, /*default=*/false) { return func(); } if let Some(tracer) = ZJITState::get_tracer() { diff --git a/zjit/src/virtualmem.rs b/zjit/src/virtualmem.rs index 0088ef1a66fecb..4c512b151e526b 100644 --- a/zjit/src/virtualmem.rs +++ b/zjit/src/virtualmem.rs @@ -333,15 +333,21 @@ pub mod sys { impl super::Allocator for SystemAllocator { fn mark_writable(&mut self, ptr: *const u8, size: u32) -> bool { - unsafe { rb_jit_mark_writable(ptr as VoidPtr, size) } + crate::stats::trace_compile_phase("mark_writable", || { + unsafe { rb_jit_mark_writable(ptr as VoidPtr, size) } + }) } fn mark_executable(&mut self, ptr: *const u8, size: u32) { - unsafe { rb_jit_mark_executable(ptr as VoidPtr, size) } + crate::stats::trace_compile_phase("mark_executable", || { + unsafe { rb_jit_mark_executable(ptr as VoidPtr, size) } + }) } fn mark_unused(&mut self, ptr: *const u8, size: u32) -> bool { - unsafe { rb_jit_mark_unused(ptr as VoidPtr, size) } + crate::stats::trace_compile_phase("mark_unused", || { + unsafe { rb_jit_mark_unused(ptr as VoidPtr, size) } + }) } } }