kotovalexarian-likes-github/ruby--ruby
1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2022-11-09 12:17:21 -05:00
Code Releases Activity

Always remove empty keyword hashes when calling methods

Browse source 
While doing so is not backwards compatible with Ruby 2.6, it is
necessary for generic argument forwarding to work for all methods:

```ruby
def foo(*args, **kw, &block)
  bar(*args, **kw, &block)
end
```

If you do not remove empty keyword hashes, and bar does not accept
keyword arguments, then a call to foo without keyword arguments
calls bar with an extra positional empty hash argument.
This commit is contained in:
Jeremy Evans 2019-09-05 10:36:28 -07:00
parent 55b96c5d2d
commit d1ef73b59c
2 changed files with 34 additions and 45 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

48
test/ruby/test_keyword.rb
View file

@ -185,9 +185,7 @@ class TestKeywordArguments < Test::Unit::TestCase
f = -> { true }
assert_equal(true, f[**{}])
assert_warn(/The keyword argument is passed as the last hash parameter/m) do
assert_raise(ArgumentError) { f[**kw] }
end
assert_equal(true, f[**kw])
assert_warn(/The keyword argument is passed as the last hash parameter/m) do
assert_raise(ArgumentError) { f[**h] }
end
@ -203,9 +201,7 @@ class TestKeywordArguments < Test::Unit::TestCase
f = ->(a) { a }
assert_raise(ArgumentError) { f[**{}] }
assert_warn(/The keyword argument is passed as the last hash parameter/m) do
assert_equal(kw, f[**kw])
end
assert_raise(ArgumentError) { f[**kw] }
assert_warn(/The keyword argument is passed as the last hash parameter/m) do
assert_equal(h, f[**h])
end
@ -283,7 +279,7 @@ class TestKeywordArguments < Test::Unit::TestCase
end
end
assert_equal([], c[**{}].args)
assert_equal([{}], c[**kw].args)
assert_equal([], c[**kw].args)
assert_equal([h], c[**h].args)
assert_equal([h], c[a: 1].args)
assert_equal([h2], c[**h2].args)
@ -294,7 +290,7 @@ class TestKeywordArguments < Test::Unit::TestCase
def initialize; end
end
assert_nil(c[**{}].args)
assert_raise(ArgumentError) { c[**kw] }
assert_nil(c[**kw].args)
assert_raise(ArgumentError) { c[**h] }
assert_raise(ArgumentError) { c[a: 1] }
assert_raise(ArgumentError) { c[**h2] }
@ -307,7 +303,7 @@ class TestKeywordArguments < Test::Unit::TestCase
end
end
assert_raise(ArgumentError) { c[**{}] }
assert_equal(kw, c[**kw].args)
assert_raise(ArgumentError) { c[**kw] }
assert_equal(h, c[**h].args)
assert_equal(h, c[a: 1].args)
assert_equal(h2, c[**h2].args)
@ -333,7 +329,7 @@ class TestKeywordArguments < Test::Unit::TestCase
end
end
assert_raise(ArgumentError) { c[**{}] }
assert_equal([kw, kw], c[**kw].args)
assert_raise(ArgumentError) { c[**kw] }
assert_equal([h, kw], c[**h].args)
assert_equal([h, kw], c[a: 1].args)
assert_equal([h2, kw], c[**h2].args)
@ -365,7 +361,7 @@ class TestKeywordArguments < Test::Unit::TestCase
args
end
assert_equal([], c.method(:m)[**{}])
assert_equal([{}], c.method(:m)[**kw])
assert_equal([], c.method(:m)[**kw])
assert_equal([h], c.method(:m)[**h])
assert_equal([h], c.method(:m)[a: 1])
assert_equal([h2], c.method(:m)[**h2])
@ -375,7 +371,7 @@ class TestKeywordArguments < Test::Unit::TestCase
c.singleton_class.remove_method(:m)
def c.m; end
assert_nil(c.method(:m)[**{}])
assert_raise(ArgumentError) { c.method(:m)[**kw] }
assert_nil(c.method(:m)[**kw])
assert_raise(ArgumentError) { c.method(:m)[**h] }
assert_raise(ArgumentError) { c.method(:m)[a: 1] }
assert_raise(ArgumentError) { c.method(:m)[**h2] }
@ -387,7 +383,7 @@ class TestKeywordArguments < Test::Unit::TestCase
args
end
assert_raise(ArgumentError) { c.method(:m)[**{}] }
assert_equal(kw, c.method(:m)[**kw])
assert_raise(ArgumentError) { c.method(:m)[**kw] }
assert_equal(h, c.method(:m)[**h])
assert_equal(h, c.method(:m)[a: 1])
assert_equal(h2, c.method(:m)[**h2])
@ -411,7 +407,7 @@ class TestKeywordArguments < Test::Unit::TestCase
[arg, args]
end
assert_raise(ArgumentError) { c.method(:m)[**{}] }
assert_equal([kw, kw], c.method(:m)[**kw])
assert_raise(ArgumentError) { c.method(:m)[**kw] }
assert_equal([h, kw], c.method(:m)[**h])
assert_equal([h, kw], c.method(:m)[a: 1])
assert_equal([h2, kw], c.method(:m)[**h2])
@ -487,9 +483,7 @@ class TestKeywordArguments < Test::Unit::TestCase
[arg, args]
end
assert_raise(ArgumentError) { c.send(:m, **{}) }
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, **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
@ -576,9 +570,7 @@ class TestKeywordArguments < Test::Unit::TestCase
[arg, args]
end
assert_raise(ArgumentError) { :m.to_proc.call(c, **{}) }
assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
assert_equal([kw, kw], :m.to_proc.call(c, **kw))
end
assert_raise(ArgumentError) { :m.to_proc.call(c, **kw) }
assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
assert_equal([h, kw], :m.to_proc.call(c, **h))
end
@ -664,9 +656,7 @@ class TestKeywordArguments < Test::Unit::TestCase
[arg, args]
end
assert_raise(ArgumentError) { c.m(**{}) }
assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/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.* for `method_missing'/m) do
assert_equal([h, kw], c.m(**h))
end
@ -707,9 +697,7 @@ class TestKeywordArguments < Test::Unit::TestCase
define_method(:m) { }
end
assert_nil(c.m(**{}))
assert_warn(/The keyword argument is passed as the last hash parameter/m) do
assert_raise(ArgumentError) { c.m(**kw) }
end
assert_nil(c.m(**kw))
assert_warn(/The keyword argument is passed as the last hash parameter/m) do
assert_raise(ArgumentError) { c.m(**h) }
end
@ -731,9 +719,7 @@ class TestKeywordArguments < Test::Unit::TestCase
define_method(:m) {|arg| arg }
end
assert_raise(ArgumentError) { c.m(**{}) }
assert_warn(/The keyword argument is passed as the last hash parameter/m) do
assert_equal(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, c.m(**h))
end
@ -779,9 +765,7 @@ class TestKeywordArguments < Test::Unit::TestCase
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

31
vm_insnhelper.c
View file

@ -1740,7 +1740,7 @@ rb_iseq_only_kwparam_p(const rb_iseq_t *iseq)
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 remove_empty_keyword_hash)
const struct rb_call_info *restrict ci)
{
if (UNLIKELY(IS_ARGS_SPLAT(ci))) {
/* This expands the rest argument to the stack.
@ -1755,9 +1755,14 @@ CALLER_SETUP_ARG(struct rb_control_frame_struct *restrict cfp,
*/
vm_caller_setup_arg_kw(cfp, calling, ci);
}
if (UNLIKELY(calling->kw_splat && remove_empty_keyword_hash)) {
if (UNLIKELY(calling->kw_splat)) {
/* This removes the last Hash object if it is empty.
* So, ci->flag & VM_CALL_KW_SPLAT is now inconsistent.
* However, you can use ci->flag & VM_CALL_KW_SPLAT to
* determine whether a hash should be added back with
* warning (for backwards compatibility in cases where
* the method does not have the number of required
* arguments.
*/
if (RHASH_EMPTY_P(cfp->sp[-1])) {
cfp->sp--;
@ -1873,7 +1878,7 @@ vm_callee_setup_arg(rb_execution_context_t *ec, struct rb_calling_info *calling,
if (LIKELY(!(ci->flag & VM_CALL_KW_SPLAT))) {
if (LIKELY(rb_simple_iseq_p(iseq))) {
rb_control_frame_t *cfp = ec->cfp;
CALLER_SETUP_ARG(cfp, calling, ci, 0); /* splat arg */
CALLER_SETUP_ARG(cfp, calling, ci);
if (calling->argc != iseq->body->param.lead_num) {
argument_arity_error(ec, iseq, calling->argc, iseq->body->param.lead_num, iseq->body->param.lead_num);
@ -1884,7 +1889,7 @@ vm_callee_setup_arg(rb_execution_context_t *ec, struct rb_calling_info *calling,
}
else if (rb_iseq_only_optparam_p(iseq)) {
rb_control_frame_t *cfp = ec->cfp;
CALLER_SETUP_ARG(cfp, calling, ci, 0); /* splat arg */
CALLER_SETUP_ARG(cfp, calling, ci);
const int lead_num = iseq->body->param.lead_num;
const int opt_num = iseq->body->param.opt_num;
@ -2237,7 +2242,7 @@ vm_call_cfunc(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, struct rb
{
RB_DEBUG_COUNTER_INC(ccf_cfunc);
CALLER_SETUP_ARG(reg_cfp, calling, ci, 0);
CALLER_SETUP_ARG(reg_cfp, calling, ci);
return vm_call_cfunc_with_frame(ec, reg_cfp, calling, ci, cc);
}
@ -2279,7 +2284,7 @@ vm_call_bmethod(rb_execution_context_t *ec, rb_control_frame_t *cfp, struct rb_c
VALUE *argv;
int argc;
CALLER_SETUP_ARG(cfp, calling, ci, 0);
CALLER_SETUP_ARG(cfp, calling, ci);
argc = calling->argc;
argv = ALLOCA_N(VALUE, argc);
MEMCPY(argv, cfp->sp - argc, VALUE, argc);
@ -2309,7 +2314,7 @@ vm_call_opt_send(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, struct
struct rb_call_info_with_kwarg ci_entry;
struct rb_call_cache cc_entry, *cc;
CALLER_SETUP_ARG(reg_cfp, calling, orig_ci, 0);
CALLER_SETUP_ARG(reg_cfp, calling, orig_ci);
i = calling->argc - 1;
@ -2414,7 +2419,7 @@ vm_call_method_missing(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp,
struct rb_call_cache cc_entry, *cc;
unsigned int argc;
CALLER_SETUP_ARG(reg_cfp, calling, orig_ci, 0);
CALLER_SETUP_ARG(reg_cfp, calling, orig_ci);
argc = calling->argc+1;
ci_entry.flag = VM_CALL_FCALL | VM_CALL_OPT_SEND | (calling->kw_splat ? VM_CALL_KW_SPLAT : 0);
@ -2619,14 +2624,14 @@ vm_call_method_each_type(rb_execution_context_t *ec, rb_control_frame_t *cfp, st
return vm_call_cfunc(ec, cfp, calling, ci, cc);
case VM_METHOD_TYPE_ATTRSET:
CALLER_SETUP_ARG(cfp, calling, ci, 1);
CALLER_SETUP_ARG(cfp, calling, ci);
rb_check_arity(calling->argc, 1, 1);
cc->aux.index = 0;
CC_SET_FASTPATH(cc, vm_call_attrset, !((ci->flag & VM_CALL_ARGS_SPLAT) || (ci->flag & VM_CALL_KWARG)));
return vm_call_attrset(ec, cfp, calling, ci, cc);
case VM_METHOD_TYPE_IVAR:
CALLER_SETUP_ARG(cfp, calling, ci, 1);
CALLER_SETUP_ARG(cfp, calling, ci);
rb_check_arity(calling->argc, 0, 0);
cc->aux.index = 0;
CC_SET_FASTPATH(cc, vm_call_ivar, !(ci->flag & VM_CALL_ARGS_SPLAT));
@ -2927,7 +2932,7 @@ vm_callee_setup_block_arg(rb_execution_context_t *ec, struct rb_calling_info *ca
rb_control_frame_t *cfp = ec->cfp;
VALUE arg0;
CALLER_SETUP_ARG(cfp, calling, ci, 0); /* splat arg */
CALLER_SETUP_ARG(cfp, calling, ci);
if (calling->kw_splat) {
rb_warn_keyword_to_last_hash(calling, ci, iseq);
@ -3015,7 +3020,7 @@ vm_invoke_symbol_block(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp,
{
VALUE val;
int argc;
CALLER_SETUP_ARG(ec->cfp, calling, ci, 0);
CALLER_SETUP_ARG(ec->cfp, calling, ci);
argc = calling->argc;
val = vm_yield_with_symbol(ec, symbol, argc, STACK_ADDR_FROM_TOP(argc), calling->kw_splat, calling->block_handler);
POPN(argc);
@ -3029,7 +3034,7 @@ vm_invoke_ifunc_block(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp,
{
VALUE val;
int argc;
CALLER_SETUP_ARG(ec->cfp, calling, ci, 0);
CALLER_SETUP_ARG(ec->cfp, calling, ci);
argc = calling->argc;
val = vm_yield_with_cfunc(ec, captured, captured->self, argc, STACK_ADDR_FROM_TOP(argc), calling->kw_splat, calling->block_handler, NULL);
POPN(argc); /* TODO: should put before C/yield? */