From 15757390fff14071f3d77fa75934b741723eb92a Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Sat, 31 Aug 2019 21:51:02 -0700 Subject: [PATCH] Don't pass an empty keyword hash when double splatting empty hash when calling cfunc This mirrors earlier changes in keyword argument separation for calling Ruby methods and calling procs/lambdas, so that behavior is kept the same. --- test/ruby/test_keyword.rb | 138 ++++++++++++++++++++++++++++++++++++++ vm_insnhelper.c | 9 ++- 2 files changed, 146 insertions(+), 1 deletion(-) diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb index b27b5dc596..ef7be79d98 100644 --- a/test/ruby/test_keyword.rb +++ b/test/ruby/test_keyword.rb @@ -227,6 +227,144 @@ class TestKeywordArguments < Test::Unit::TestCase assert_equal([1, h3], f[**h3]) end + def test_cfunc_kwsplat_call + kw = {} + h = {'a'=>1} + h2 = {'a'=>1} + h3 = {'a'=>1, :a=>1} + + sc = Class.new do + attr_reader :args + class << self + alias [] new + end + end + + c = Class.new(sc) do + def initialize(*args) + @args = args + end + end + assert_equal([], c[**{}].args) + assert_equal([], c[**kw].args) + assert_equal([h], c[**h].args) + assert_equal([h2], c[**h2].args) + assert_equal([h3], c[**h3].args) + + c = Class.new(sc) do + def initialize; end + end + assert_nil(c[**{}].args) + assert_nil(c[**kw].args) + assert_raise(ArgumentError) { c[**h] } + assert_raise(ArgumentError) { c[**h2] } + assert_raise(ArgumentError) { c[**h3] } + + c = Class.new(sc) do + def initialize(args) + @args = args + end + end + assert_raise(ArgumentError) { c[**{}] } + assert_raise(ArgumentError) { c[**kw] } + assert_equal(h, c[**h].args) + assert_equal(h2, c[**h2].args) + assert_equal(h3, c[**h3].args) + + c = Class.new(sc) do + def initialize(**args) + @args = args + end + end + assert_equal(kw, c[**{}].args) + assert_equal(kw, c[**kw].args) + assert_equal(h, c[**h].args) + assert_equal(h2, c[**h2].args) + assert_equal(h3, c[**h3].args) + + c = Class.new(sc) do + def initialize(arg, **args) + @args = [arg, args] + end + end + assert_raise(ArgumentError) { c[**{}] } + assert_raise(ArgumentError) { c[**kw] } + assert_equal([h, kw], c[**h].args) + assert_equal([h2, kw], c[**h2].args) + assert_equal([h3, kw], c[**h3].args) + + c = Class.new(sc) do + def initialize(arg=1, **args) + @args = [arg=1, args] + end + end + assert_equal([1, kw], c[**{}].args) + assert_equal([1, kw], c[**kw].args) + assert_equal([1, h], c[**h].args) + assert_equal([1, h2], c[**h2].args) + assert_equal([1, h3], c[**h3].args) + end + + def test_method_kwsplat_call + kw = {} + h = {'a'=>1} + h2 = {'a'=>1} + h3 = {'a'=>1, :a=>1} + + c = Object.new + def c.m(*args) + args + end + assert_equal([], c.method(:m)[**{}]) + assert_equal([], c.method(:m)[**kw]) + assert_equal([h], c.method(:m)[**h]) + assert_equal([h2], c.method(:m)[**h2]) + assert_equal([h3], c.method(:m)[**h3]) + + def c.m; end + assert_nil(c.method(:m)[**{}]) + assert_nil(c.method(:m)[**kw]) + assert_raise(ArgumentError) { c.method(:m)[**h] } + assert_raise(ArgumentError) { c.method(:m)[**h2] } + assert_raise(ArgumentError) { c.method(:m)[**h3] } + + def c.m(args) + args + end + assert_raise(ArgumentError) { c.method(:m)[**{}] } + assert_raise(ArgumentError) { c.method(:m)[**kw] } + assert_equal(h, c.method(:m)[**h]) + assert_equal(h2, c.method(:m)[**h2]) + assert_equal(h3, c.method(:m)[**h3]) + + def c.m(**args) + args + end + assert_equal(kw, c.method(:m)[**{}]) + assert_equal(kw, c.method(:m)[**kw]) + assert_equal(h, c.method(:m)[**h]) + assert_equal(h2, c.method(:m)[**h2]) + assert_equal(h3, c.method(:m)[**h3]) + + def c.m(arg, **args) + [arg, args] + end + assert_raise(ArgumentError) { c.method(:m)[**{}] } + assert_raise(ArgumentError) { c.method(:m)[**kw] } + assert_equal([h, kw], c.method(:m)[**h]) + assert_equal([h2, kw], c.method(:m)[**h2]) + assert_equal([h3, kw], c.method(:m)[**h3]) + + def c.m(arg=1, **args) + [arg=1, args] + end + assert_equal([1, kw], c.method(:m)[**{}]) + assert_equal([1, kw], c.method(:m)[**kw]) + assert_equal([1, h], c.method(:m)[**h]) + assert_equal([1, h2], c.method(:m)[**h2]) + assert_equal([1, h3], c.method(:m)[**h3]) + end + def p1 Proc.new do |str: "foo", num: 424242| [str, num] diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 71c5930b2e..ae39472b54 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -2186,6 +2186,13 @@ vm_call_cfunc_with_frame(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp VALUE recv = calling->recv; VALUE block_handler = calling->block_handler; int argc = calling->argc; + int orig_argc = argc; + + if (UNLIKELY(IS_ARGS_KW_SPLAT(ci))) { + if (RHASH_EMPTY_P(*(GET_SP()-1))) { + argc--; + } + } RUBY_DTRACE_CMETHOD_ENTRY_HOOK(ec, me->owner, me->def->original_id); EXEC_EVENT_HOOK(ec, RUBY_EVENT_C_CALL, recv, me->def->original_id, ci->mid, me->owner, Qundef); @@ -2196,7 +2203,7 @@ vm_call_cfunc_with_frame(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp if (len >= 0) rb_check_arity(argc, len, len); - reg_cfp->sp -= argc + 1; + reg_cfp->sp -= orig_argc + 1; val = (*cfunc->invoker)(recv, argc, reg_cfp->sp + 1, cfunc->func); CHECK_CFP_CONSISTENCY("vm_call_cfunc");