ruby-changes:57587
From: Yusuke <ko1@a...>
Date: Fri, 6 Sep 2019 10:45:10 +0900 (JST)
Subject: [ruby-changes:57587] c5555e2eb8 (master): CALLER_SETUP_ARG removes an empty keyword hash from argv
https://git.ruby-lang.org/ruby.git/commit/?id=c5555e2eb8 From c5555e2eb8631e649cc9377f0bd75ebc2788cc7d Mon Sep 17 00:00:00 2001 From: Yusuke Endoh <mame@r...> Date: Thu, 5 Sep 2019 16:27:26 +0900 Subject: CALLER_SETUP_ARG removes an empty keyword hash from argv ...only when a "remove_empty_keyword_hash" flag is specified. After CALLER_SETUP_ARG is called, `ci->flag & VM_CALL_KW_SPLAT` must not be used. Instead. use `calling->kw_splat`. This is because CALLER_SETUP_ARG may modify argv and update `calling->kw_splat`, and `ci->flag & VM_CALL_KW_SPLAT` may be inconsistent with the result. diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb index 9e32faa..98e121f 100644 --- a/test/ruby/test_keyword.rb +++ b/test/ruby/test_keyword.rb @@ -464,12 +464,8 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L464 def c.m(arg, **args) [arg, args] end - assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do - assert_equal([kw, kw], c.send(:m, **{})) - end - assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do - assert_equal([kw, kw], c.send(:m, **kw)) - end + assert_raise(ArgumentError) { c.send(:m, **{}) } + assert_raise(ArgumentError) { c.send(:m, **kw) } assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do assert_equal([h, kw], c.send(:m, **h)) end @@ -643,12 +639,8 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L639 def c.method_missing(_, arg, **args) [arg, args] end - assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do - assert_equal([kw, kw], c.send(:m, **{})) - end - assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do - assert_equal([kw, kw], c.send(:m, **kw)) - end + assert_raise(ArgumentError) { c.send(:m, **{}) } + assert_raise(ArgumentError) { c.send(:m, **kw) } assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do assert_equal([h, kw], c.send(:m, **h)) end @@ -737,9 +729,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L729 define_method(:m) {|arg, **opt| [arg, opt] } end assert_raise(ArgumentError) { c.m(**{}) } - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_equal([kw, kw], c.m(**kw)) - end + assert_raise(ArgumentError) { c.m(**kw) } assert_warn(/The keyword argument is passed as the last hash parameter/m) do assert_equal([h, kw], c.m(**h)) end diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 577b08a..f8d68cc 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1740,7 +1740,7 @@ rb_iseq_only_kwparam_p(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1740 static inline void CALLER_SETUP_ARG(struct rb_control_frame_struct *restrict cfp, struct rb_calling_info *restrict calling, - const struct rb_call_info *restrict ci, int cfunc) + const struct rb_call_info *restrict ci, int remove_empty_keyword_hash) { if (UNLIKELY(IS_ARGS_SPLAT(ci))) { vm_caller_setup_arg_splat(cfp, calling); @@ -1748,6 +1748,13 @@ CALLER_SETUP_ARG(struct rb_control_frame_struct *restrict cfp, https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1748 if (UNLIKELY(IS_ARGS_KEYWORD(ci))) { vm_caller_setup_arg_kw(cfp, calling, ci); } + if (UNLIKELY(calling->kw_splat && remove_empty_keyword_hash)) { + if (RHASH_EMPTY_P(cfp->sp[-1])) { + cfp->sp--; + calling->argc--; + calling->kw_splat = 0; + } + } } #define USE_OPT_HIST 0 @@ -2189,12 +2196,6 @@ vm_call_cfunc_with_frame(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L2196 int argc = calling->argc; int orig_argc = argc; - if (UNLIKELY(calling->kw_splat)) { - if (RHASH_EMPTY_P(*(GET_SP()-1))) { - argc--; - calling->kw_splat = 0; - } - } if (UNLIKELY(IS_ARGS_KW_OR_KW_SPLAT(ci))) { frame_type |= VM_FRAME_FLAG_CFRAME_KW; } @@ -2315,12 +2316,6 @@ vm_call_opt_send(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, struct https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L2316 ci = &ci_entry.ci; ci_entry.ci = *orig_ci; } - unsigned int kw_splat = ci->flag & VM_CALL_KW_SPLAT; - if (!kw_splat && (ci->flag & VM_CALL_KWARG)) { - /* TODO: delegate kw_arg without making a Hash object */ - ci->flag = ci->flag & ~VM_CALL_KWARG; - kw_splat = VM_CALL_KW_SPLAT; - } /* setup new cc */ cc_entry = *orig_cc; @@ -2350,7 +2345,7 @@ vm_call_opt_send(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, struct https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L2345 } cc->me = rb_callable_method_entry_with_refinements(CLASS_OF(calling->recv), ci->mid, NULL); - ci->flag = VM_CALL_FCALL | VM_CALL_OPT_SEND | kw_splat; + ci->flag = VM_CALL_FCALL | VM_CALL_OPT_SEND | (calling->kw_splat ? VM_CALL_KW_SPLAT : 0); return vm_call_method(ec, reg_cfp, calling, ci, cc); } @@ -2924,13 +2919,6 @@ vm_callee_setup_block_arg(rb_execution_context_t *ec, struct rb_calling_info *ca https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L2919 CALLER_SETUP_ARG(cfp, calling, ci, 1); /* splat arg */ - if (UNLIKELY(calling->kw_splat)) { - if (RHASH_EMPTY_P(argv[calling->argc-1])) { - calling->argc--; - calling->kw_splat = 0; - } - } - if (arg_setup_type == arg_setup_block && calling->argc == 1 && iseq->body->param.flags.has_lead && -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/