ruby-changes:57595
From: Jeremy <ko1@a...>
Date: Fri, 6 Sep 2019 12:55:12 +0900 (JST)
Subject: [ruby-changes:57595] d1ef73b59c (master): Always remove empty keyword hashes when calling methods
https://git.ruby-lang.org/ruby.git/commit/?id=d1ef73b59c From d1ef73b59cede58f2173fa0f4ff7480a820f25d6 Mon Sep 17 00:00:00 2001 From: Jeremy Evans <code@j...> Date: Thu, 5 Sep 2019 10:36:28 -0700 Subject: Always remove empty keyword hashes when calling methods 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. diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb index 71b72bd..b337343 100644 --- a/test/ruby/test_keyword.rb +++ b/test/ruby/test_keyword.rb @@ -185,9 +185,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L185 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L201 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L279 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L290 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L303 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L329 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L361 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L371 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L383 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L407 [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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L483 [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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L570 [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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L656 [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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L697 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L719 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 https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L765 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 baf93de..4ae94e3 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 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, https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1755 */ 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-> (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/