ruby-changes:57606
From: Jeremy <ko1@a...>
Date: Fri, 6 Sep 2019 16:25:10 +0900 (JST)
Subject: [ruby-changes:57606] e2878a96f7 (master): Convert empty keyword hash to required positional argument and warn for lambda and bmethod
https://git.ruby-lang.org/ruby.git/commit/?id=e2878a96f7 From e2878a96f77978b224f8461244cd3e1efc248d83 Mon Sep 17 00:00:00 2001 From: Jeremy Evans <code@j...> Date: Thu, 5 Sep 2019 12:43:06 -0700 Subject: Convert empty keyword hash to required positional argument and warn for lambda and bmethod The lambda case is similar to the attr_writer case, except we have to determine the number of required parameters from the iseq instead of being able to assume a single required parameter. This fixes a lot of lambda tests which were switched to require warnings for all usage of keyword arguments. Similar to method handling, we do not warn when passing keyword arguments to lambdas that do not accept keyword arguments, the argument is just passed as a positional hash in that case, unless it is empty. If it is empty and not the final required parameter, then we ignore it. If it is empty and the final required parameter, then we pass it for backwards compatibility and emit a warning, as in Ruby 3 we will not pass it. The bmethod case is similar to the send case, in that we do not want to remove empty keyword splats in vm_call_bmethod, as that prevents later call handling from moving them to required positional arguments and warning. diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb index 81ac432..c5b9c15 100644 --- a/test/ruby/test_keyword.rb +++ b/test/ruby/test_keyword.rb @@ -280,37 +280,23 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L280 f = -> { true } assert_equal(true, f[**{}]) 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 - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_raise(ArgumentError) { f[a: 1] } - end - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_raise(ArgumentError) { f[**h2] } - end - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_raise(ArgumentError) { f[**h3] } - end + assert_raise(ArgumentError) { f[**h] } + assert_raise(ArgumentError) { f[a: 1] } + assert_raise(ArgumentError) { f[**h2] } + assert_raise(ArgumentError) { f[**h3] } f = ->(a) { a } - assert_raise(ArgumentError) { f[**{}] } - assert_raise(ArgumentError) { f[**kw] } assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_equal(h, f[**h]) + assert_equal(kw, f[**{}]) end assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_equal(h, f[a: 1]) - end - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_equal(h2, f[**h2]) - end - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_equal(h3, f[**h3]) - end - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_equal(h3, f[a: 1, **h2]) + assert_equal(kw, f[**kw]) end + assert_equal(h, f[**h]) + assert_equal(h, f[a: 1]) + assert_equal(h2, f[**h2]) + assert_equal(h3, f[**h3]) + assert_equal(h3, f[a: 1, **h2]) f = ->(**x) { x } assert_equal(kw, f[**{}]) @@ -800,43 +786,27 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L786 end assert_nil(c.m(**{})) 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 - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_raise(ArgumentError) { c.m(a: 1) } - end - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_raise(ArgumentError) { c.m(**h2) } - end - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_raise(ArgumentError) { c.m(**h3) } - end - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_raise(ArgumentError) { c.m(a: 1, **h2) } - end + assert_raise(ArgumentError) { c.m(**h) } + assert_raise(ArgumentError) { c.m(a: 1) } + assert_raise(ArgumentError) { c.m(**h2) } + assert_raise(ArgumentError) { c.m(**h3) } + assert_raise(ArgumentError) { c.m(a: 1, **h2) } c = Object.new class << c define_method(:m) {|arg| arg } end - assert_raise(ArgumentError) { c.m(**{}) } - 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 assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_equal(h, c.m(a: 1)) - end - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_equal(h2, c.m(**h2)) - end - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_equal(h3, c.m(**h3)) + assert_equal(kw, c.m(**{})) end assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_equal(h3, c.m(a: 1, **h2)) + assert_equal(kw, c.m(**kw)) end + assert_equal(h, c.m(**h)) + assert_equal(h, c.m(a: 1)) + assert_equal(h2, c.m(**h2)) + assert_equal(h3, c.m(**h3)) + assert_equal(h3, c.m(a: 1, **h2)) c = Object.new class << c @@ -866,8 +836,12 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L836 class << c define_method(:m) {|arg, **opt| [arg, opt] } end - assert_raise(ArgumentError) { c.m(**{}) } - assert_raise(ArgumentError) { c.m(**kw) } + assert_warn(/The keyword argument is passed as the last hash parameter/m) do + assert_equal([kw, kw], c.m(**{})) + end + assert_warn(/The keyword argument is passed as the last hash parameter/m) do + assert_equal([kw, kw], c.m(**kw)) + end assert_warn(/The keyword argument is passed as the last hash parameter/m) do assert_equal([h, kw], c.m(**h)) end @@ -1347,14 +1321,10 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L1321 o = Object.new def o.to_hash() { a: 1 } end - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_equal({a: 1}, m1(**o) {|x| break x}, bug9898) - end + assert_equal({a: 1}, m1(**o) {|x| break x}, bug9898) o2 = Object.new def o2.to_hash() { b: 2 } end - assert_warn(/The keyword argument is passed as the last hash parameter/m) do - assert_equal({a: 1, b: 2}, m1(**o, **o2) {|x| break x}, bug9898) - end + assert_equal({a: 1, b: 2}, m1(**o, **o2) {|x| break x}, bug9898) end def test_implicit_hash_conversion diff --git a/vm_insnhelper.c b/vm_insnhelper.c index fe333d1..b1ea71a 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -2294,7 +2294,7 @@ vm_call_bmethod(rb_execution_context_t *ec, rb_control_frame_t *cfp, struct rb_c https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L2294 VALUE *argv; int argc; - CALLER_SETUP_ARG(cfp, calling, ci); + CALLER_SETUP_ARG_WITHOUT_KW_SPLAT(cfp, calling, ci); argc = calling->argc; argv = ALLOCA_N(VALUE, argc); MEMCPY(argv, cfp->sp - argc, VALUE, argc); @@ -2949,11 +2949,13 @@ 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#L2949 rb_control_frame_t *cfp = ec->cfp; VALUE arg0; - CALLER_SETUP_ARG(cfp, calling, ci); - - if (calling->kw_splat) { + if (calling->kw_splat && calling->argc == iseq->body->param.lead_num + iseq->body->param.post_num && RHASH_EMPTY_P(cfp->sp[-1])) { + CALLER_SETUP_ARG_WITHOUT_KW_SPLAT(cfp, calling, ci); rb_warn_keyword_to_last_hash(calling, ci, iseq); } + else { + CALLER_SETUP_ARG(cfp, calling, ci); + } if (arg_setup_type == arg_setup_block && calling->argc == 1 && -- cgit v0.10.2 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/