diff options
| -rw-r--r-- | NEWS.md | 5 | ||||
| -rw-r--r-- | benchmark/keyword_arguments.yml | 13 | ||||
| -rw-r--r-- | compile.c | 62 | ||||
| -rw-r--r-- | hash.c | 2 | ||||
| -rw-r--r-- | iseq.c | 1 | ||||
| -rw-r--r-- | test/ruby/test_keyword.rb | 212 | ||||
| -rw-r--r-- | vm.c | 13 | ||||
| -rw-r--r-- | vm_args.c | 85 | ||||
| -rw-r--r-- | vm_callinfo.h | 6 | ||||
| -rw-r--r-- | vm_insnhelper.c | 27 | ||||
| -rw-r--r-- | vm_insnhelper.h | 1 |
11 files changed, 368 insertions, 59 deletions
@@ -139,6 +139,11 @@ Excluding feature bug fixes. ## Implementation improvements +* The number of hashes allocated when using a keyword splat in + a method call has been reduced to a maximum of 1, and passing + a keyword splat to a method that accepts specific keywords + does not allocate a hash. + ## Miscellaneous changes * Methods using `ruby2_keywords` will no longer keep empty keyword diff --git a/benchmark/keyword_arguments.yml b/benchmark/keyword_arguments.yml new file mode 100644 index 0000000000..fce6bce0b8 --- /dev/null +++ b/benchmark/keyword_arguments.yml @@ -0,0 +1,13 @@ +prelude: | + h = {a: 1} + def kw(a: 1) a end + def kws(**kw) kw end +benchmark: + kw_to_kw: "kw(a: 1)" + kw_splat_to_kw: "kw(**h)" + kw_to_kw_splat: "kws(a: 1)" + kw_splat_to_kw_splat: "kws(**h)" + kw_and_splat_to_kw: "kw(a: 1, **h)" + kw_splats_to_kw: "kw(**h, **h)" + kw_and_splat_to_kw_splat: "kws(a: 1, **h)" + kw_splats_to_kw_splat: "kws(**h, **h)" @@ -3918,20 +3918,27 @@ compile_keyword_arg(rb_iseq_t *iseq, LINK_ANCHOR *const ret, if (root_node->nd_head && nd_type(root_node->nd_head) == NODE_LIST) { const NODE *node = root_node->nd_head; + int seen_nodes = 0; while (node) { const NODE *key_node = node->nd_head; + seen_nodes++; assert(nd_type(node) == NODE_LIST); - if (!key_node) { - if (flag) *flag |= VM_CALL_KW_SPLAT; - return FALSE; - } - else if (nd_type(key_node) == NODE_LIT && RB_TYPE_P(key_node->nd_lit, T_SYMBOL)) { + if (key_node && nd_type(key_node) == NODE_LIT && RB_TYPE_P(key_node->nd_lit, T_SYMBOL)) { /* can be keywords */ } else { - if (flag) *flag |= VM_CALL_KW_SPLAT; + if (flag) { + *flag |= VM_CALL_KW_SPLAT; + if (seen_nodes > 1 || node->nd_next->nd_next) { + /* A new hash will be created for the keyword arguments + * in this case, so mark the method as passing mutable + * keyword splat. + */ + *flag |= VM_CALL_KW_SPLAT_MUT; + } + } return FALSE; } node = node->nd_next; /* skip value node */ @@ -4312,31 +4319,47 @@ compile_hash(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, int popp if (empty_kw) { if (only_kw && method_call_keywords) { - /* **{} appears at the last, so it won't be modified. + /* **{} appears at the only keyword argument in method call, + * so it won't be modified. * kw is a special NODE_LIT that contains a special empty hash, - * so this emits: putobject {} + * so this emits: putobject {}. + * This is only done for method calls and not for literal hashes, + * because literal hashes should always result in a new hash. */ NO_CHECK(COMPILE(ret, "keyword splat", kw)); } else if (first_kw) { - /* **{} appears at the first, so it may be modified. + /* **{} appears as the first keyword argument, so it may be modified. * We need to create a fresh hash object. */ ADD_INSN1(ret, line, newhash, INT2FIX(0)); } + /* Any empty keyword splats that are not the first can be ignored. + * since merging an empty hash into the existing hash is the same + * as not merging it. */ } else { - /* This is not empty hash: **{k:1}. - * We need to clone the hash (if first), or merge the hash to - * the accumulated hash (if not first). - */ - ADD_INSN1(ret, line, putspecialobject, INT2FIX(VM_SPECIAL_OBJECT_VMCORE)); - if (first_kw) ADD_INSN1(ret, line, newhash, INT2FIX(0)); - else ADD_INSN(ret, line, swap); + if (only_kw && method_call_keywords) { + /* **kw is only keyword argument in method call. + * Use directly. This will be not be flagged as mutable. + * This is only done for method calls and not for literal hashes, + * because literal hashes should always result in a new hash. + */ + NO_CHECK(COMPILE(ret, "keyword splat", kw)); + } + else { + /* There is more than one keyword argument, or this is not a method + * call. In that case, we need to add an empty hash (if first keyword), + * or merge the hash to the accumulated hash (if not the first keyword). + */ + ADD_INSN1(ret, line, putspecialobject, INT2FIX(VM_SPECIAL_OBJECT_VMCORE)); + if (first_kw) ADD_INSN1(ret, line, newhash, INT2FIX(0)); + else ADD_INSN(ret, line, swap); - NO_CHECK(COMPILE(ret, "keyword splat", kw)); + NO_CHECK(COMPILE(ret, "keyword splat", kw)); - ADD_SEND(ret, line, id_core_hash_merge_kwd, INT2FIX(2)); + ADD_SEND(ret, line, id_core_hash_merge_kwd, INT2FIX(2)); + } } first_chunk = 0; @@ -7808,10 +7831,10 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, in if (local_body->param.flags.has_kwrest) { int idx = local_body->local_table_size - local_kwd->rest_start; ADD_GETLOCAL(args, line, idx, lvar_level); - ADD_SEND (args, line, rb_intern("dup"), INT2FIX(0)); } else { ADD_INSN1(args, line, newhash, INT2FIX(0)); + flag |= VM_CALL_KW_SPLAT_MUT; } for (i = 0; i < local_kwd->num; ++i) { ID id = local_kwd->table[i]; @@ -7831,7 +7854,6 @@ iseq_compile_each0(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *node, in int idx = local_body->local_table_size - local_kwd->rest_start; ADD_GETLOCAL(args, line, idx, lvar_level); - ADD_SEND (args, line, rb_intern("dup"), INT2FIX(0)); if (local_body->param.flags.has_rest) { ADD_INSN1(args, line, newarray, INT2FIX(1)); ADD_INSN (args, line, concatarray); @@ -1868,7 +1868,7 @@ rb_hash_s_create(int argc, VALUE *argv, VALUE klass) return hash; } -VALUE +MJIT_FUNC_EXPORTED VALUE rb_to_hash_type(VALUE hash) { return rb_convert_type_with_id(hash, T_HASH, "Hash", idTo_hash); @@ -1992,6 +1992,7 @@ rb_insn_operand_intern(const rb_iseq_t *iseq, CALL_FLAG(ZSUPER); CALL_FLAG(KWARG); CALL_FLAG(KW_SPLAT); + CALL_FLAG(KW_SPLAT_MUT); CALL_FLAG(OPT_SEND); /* maybe not reachable */ rb_ary_push(ary, rb_ary_join(flags, rb_str_new2("|"))); } diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb index 48d6a7336a..8ec0636d5c 100644 --- a/test/ruby/test_keyword.rb +++ b/test/ruby/test_keyword.rb @@ -190,6 +190,218 @@ class TestKeywordArguments < Test::Unit::TestCase assert_equal(["bar", 111111], f[str: "bar", num: 111111]) end + def test_keyword_splat_new + kw = {} + h = {a: 1} + + def self.assert_equal_not_same(kw, res) + assert_instance_of(Hash, res) + assert_equal(kw, res) + assert_not_same(kw, res) + end + + def self.y(**kw) kw end + m = method(:y) + assert_equal(false, y(**{}).frozen?) + assert_equal_not_same(kw, y(**kw)) + assert_equal_not_same(h, y(**h)) + assert_equal(false, send(:y, **{}).frozen?) + assert_equal_not_same(kw, send(:y, **kw)) + assert_equal_not_same(h, send(:y, **h)) + assert_equal(false, public_send(:y, **{}).frozen?) + assert_equal_not_same(kw, public_send(:y, **kw)) + assert_equal_not_same(h, public_send(:y, **h)) + assert_equal(false, m.(**{}).frozen?) + assert_equal_not_same(kw, m.(**kw)) + assert_equal_not_same(h, m.(**h)) + assert_equal(false, m.send(:call, **{}).frozen?) + assert_equal_not_same(kw, m.send(:call, **kw)) + assert_equal_not_same(h, m.send(:call, **h)) + + m = method(:send) + assert_equal(false, m.(:y, **{}).frozen?) + assert_equal_not_same(kw, m.(:y, **kw)) + assert_equal_not_same(h, m.(:y, **h)) + assert_equal(false, m.send(:call, :y, **{}).frozen?) + assert_equal_not_same(kw, m.send(:call, :y, **kw)) + assert_equal_not_same(h, m.send(:call, :y, **h)) + + singleton_class.send(:remove_method, :y) + define_singleton_method(:y) { |**kw| kw } + m = method(:y) + assert_equal(false, y(**{}).frozen?) + assert_equal_not_same(kw, y(**kw)) + assert_equal_not_same(h, y(**h)) + assert_equal(false, send(:y, **{}).frozen?) + assert_equal_not_same(kw, send(:y, **kw)) + assert_equal_not_same(h, send(:y, **h)) + assert_equal(false, public_send(:y, **{}).frozen?) + assert_equal_not_same(kw, public_send(:y, **kw)) + assert_equal_not_same(h, public_send(:y, **h)) + assert_equal(false, m.(**{}).frozen?) + assert_equal_not_same(kw, m.(**kw)) + assert_equal_not_same(h, m.(**h)) + assert_equal(false, m.send(:call, **{}).frozen?) + assert_equal_not_same(kw, m.send(:call, **kw)) + assert_equal_not_same(h, m.send(:call, **h)) + + y = lambda { |**kw| kw } + m = y.method(:call) + assert_equal(false, y.(**{}).frozen?) + assert_equal_not_same(kw, y.(**kw)) + assert_equal_not_same(h, y.(**h)) + assert_equal(false, y.send(:call, **{}).frozen?) + assert_equal_not_same(kw, y.send(:call, **kw)) + assert_equal_not_same(h, y.send(:call, **h)) + assert_equal(false, y.public_send(:call, **{}).frozen?) + assert_equal_not_same(kw, y.public_send(:call, **kw)) + assert_equal_not_same(h, y.public_send(:call, **h)) + assert_equal(false, m.(**{}).frozen?) + assert_equal_not_same(kw, m.(**kw)) + assert_equal_not_same(h, m.(**h)) + assert_equal(false, m.send(:call, **{}).frozen?) + assert_equal_not_same(kw, m.send(:call, **kw)) + assert_equal_not_same(h, m.send(:call, **h)) + + y = :y.to_proc + m = y.method(:call) + assert_equal(false, y.(self, **{}).frozen?) + assert_equal_not_same(kw, y.(self, **kw)) + assert_equal_not_same(h, y.(self, **h)) + assert_equal(false, y.send(:call, self, **{}).frozen?) + assert_equal_not_same(kw, y.send(:call, self, **kw)) + assert_equal_not_same(h, y.send(:call, self, **h)) + assert_equal(false, y.public_send(:call, self, **{}).frozen?) + assert_equal_not_same(kw, y.public_send(:call, self, **kw)) + assert_equal_not_same(h, y.public_send(:call, self, **h)) + assert_equal(false, m.(self, **{}).frozen?) + assert_equal_not_same(kw, m.(self, **kw)) + assert_equal_not_same(h, m.(self, **h)) + assert_equal(false, m.send(:call, self, **{}).frozen?) + assert_equal_not_same(kw, m.send(:call, self, **kw)) + assert_equal_not_same(h, m.send(:call, self, **h)) + + c = Class.new do + def y(**kw) kw end + end + o = c.new + def o.y(**kw) super end + m = o.method(:y) + assert_equal(false, o.y(**{}).frozen?) + assert_equal_not_same(kw, o.y(**kw)) + assert_equal_not_same(h, o.y(**h)) + assert_equal(false, o.send(:y, **{}).frozen?) + assert_equal_not_same(kw, o.send(:y, **kw)) + assert_equal_not_same(h, o.send(:y, **h)) + assert_equal(false, o.public_send(:y, **{}).frozen?) + assert_equal_not_same(kw, o.public_send(:y, **kw)) + assert_equal_not_same(h, o.public_send(:y, **h)) + assert_equal(false, m.(**{}).frozen?) + assert_equal_not_same(kw, m.(**kw)) + assert_equal_not_same(h, m.(**h)) + assert_equal(false, m.send(:call, **{}).frozen?) + assert_equal_not_same(kw, m.send(:call, **kw)) + assert_equal_not_same(h, m.send(:call, **h)) + + o.singleton_class.send(:remove_method, :y) + def o.y(**kw) super(**kw) end + assert_equal(false, o.y(**{}).frozen?) + assert_equal_not_same(kw, o.y(**kw)) + assert_equal_not_same(h, o.y(**h)) + assert_equal(false, o.send(:y, **{}).frozen?) + assert_equal_not_same(kw, o.send(:y, **kw)) + assert_equal_not_same(h, o.send(:y, **h)) + assert_equal(false, o.public_send(:y, **{}).frozen?) + assert_equal_not_same(kw, o.public_send(:y, **kw)) + assert_equal_not_same(h, o.public_send(:y, **h)) + assert_equal(false, m.(**{}).frozen?) + assert_equal_not_same(kw, m.(**kw)) + assert_equal_not_same(h, m.(**h)) + assert_equal(false, m.send(:call, **{}).frozen?) + assert_equal_not_same(kw, m.send(:call, **kw)) + assert_equal_not_same(h, m.send(:call, **h)) + + c = Class.new do + def method_missing(_, **kw) kw end + end + o = c.new + def o.y(**kw) super end + m = o.method(:y) + assert_equal(false, o.y(**{}).frozen?) + assert_equal_not_same(kw, o.y(**kw)) + assert_equal_not_same(h, o.y(**h)) + assert_equal(false, o.send(:y, **{}).frozen?) + assert_equal_not_same(kw, o.send(:y, **kw)) + assert_equal_not_same(h, o.send(:y, **h)) + assert_equal(false, o.public_send(:y, **{}).frozen?) + assert_equal_not_same(kw, o.public_send(:y, **kw)) + assert_equal_not_same(h, o.public_send(:y, **h)) + assert_equal(false, m.(**{}).frozen?) + assert_equal_not_same(kw, m.(**kw)) + assert_equal_not_same(h, m.(**h)) + assert_equal(false, m.send(:call, **{}).frozen?) + assert_equal_not_same(kw, m.send(:call, **kw)) + assert_equal_not_same(h, m.send(:call, **h)) + + o.singleton_class.send(:remove_method, :y) + def o.y(**kw) super(**kw) end + assert_equal(false, o.y(**{}).frozen?) + assert_equal_not_same(kw, o.y(**kw)) + assert_equal_not_same(h, o.y(**h)) + assert_equal(false, o.send(:y, **{}).frozen?) + assert_equal_not_same(kw, o.send(:y, **kw)) + assert_equal_not_same(h, o.send(:y, **h)) + assert_equal(false, o.public_send(:y, **{}).frozen?) + assert_equal_not_same(kw, o.public_send(:y, **kw)) + assert_equal_not_same(h, o.public_send(:y, **h)) + assert_equal(false, m.(**{}).frozen?) + assert_equal_not_same(kw, m.(**kw)) + assert_equal_not_same(h, m.(**h)) + assert_equal(false, m.send(:call, **{}).frozen?) + assert_equal_not_same(kw, m.send(:call, **kw)) + assert_equal_not_same(h, m.send(:call, **h)) + + c = Class.new do + attr_reader :kw + def initialize(**kw) @kw = kw end + end + m = c.method(:new) + assert_equal(false, c.new(**{}).kw.frozen?) + assert_equal_not_same(kw, c.new(**kw).kw) + assert_equal_not_same(h, c.new(**h).kw) + assert_equal(false, c.send(:new, **{}).kw.frozen?) + assert_equal_not_same(kw, c.send(:new, **kw).kw) + assert_equal_not_same(h, c.send(:new, **h).kw) + assert_equal(false, c.public_send(:new, **{}).kw.frozen?) + assert_equal_not_same(kw, c.public_send(:new, **kw).kw) + assert_equal_not_same(h, c.public_send(:new, **h).kw) + assert_equal(false, m.(**{}).kw.frozen?) + assert_equal_not_same(kw, m.(**kw).kw) + assert_equal_not_same(h, m.(**h).kw) + assert_equal(false, m.send(:call, **{}).kw.frozen?) + assert_equal_not_same(kw, m.send(:call, **kw).kw) + assert_equal_not_same(h, m.send(:call, **h).kw) + + singleton_class.send(:attr_writer, :y) + m = method(:y=) + assert_equal_not_same(h, send(:y=, **h)) + assert_equal_not_same(h, public_send(:y=, **h)) + assert_equal_not_same(h, m.(**h)) + assert_equal_not_same(h, m.send(:call, **h)) + + singleton_class.send(:remove_method, :y) + def self.method_missing(_, **kw) kw end + assert_equal(false, y(**{}).frozen?) + assert_equal_not_same(kw, y(**kw)) + assert_equal_not_same(h, y(**h)) + assert_equal(false, send(:y, **{}).frozen?) + assert_equal_not_same(kw, send(:y, **kw)) + assert_equal_not_same(h, send(:y, **h)) + assert_equal(false, public_send(:y, **{}).frozen?) + assert_equal_not_same(kw, public_send(:y, **kw)) + assert_equal_not_same(h, public_send(:y, **h)) + end + def test_regular_kwsplat kw = {} h = {:a=>1} @@ -1223,9 +1223,16 @@ invoke_block_from_c_proc(rb_execution_context_t *ec, const rb_proc_t *proc, case block_type_iseq: return invoke_iseq_block_from_c(ec, &block->as.captured, self, argc, argv, kw_splat, passed_block_handler, NULL, is_lambda, me); case block_type_ifunc: - if (kw_splat == 1 && RHASH_EMPTY_P(argv[argc-1])) { - argc--; - kw_splat = 2; + if (kw_splat == 1) { + VALUE keyword_hash = argv[argc-1]; + if (!RB_TYPE_P(keyword_hash, T_HASH)) { + keyword_hash = rb_to_hash_type(keyword_hash); + } + if (RHASH_EMPTY_P(keyword_hash)) { + argc--; + } else { + ((VALUE *)argv)[argc-1] = rb_hash_dup(keyword_hash); + } } return vm_yield_with_cfunc(ec, &block->as.captured, self, argc, argv, kw_splat, passed_block_handler, me); case block_type_symbol: @@ -400,9 +400,15 @@ args_setup_kw_parameters(rb_execution_context_t *const ec, const rb_iseq_t *cons } static inline void -args_setup_kw_rest_parameter(VALUE keyword_hash, VALUE *locals) +args_setup_kw_rest_parameter(VALUE keyword_hash, VALUE *locals, int kw_flag) { - locals[0] = NIL_P(keyword_hash) ? rb_hash_new() : rb_hash_dup(keyword_hash); + if (NIL_P(keyword_hash)) { + keyword_hash = rb_hash_new(); + } + else if (!(kw_flag & VM_CALL_KW_SPLAT_MUT)) { + keyword_hash = rb_hash_dup(keyword_hash); + } + locals[0] = keyword_hash; } static inline void @@ -429,11 +435,20 @@ fill_keys_values(st_data_t key, st_data_t val, st_data_t ptr) } static inline int -ignore_keyword_hash_p(VALUE keyword_hash, const rb_iseq_t * const iseq) +ignore_keyword_hash_p(VALUE keyword_hash, const rb_iseq_t * const iseq, unsigned int * kw_flag, VALUE * converted_keyword_hash) { + if (!RB_TYPE_P(keyword_hash, T_HASH)) { + keyword_hash = rb_to_hash_type(keyword_hash); + } + if (!(*kw_flag & VM_CALL_KW_SPLAT_MUT) && + (iseq->body->param.flags.has_kwrest || + iseq->body->param.flags.ruby2_keywords)) { + *kw_flag |= VM_CALL_KW_SPLAT_MUT; + keyword_hash = rb_hash_dup(keyword_hash); + } + *converted_keyword_hash = keyword_hash; return !(iseq->body->param.flags.has_kw) && !(iseq->body->param.flags.has_kwrest) && - RB_TYPE_P(keyword_hash, T_HASH) && RHASH_EMPTY_P(keyword_hash); } @@ -446,13 +461,14 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co const int min_argc = iseq->body->param.lead_num + iseq->body->param.post_num; const int max_argc = (iseq->body->param.flags.has_rest == FALSE) ? min_argc + iseq->body->param.opt_num : UNLIMITED_ARGUMENTS; int given_argc; - unsigned int kw_flag = vm_ci_flag(ci) & (VM_CALL_KWARG | VM_CALL_KW_SPLAT); + unsigned int kw_flag = vm_ci_flag(ci) & (VM_CALL_KWARG | VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT); int opt_pc = 0, allow_autosplat = !kw_flag; struct args_info args_body, *args; VALUE keyword_hash = Qnil; VALUE * const orig_sp = ec->cfp->sp; unsigned int i; VALUE flag_keyword_hash = 0; + VALUE converted_keyword_hash = 0; vm_check_canary(ec, orig_sp); /* @@ -494,7 +510,7 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co else { args->kw_argv = NULL; given_argc = args_kw_argv_to_hash(args); - kw_flag |= VM_CALL_KW_SPLAT; + kw_flag |= VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT; } } else { @@ -515,7 +531,7 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co if (RB_TYPE_P(rest_last, T_HASH) && (((struct RHash *)rest_last)->basic.flags & RHASH_PASS_AS_KEYWORDS)) { rest_last = rb_hash_dup(rest_last); - kw_flag |= VM_CALL_KW_SPLAT; + kw_flag |= VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT; } else { rest_last = 0; @@ -523,38 +539,53 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co } if (kw_flag & VM_CALL_KW_SPLAT) { - if (len > 0 && ignore_keyword_hash_p(rest_last, iseq)) { + if (ignore_keyword_hash_p(rest_last, iseq, &kw_flag, &converted_keyword_hash)) { arg_rest_dup(args); rb_ary_pop(args->rest); given_argc--; - kw_flag &= ~VM_CALL_KW_SPLAT; + kw_flag &= ~(VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT); } - else if (iseq->body->param.flags.ruby2_keywords && rest_last) { - flag_keyword_hash = rest_last; - } - else if (iseq->body->param.flags.has_kw || iseq->body->param.flags.has_kwrest) { - arg_rest_dup(args); - rb_ary_pop(args->rest); - given_argc--; - keyword_hash = rest_last; + else { + if (rest_last != converted_keyword_hash) { + rest_last = converted_keyword_hash; + arg_rest_dup(args); + RARRAY_ASET(args->rest, len - 1, rest_last); + } + + if (iseq->body->param.flags.ruby2_keywords && rest_last) { + flag_keyword_hash = rest_last; + } + else if (iseq->body->param.flags.has_kw || iseq->body->param.flags.has_kwrest) { + arg_rest_dup(args); + rb_ary_pop(args->rest); + given_argc--; + keyword_hash = rest_last; + } } } } else { if (kw_flag & VM_CALL_KW_SPLAT) { VALUE last_arg = args->argv[args->argc-1]; - if (ignore_keyword_hash_p(last_arg, iseq)) { + if (ignore_keyword_hash_p(last_arg, iseq, &kw_flag, &converted_keyword_hash)) { args->argc--; given_argc--; - kw_flag &= ~VM_CALL_KW_SPLAT; + kw_flag &= ~(VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT); } - else if (iseq->body->param.flags.ruby2_keywords) { - flag_keyword_hash = last_arg; - } - else if (iseq->body->param.flags.has_kw || iseq->body->param.flags.has_kwrest) { - args->argc--; - given_argc--; - keyword_hash = last_arg; + else { + if (last_arg != converted_keyword_hash) { + last_arg = converted_keyword_hash; + args->argv[args->argc-1] = last_arg; + } + + if (iseq->body->param.flags.ruby2_keywords) { + flag_keyword_hash = last_arg; + } + else if (iseq->body->param.flags.has_kw || iseq->body->param.flags.has_kwrest) { + args->argc--; + given_argc--; + keyword_hash = last_arg; + } } } args->rest = Qfalse; @@ -651,7 +682,7 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co } } else if (iseq->body->param.flags.has_kwrest) { - args_setup_kw_rest_parameter(keyword_hash, locals + iseq->body->param.keyword->rest_start); + args_setup_kw_rest_parameter(keyword_hash, locals + iseq->body->param.keyword->rest_start, kw_flag); } else if (!NIL_P(keyword_hash) && RHASH_SIZE(keyword_hash) > 0 && arg_setup_type == arg_setup_method) { argument_kw_error(ec, iseq, "unknown", rb_hash_keys(keyword_hash)); diff --git a/vm_callinfo.h b/vm_callinfo.h index 30c4f703ba..f99f808a83 100644 --- a/vm_callinfo.h +++ b/vm_callinfo.h @@ -13,6 +13,7 @@ enum vm_call_flag_bits { VM_CALL_SUPER_bit, /* super */ VM_CALL_ZSUPER_bit, /* zsuper */ VM_CALL_OPT_SEND_bit, /* internal flag */ + VM_CALL_KW_SPLAT_MUT_bit, /* kw splat hash can be modified (to avoid allocating a new one) */ VM_CALL__END }; @@ -28,6 +29,7 @@ enum vm_call_flag_bits { #define VM_CALL_SUPER (0x01 << VM_CALL_SUPER_bit) #define VM_CALL_ZSUPER (0x01 << VM_CALL_ZSUPER_bit) #define VM_CALL_OPT_SEND (0x01 << VM_CALL_OPT_SEND_bit) +#define VM_CALL_KW_SPLAT_MUT (0x01 << VM_CALL_KW_SPLAT_MUT_bit) struct rb_callinfo_kwarg { int keyword_len; @@ -64,8 +66,8 @@ struct rb_callinfo { #define CI_EMBED_ID_bits 32 #elif SIZEOF_VALUE == 4 #define CI_EMBED_TAG_bits 1 -#define CI_EMBED_ARGC_bits 4 -#define CI_EMBED_FLAG_bits 12 +#define CI_EMBED_ARGC_bits 3 +#define CI_EMBED_FLAG_bits 13 #define CI_EMBED_ID_bits 15 #endif diff --git a/vm_insnhelper.c b/vm_insnhelper.c index e0e1f9e971..626557cb45 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1985,12 +1985,27 @@ CALLER_SETUP_ARG(struct rb_control_frame_struct *restrict cfp, calling->kw_splat = 1; } } - if (UNLIKELY(IS_ARGS_KEYWORD(ci))) { - /* This converts VM_CALL_KWARG style to VM_CALL_KW_SPLAT style - * by creating a keyword hash. - * So, vm_ci_flag(ci) & VM_CALL_KWARG is now inconsistent. - */ - vm_caller_setup_arg_kw(cfp, calling, ci); + if (UNLIKELY(IS_ARGS_KW_OR_KW_SPLAT(ci))) { + if (IS_ARGS_KEYWORD(ci)) { + /* This converts VM_CALL_KWARG style to VM_CALL_KW_SPLAT style + * by creating a keyword hash. + * So, vm_ci_flag(ci) & VM_CALL_KWARG is now inconsistent. + */ + vm_caller_setup_arg_kw(cfp, calling, ci); + } + else { + VALUE keyword_hash = cfp->sp[-1]; + if (!RB_TYPE_P(keyword_hash, T_HASH)) { + /* Convert a non-hash keyword splat to a new hash */ + cfp->sp[-1] = rb_hash_dup(rb_to_hash_type(keyword_hash)); + } + else if (!IS_ARGS_KW_SPLAT_MUT(ci)) { + /* Convert a hash keyword splat to a new hash unless + * a mutable keyword splat was passed. + */ + cfp->sp[-1] = rb_hash_dup(keyword_hash); + } + } } } diff --git a/vm_insnhelper.h b/vm_insnhelper.h index 07b38ea9d9..e20a6effa4 100644 --- a/vm_insnhelper.h +++ b/vm_insnhelper.h @@ -247,6 +247,7 @@ THROW_DATA_CONSUMED_SET(struct vm_throw_data *obj) #define IS_ARGS_KEYWORD(ci) (vm_ci_flag(ci) & VM_CALL_KWARG) #define IS_ARGS_KW_SPLAT(ci) (vm_ci_flag(ci) & VM_CALL_KW_SPLAT) #define IS_ARGS_KW_OR_KW_SPLAT(ci) (vm_ci_flag(ci) & (VM_CALL_KWARG | VM_CALL_KW_SPLAT)) +#define IS_ARGS_KW_SPLAT_MUT(ci) (vm_ci_flag(ci) & VM_CALL_KW_SPLAT_MUT) /* If this returns true, an optimized function returned by `vm_call_iseq_setup_func` can be used as a fastpath. */ |