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.

9 files changed, 238 insertions, 28 deletions

diff --git a/eval.c b/eval.c
index c588171b0a..fbdde7da9b 100644
--- a/eval.c
+++ b/eval.c
@@ -907,6 +907,22 @@ rb_keyword_given_p(void)
     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
 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 65bcd382f2..1e4bafcc86 100644
--- a/include/ruby/ruby.h
+++ b/include/ruby/ruby.h
@@ -1887,6 +1887,7 @@ VALUE rb_eval_string_protect(const char*, int*);
 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 21491c317f..7446ce6041 100644
--- a/internal.h
+++ b/internal.h
@@ -1552,6 +1552,10 @@ void rb_class_modify_check(VALUE);
 #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 db2f62e090..3c65d3d0ac 100644
--- a/proc.c
+++ b/proc.c
@@ -2223,8 +2223,14 @@ call_method_data(rb_execution_context_t *ec, const struct METHOD *data,
 		 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 7624e6d386..9c8e60a2f8 100644
--- a/test/ruby/test_keyword.rb
+++ b/test/ruby/test_keyword.rb
@@ -340,7 +340,7 @@ class TestKeywordArguments < Test::Unit::TestCase
     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
         @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
         @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
     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
     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
     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_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([h3, kw], c.method(:m)[a: 1, **h2])
+    end
 
     c.singleton_class.remove_method(:m)
     def c.m(arg=1, **args)
@@ -507,6 +543,102 @@ class TestKeywordArguments < Test::Unit::TestCase
     assert_equal([1, h3], c.method(:m)[a: 1, **h2])
   end
 
+  def test_UnboundMethod_bindcall_kwsplat_call
+    kw = {}
+    h = {:a=>1}
+    h2 = {'a'=>1}
+    h3 = {'a'=>1, :a=>1}
+
+    c = Object.new
+    sc = c.singleton_class
+    def c.m(*args)
+      args
+    end
+    assert_equal([], sc.instance_method(:m).bind_call(c, **{}))
+    assert_equal([], sc.instance_method(:m).bind_call(c, **kw))
+    assert_equal([h], sc.instance_method(:m).bind_call(c, **h))
+    assert_equal([h], sc.instance_method(:m).bind_call(c, a: 1))
+    assert_equal([h2], sc.instance_method(:m).bind_call(c, **h2))
+    assert_equal([h3], sc.instance_method(:m).bind_call(c, **h3))
+    assert_equal([h3], sc.instance_method(:m).bind_call(c, a: 1, **h2))
+
+    sc.remove_method(:m)
+    def c.m; end
+    assert_nil(sc.instance_method(:m).bind_call(c, **{}))
+    assert_nil(sc.instance_method(:m).bind_call(c, **kw))
+    assert_raise(ArgumentError) { sc.instance_method(:m).bind_call(c, **h) }
+    assert_raise(ArgumentError) { sc.instance_method(:m).bind_call(c, a: 1) }
+    assert_raise(ArgumentError) { sc.instance_method(:m).bind_call(c, **h2) }
+    assert_raise(ArgumentError) { sc.instance_method(:m).bind_call(c, **h3) }
+    assert_raise(ArgumentError) { sc.instance_method(:m).bind_call(c, a: 1, **h2) }
+
+    sc.remove_method(:m)
+    def c.m(args)
+      args
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal(kw, sc.instance_method(:m).bind_call(c, **{}))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal(kw, sc.instance_method(:m).bind_call(c, **kw))
+    end
+    assert_equal(h, sc.instance_method(:m).bind_call(c, **h))
+    assert_equal(h, sc.instance_method(:m).bind_call(c, a: 1))
+    assert_equal(h2, sc.instance_method(:m).bind_call(c, **h2))
+    assert_equal(h3, sc.instance_method(:m).bind_call(c, **h3))
+    assert_equal(h3, sc.instance_method(:m).bind_call(c, a: 1, **h2))
+
+    sc.remove_method(:m)
+    def c.m(**args)
+      args
+    end
+    assert_equal(kw, sc.instance_method(:m).bind_call(c, **{}))
+    assert_equal(kw, sc.instance_method(:m).bind_call(c, **kw))
+    assert_equal(h, sc.instance_method(:m).bind_call(c, **h))
+    assert_equal(h, sc.instance_method(:m).bind_call(c, a: 1))
+    assert_equal(h2, sc.instance_method(:m).bind_call(c, **h2))
+    assert_equal(h3, sc.instance_method(:m).bind_call(c, **h3))
+    assert_equal(h3, sc.instance_method(:m).bind_call(c, a: 1, **h2))
+
+    sc.remove_method(:m)
+    def c.m(arg, **args)
+      [arg, args]
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([kw, kw], sc.instance_method(:m).bind_call(c, **{}))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([kw, kw], sc.instance_method(:m).bind_call(c, **kw))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([h, kw], sc.instance_method(:m).bind_call(c, **h))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([h, kw], sc.instance_method(:m).bind_call(c, a: 1))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([h2, kw], sc.instance_method(:m).bind_call(c, **h2))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([h3, kw], sc.instance_method(:m).bind_call(c, **h3))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([h3, kw], sc.instance_method(:m).bind_call(c, a: 1, **h2))
+    end
+
+    sc.remove_method(:m)
+    def c.m(arg=1, **args)
+      [arg=1, args]
+    end
+    assert_equal([1, kw], sc.instance_method(:m).bind_call(c, **{}))
+    assert_equal([1, kw], sc.instance_method(:m).bind_call(c, **kw))
+    assert_equal([1, h], sc.instance_method(:m).bind_call(c, **h))
+    assert_equal([1, h], sc.instance_method(:m).bind_call(c, a: 1))
+    assert_equal([1, h2], sc.instance_method(:m).bind_call(c, **h2))
+    assert_equal([1, h3], sc.instance_method(:m).bind_call(c, **h3))
+    assert_equal([1, h3], sc.instance_method(:m).bind_call(c, a: 1, **h2))
+  end
+
   def test_send_kwsplat
     kw = {}
     h = {:a=>1}
diff --git a/vm.c b/vm.c
index 8ba1ef3952..5cc60748e0 100644
--- a/vm.c
+++ b/vm.c
@@ -102,6 +102,13 @@ rb_vm_cframe_keyword_p(const rb_control_frame_t *cfp)
     return VM_FRAME_CFRAME_KW_P(cfp);
 }
 
+/* -- Remove In 3.0 -- */
+int
+rb_vm_cframe_empty_keyword_p(const rb_control_frame_t *cfp)
+{
+    return VM_FRAME_CFRAME_EMPTY_KW_P(cfp);
+}
+
 VALUE
 rb_vm_frame_block_handler(const rb_control_frame_t *cfp)
 {
diff --git a/vm_core.h b/vm_core.h
index fdd34e4fae..52d08fc678 100644
--- a/vm_core.h
+++ b/vm_core.h
@@ -1138,11 +1138,11 @@ typedef rb_control_frame_t *
 
 enum {
     /* Frame/Environment flag bits:
-     *   MMMM MMMM MMMM MMMM ____ _FFF FFFF EEEX (LSB)
+     *   MMMM MMMM MMMM MMMM ____ FFFF FFFF EEEX (LSB)
      *
      * X   : tag for GC marking (It seems as Fixnum)
      * EEE : 3 bits Env flags
-     * FF..: 7 bits Frame flags
+     * FF..: 8 bits Frame flags
      * MM..: 15 bits frame magic (to check frame corruption)
      */
 
@@ -1167,6 +1167,7 @@ enum {
     VM_FRAME_FLAG_LAMBDA    = 0x0100,
     VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM = 0x0200,
     VM_FRAME_FLAG_CFRAME_KW = 0x0400,
+    VM_FRAME_FLAG_CFRAME_EMPTY_KW = 0x0800, /* -- Remove In 3.0 -- */
 
     /* env flag */
     VM_ENV_FLAG_LOCAL       = 0x0002,
@@ -1227,6 +1228,13 @@ VM_FRAME_CFRAME_KW_P(const rb_control_frame_t *cfp)
     return VM_ENV_FLAGS(cfp->ep, VM_FRAME_FLAG_CFRAME_KW) != 0;
 }
 
+/* -- Remove In 3.0 -- */
+static inline int
+VM_FRAME_CFRAME_EMPTY_KW_P(const rb_control_frame_t *cfp)
+{
+    return VM_ENV_FLAGS(cfp->ep, VM_FRAME_FLAG_CFRAME_EMPTY_KW) != 0;
+}
+
 static inline int
 VM_FRAME_FINISHED_P(const rb_control_frame_t *cfp)
 {
@@ -1652,6 +1660,8 @@ void rb_vm_inc_const_missing_count(void);
 void rb_vm_gvl_destroy(rb_vm_t *vm);
 VALUE rb_vm_call(rb_execution_context_t *ec, VALUE recv, VALUE id, int argc,
 		 const VALUE *argv, const rb_callable_method_entry_t *me);
+VALUE rb_vm_call_kw(rb_execution_context_t *ec, VALUE recv, VALUE id, int argc,
+                 const VALUE *argv, const rb_callable_method_entry_t *me, int kw_splat);
 MJIT_STATIC void rb_vm_pop_frame(rb_execution_context_t *ec);
 
 void rb_thread_start_timer_thread(void);
diff --git a/vm_eval.c b/vm_eval.c
index b0249391bf..4a3b9e325f 100644
--- a/vm_eval.c
+++ b/vm_eval.c
@@ -33,6 +33,7 @@ typedef enum call_type {
     CALL_FCALL,
     CALL_VCALL,
     CALL_PUBLIC_KW,
+    CALL_FCALL_KW,
     CALL_TYPE_MAX
 } call_type;
 
@@ -210,6 +211,12 @@ rb_vm_call(rb_execution_context_t *ec, VALUE recv, VALUE id, int argc, const VAL
     return rb_vm_call0(ec, recv, id, argc, argv, me, VM_NO_KEYWORDS);
 }
 
+VALUE
+rb_vm_call_kw(rb_execution_context_t *ec, VALUE recv, VALUE id, int argc, const VALUE *argv, const rb_callable_method_entry_t *me, int kw_splat)
+{
+    return rb_vm_call0(ec, recv, id, argc, argv, me, kw_splat);
+}
+
 static inline VALUE
 vm_call_super(rb_execution_context_t *ec, int argc, const VALUE *argv)
 {
@@ -298,9 +305,17 @@ rb_call0(rb_execution_context_t *ec,
     call_type scope = call_scope;
     int kw_splat = VM_NO_KEYWORDS;
 
-    if (scope == CALL_PUBLIC_KW) {
+    switch(scope) {
+      case(CALL_PUBLIC_KW):
         scope = CALL_PUBLIC;
         kw_splat = 1;
+        break;
+      case(CALL_FCALL_KW):
+        scope = CALL_FCALL;
+        kw_splat = 1;
+        break;
+      default:
+        break;
     }
 
     if (scope == CALL_PUBLIC) {
@@ -861,6 +876,12 @@ rb_funcallv(VALUE recv, ID mid, int argc, const VALUE *argv)
     return rb_call(recv, mid, argc, argv, CALL_FCALL);
 }
 
+VALUE
+rb_funcallv_kw(VALUE recv, ID mid, int argc, const VALUE *argv, int kw_splat)
+{
+    return rb_call(recv, mid, argc, argv, kw_splat ? CALL_FCALL_KW : CALL_FCALL);
+}
+
 /*!
  * Calls a method.
  *
diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index c04c620b44..a83e3a952a 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -2206,8 +2206,9 @@ vm_method_cfunc_entry(const rb_callable_method_entry_t *me)
     return UNALIGNED_MEMBER_PTR(me->def, body.cfunc);
 }
 
+/* -- Remove empty_kw_splat In 3.0 -- */
 static VALUE
-vm_call_cfunc_with_frame(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, struct rb_calling_info *calling, const struct rb_call_info *ci, struct rb_call_cache *cc)
+vm_call_cfunc_with_frame(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, struct rb_calling_info *calling, const struct rb_call_info *ci, struct rb_call_cache *cc, int empty_kw_splat)
 {
     VALUE val;
     const rb_callable_method_entry_t *me = cc->me;
@@ -2223,6 +2224,9 @@ vm_call_cfunc_with_frame(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp
     if (UNLIKELY(calling->kw_splat)) {
         frame_type |= VM_FRAME_FLAG_CFRAME_KW;
     }
+    else if (UNLIKELY(empty_kw_splat)) {
+        frame_type |= VM_FRAME_FLAG_CFRAME_EMPTY_KW;
+    }
 
     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);
@@ -2249,10 +2253,14 @@ vm_call_cfunc_with_frame(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp
 static VALUE
 vm_call_cfunc(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, struct rb_calling_info *calling, const struct rb_call_info *ci, struct rb_call_cache *cc)
 {
+    int empty_kw_splat = calling->kw_splat;
     RB_DEBUG_COUNTER_INC(ccf_cfunc);
 
     CALLER_SETUP_ARG(reg_cfp, calling, ci);
-    return vm_call_cfunc_with_frame(ec, reg_cfp, calling, ci, cc);
+    if (empty_kw_splat && calling->kw_splat) {
+        empty_kw_splat = 0;
+    }
+    return vm_call_cfunc_with_frame(ec, reg_cfp, calling, ci, cc, empty_kw_splat);
 }
 
 static VALUE