ruby-changes:57627
From: Jeremy <ko1@a...>
Date: Sat, 7 Sep 2019 11:50:13 +0900 (JST)
Subject: [ruby-changes:57627] 37a2c660aa (master): Convert keyword argument to required positional hash argument for Class#new, Method#call, UnboundMethod#bind_call
https://git.ruby-lang.org/ruby.git/commit/?id=37a2c660aa From 37a2c660aa4f4aacfd6a56651b10124e3ac01321 Mon Sep 17 00:00:00 2001 From: Jeremy Evans <code@j...> Date: Thu, 5 Sep 2019 19:25:34 -0700 Subject: Convert keyword argument to required positional hash argument for Class#new, Method#call, UnboundMethod#bind_call Also add keyword argument separation warnings for Class#new and Method#call. To allow for keyword argument to required positional hash converstion in cfuncs, add a vm frame flag indicating the cfunc was called with an empty keyword hash (which was removed before calling the cfunc). The cfunc can check this frame flag and add back an empty hash if it is passing its arguments to another Ruby method. Add rb_empty_keyword_given_p function for checking if called with an empty keyword hash, and rb_add_empty_keyword for adding back an empty hash to argv. All of this empty keyword argument support is only for 2.7. It will be removed in 3.0 as Ruby 3 will not convert empty keyword arguments to required positional hash arguments. Comment all of the relevent code to make it obvious this is expected to be removed. Add rb_funcallv_kw as an public C-API function, just like rb_funcallv but with a keyword flag. This is used by rb_obj_call_init (internals of Class#new). This also required expected call_type enum with CALL_FCALL_KW, similar to the recent addition of CALL_PUBLIC_KW. Add rb_vm_call_kw as a internal function, used by call_method_data (internals of Method#call and UnboundMethod#bind_call). Add tests for UnboundMethod#bind_call keyword handling. diff --git a/eval.c b/eval.c index c588171..fbdde7d 100644 --- a/eval.c +++ b/eval.c @@ -907,6 +907,22 @@ rb_keyword_given_p(void) https://github.com/ruby/ruby/blob/trunk/eval.c#L907 return rb_vm_cframe_keyword_p(GET_EC()->cfp); } +/* -- Remove In 3.0 -- */ +int rb_vm_cframe_empty_keyword_p(const rb_control_frame_t *cfp); +int +rb_empty_keyword_given_p(void) +{ + return rb_vm_cframe_empty_keyword_p(GET_EC()->cfp); +} +VALUE * +rb_add_empty_keyword(int argc, const VALUE *argv) +{ + VALUE *ptr = ALLOC_N(VALUE,argc+1); + memcpy(ptr, argv, sizeof(VALUE)*(argc)); + ptr[argc] = rb_hash_new(); + return ptr; +} + VALUE rb_eThreadError; /*! Declares that the current method needs a block. @@ -1664,7 +1680,12 @@ void https://github.com/ruby/ruby/blob/trunk/eval.c#L1680 rb_obj_call_init(VALUE obj, int argc, const VALUE *argv) { PASS_PASSED_BLOCK_HANDLER(); - rb_funcallv(obj, idInitialize, argc, argv); + if (rb_empty_keyword_given_p()) { + rb_funcallv_kw(obj, idInitialize, argc+1, rb_add_empty_keyword(argc, argv), 1); + } + else { + rb_funcallv_kw(obj, idInitialize, argc, argv, rb_keyword_given_p()); + } } /*! diff --git a/include/ruby/ruby.h b/include/ruby/ruby.h index 65bcd38..1e4bafc 100644 --- a/include/ruby/ruby.h +++ b/include/ruby/ruby.h @@ -1887,6 +1887,7 @@ VALUE rb_eval_string_protect(const char*, int*); https://github.com/ruby/ruby/blob/trunk/include/ruby/ruby.h#L1887 VALUE rb_eval_string_wrap(const char*, int*); VALUE rb_funcall(VALUE, ID, int, ...); VALUE rb_funcallv(VALUE, ID, int, const VALUE*); +VALUE rb_funcallv_kw(VALUE, ID, int, const VALUE*, int); VALUE rb_funcallv_public(VALUE, ID, int, const VALUE*); #define rb_funcall2 rb_funcallv #define rb_funcall3 rb_funcallv_public diff --git a/internal.h b/internal.h index 21491c3..7446ce6 100644 --- a/internal.h +++ b/internal.h @@ -1552,6 +1552,10 @@ void rb_class_modify_check(VALUE); https://github.com/ruby/ruby/blob/trunk/internal.h#L1552 #define id_status ruby_static_id_status NORETURN(VALUE rb_f_raise(int argc, VALUE *argv)); +/* -- Remove In 3.0 -- */ +int rb_empty_keyword_given_p(void); +VALUE * rb_add_empty_keyword(int argc, const VALUE *argv); + /* eval_error.c */ VALUE rb_get_backtrace(VALUE info); diff --git a/proc.c b/proc.c index db2f62e..3c65d3d 100644 --- a/proc.c +++ b/proc.c @@ -2223,8 +2223,14 @@ call_method_data(rb_execution_context_t *ec, const struct METHOD *data, https://github.com/ruby/ruby/blob/trunk/proc.c#L2223 int argc, const VALUE *argv, VALUE passed_procval) { vm_passed_block_handler_set(ec, proc_to_block_handler(passed_procval)); - return rb_vm_call(ec, data->recv, data->me->called_id, argc, argv, - method_callable_method_entry(data)); + if (rb_empty_keyword_given_p()) { + return rb_vm_call_kw(ec, data->recv, data->me->called_id, argc+1, rb_add_empty_keyword(argc, argv), + method_callable_method_entry(data), 1); + } + else { + return rb_vm_call_kw(ec, data->recv, data->me->called_id, argc, argv, + method_callable_method_entry(data), rb_keyword_given_p()); + } } static VALUE diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb index 7624e6d..9c8e60a 100644 --- a/test/ruby/test_keyword.rb +++ b/test/ruby/test_keyword.rb @@ -340,7 +340,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L340 assert_equal([1, h3], f[a: 1, **h2]) end - def test_cfunc_kwsplat_call + def test_Class_new_kwsplat_call kw = {} h = {:a=>1} h2 = {'a'=>1} @@ -382,8 +382,12 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L382 @args = args end end - assert_raise(ArgumentError) { c[**{}] } - assert_raise(ArgumentError) { c[**kw] } + assert_warn(/The keyword argument is passed as the last hash parameter.* for `initialize'/m) do + assert_equal(kw, c[**{}].args) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `initialize'/m) do + assert_equal(kw, c[**kw].args) + end assert_equal(h, c[**h].args) assert_equal(h, c[a: 1].args) assert_equal(h2, c[**h2].args) @@ -408,13 +412,27 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L412 @args = [arg, args] end end - assert_raise(ArgumentError) { c[**{}] } - 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) - assert_equal([h3, kw], c[**h3].args) - assert_equal([h3, kw], c[a: 1, **h2].args) + assert_warn(/The keyword argument is passed as the last hash parameter.* for `initialize'/m) do + assert_equal([kw, kw], c[**{}].args) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `initialize'/m) do + assert_equal([kw, kw], c[**kw].args) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `initialize'/m) do + assert_equal([h, kw], c[**h].args) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `initialize'/m) do + assert_equal([h, kw], c[a: 1].args) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `initialize'/m) do + assert_equal([h2, kw], c[**h2].args) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `initialize'/m) do + assert_equal([h3, kw], c[**h3].args) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `initialize'/m) do + assert_equal([h3, kw], c[a: 1, **h2].args) + end c = Class.new(sc) do def initialize(arg=1, **args) @@ -430,7 +448,7 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L448 assert_equal([1, h3], c[a: 1, **h2].args) end - def test_method_kwsplat_call + def test_Method_call_kwsplat_call kw = {} h = {:a=>1} h2 = {'a'=>1} @@ -462,8 +480,12 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L480 def c.m(args) args end - assert_raise(ArgumentError) { c.method(:m)[**{}] } - assert_raise(ArgumentError) { c.method(:m)[**kw] } + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal(kw, c.method(:m)[**{}]) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal(kw, c.method(:m)[**kw]) + end assert_equal(h, c.method(:m)[**h]) assert_equal(h, c.method(:m)[a: 1]) assert_equal(h2, c.method(:m)[**h2]) @@ -486,13 +508,27 @@ class TestKeywordArguments < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L508 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([h, kw], c.method(:m)[a: 1]) - assert_equal([h2, kw], c.method(:m)[**h2]) - assert_equal([h3, kw], c.method(:m)[**h3]) - assert_equal([h3, kw], c.method(:m)[a: 1, **h2]) + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal([kw, kw], c.method(:m)[**{}]) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal([kw, kw], c.method(:m)[**kw]) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal([h, kw], c.method(:m)[**h]) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal([h, kw], c.method(:m)[a: 1]) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal([h2, kw], c.method(:m)[**h2]) + end + assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do + assert_equal([h3, kw], c.method(:m)[**h3]) + end + assert_w (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/