Fix keyword argument separation issues with sym procs when using refinements

Make sure that vm_yield_with_cfunc can correctly set the empty keyword
flag by passing 2 as the kw_splat value when calling it in
vm_invoke_ifunc_block.  Make sure calling.kw_splat is set to 1 and not
128 in vm_sendish, so we can safely check for different kw_splat values.

vm_args.c needs to call add_empty_keyword, and to make JIT happy, the
function needs to be exported.  Rename the function to
rb_adjust_argv_kw_splat to more accurately reflect what it does, and
mark it as MJIT exported.

test/ruby/test_keyword.rb

@@ -2731,3 +2731,306 @@ class TestKeywordArguments < Test::Unit::TestCase
     assert_valid_syntax("bug15087(**{}, &nil)")
   end
 end
+
+class TestKeywordArgumentsSymProcRefinements < Test::Unit::TestCase
+  class C
+    def call(*args, **kw)
+      yield(self, *args, **kw)
+    end
+  end
+  using(Module.new do
+    refine C do
+      def m(*args, **kw)
+        super
+      end
+    end
+  end)
+
+  def test_sym_proc_refine_kwsplat
+    kw = {}
+    h = {:a=>1}
+    h2 = {'a'=>1}
+    h3 = {'a'=>1, :a=>1}
+
+    c = C.new
+    def c.m(*args)
+      args
+    end
+    assert_equal([], c.call(**{}, &:m))
+    assert_equal([], c.call(**kw, &:m))
+    assert_equal([h], c.call(**h, &:m))
+    assert_equal([h], c.call(h, **{}, &:m))
+    assert_equal([h], c.call(a: 1, &:m))
+    assert_equal([h2], c.call(**h2, &:m))
+    assert_equal([h3], c.call(**h3, &:m))
+    assert_equal([h3], c.call(a: 1, **h2, &:m))
+
+    c.singleton_class.remove_method(:m)
+    def c.m; end
+    assert_nil(c.call(**{}, &:m))
+    assert_nil(c.call(**kw, &:m))
+    assert_raise(ArgumentError) { c.call(**h, &:m) }
+    assert_raise(ArgumentError) { c.call(a: 1, &:m) }
+    assert_raise(ArgumentError) { c.call(**h2, &:m) }
+    assert_raise(ArgumentError) { c.call(**h3, &:m) }
+    assert_raise(ArgumentError) { c.call(a: 1, **h2, &:m) }
+
+    c.singleton_class.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, c.call(**{}, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal(kw, c.call(**kw, &:m))
+    end
+    assert_equal(h, c.call(**h, &:m))
+    assert_equal(h, c.call(a: 1, &:m))
+    assert_equal(h2, c.call(**h2, &:m))
+    assert_equal(h3, c.call(**h3, &:m))
+    assert_equal(h3, c.call(a: 1, **h2, &:m))
+
+    c.singleton_class.remove_method(:m)
+    def c.m(**args)
+      args
+    end
+    assert_equal(kw, c.call(**{}, &:m))
+    assert_equal(kw, c.call(**kw, &:m))
+    assert_equal(h, c.call(**h, &:m))
+    assert_equal(h, c.call(a: 1, &:m))
+    assert_equal(h2, c.call(**h2, &:m))
+    assert_equal(h3, c.call(**h3, &:m))
+    assert_equal(h3, c.call(a: 1, **h2, &:m))
+
+    c.singleton_class.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], c.call(**{}, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([kw, kw], c.call(**kw, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([h, kw], c.call(**h, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([h, kw], c.call(a: 1, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([h2, kw], c.call(**h2, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([h3, kw], c.call(**h3, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `m'/m) do
+      assert_equal([h3, kw], c.call(a: 1, **h2, &:m))
+    end
+
+    c.singleton_class.remove_method(:m)
+    def c.m(arg=1, **args)
+      [arg=1, args]
+    end
+    assert_equal([1, kw], c.call(**{}, &:m))
+    assert_equal([1, kw], c.call(**kw, &:m))
+    assert_equal([1, h], c.call(**h, &:m))
+    assert_equal([1, h], c.call(a: 1, &:m))
+    assert_equal([1, h2], c.call(**h2, &:m))
+    assert_equal([1, h3], c.call(**h3, &:m))
+    assert_equal([1, h3], c.call(a: 1, **h2, &:m))
+  end
+
+  def test_sym_proc_refine_super_method_missing_kwsplat
+    kw = {}
+    h = {:a=>1}
+    h2 = {'a'=>1}
+    h3 = {'a'=>1, :a=>1}
+
+    c = C.new
+    def c.method_missing(_, *args)
+      args
+    end
+    assert_equal([], c.call(**{}, &:m))
+    assert_equal([], c.call(**kw, &:m))
+    assert_equal([h], c.call(**h, &:m))
+    assert_equal([h], c.call(h, **{}, &:m))
+    assert_equal([h], c.call(a: 1, &:m))
+    assert_equal([h2], c.call(**h2, &:m))
+    assert_equal([h3], c.call(**h3, &:m))
+    assert_equal([h3], c.call(a: 1, **h2, &:m))
+
+    c.singleton_class.remove_method(:method_missing)
+    def c.method_missing(_) end
+    assert_nil(c.call(**{}, &:m))
+    assert_nil(c.call(**kw, &:m))
+    assert_raise(ArgumentError) { c.call(**h, &:m) }
+    assert_raise(ArgumentError) { c.call(a: 1, &:m) }
+    assert_raise(ArgumentError) { c.call(**h2, &:m) }
+    assert_raise(ArgumentError) { c.call(**h3, &:m) }
+    assert_raise(ArgumentError) { c.call(a: 1, **h2, &:m) }
+
+    c.singleton_class.remove_method(:method_missing)
+    def c.method_missing(_, args)
+      args
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal(kw, c.call(**{}, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal(kw, c.call(**kw, &:m))
+    end
+    assert_equal(h, c.call(**h, &:m))
+    assert_equal(h, c.call(a: 1, &:m))
+    assert_equal(h2, c.call(**h2, &:m))
+    assert_equal(h3, c.call(**h3, &:m))
+    assert_equal(h3, c.call(a: 1, **h2, &:m))
+
+    c.singleton_class.remove_method(:method_missing)
+    def c.method_missing(_, **args)
+      args
+    end
+    assert_equal(kw, c.call(**{}, &:m))
+    assert_equal(kw, c.call(**kw, &:m))
+    assert_equal(h, c.call(**h, &:m))
+    assert_equal(h, c.call(a: 1, &:m))
+    assert_equal(h2, c.call(**h2, &:m))
+    assert_equal(h3, c.call(**h3, &:m))
+    assert_equal(h3, c.call(a: 1, **h2, &:m))
+
+    c.singleton_class.remove_method(:method_missing)
+    def c.method_missing(_, arg, **args)
+      [arg, args]
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([kw, kw], c.call(**{}, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([kw, kw], c.call(**kw, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([h, kw], c.call(**h, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([h, kw], c.call(a: 1, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([h2, kw], c.call(**h2, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([h3, kw], c.call(**h3, &:m))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([h3, kw], c.call(a: 1, **h2, &:m))
+    end
+
+    c.singleton_class.remove_method(:method_missing)
+    def c.method_missing(_, arg=1, **args)
+      [arg=1, args]
+    end
+    assert_equal([1, kw], c.call(**{}, &:m))
+    assert_equal([1, kw], c.call(**kw, &:m))
+    assert_equal([1, h], c.call(**h, &:m))
+    assert_equal([1, h], c.call(a: 1, &:m))
+    assert_equal([1, h2], c.call(**h2, &:m))
+    assert_equal([1, h3], c.call(**h3, &:m))
+    assert_equal([1, h3], c.call(a: 1, **h2, &:m))
+  end
+
+  def test_sym_proc_refine_method_missing_kwsplat
+    kw = {}
+    h = {:a=>1}
+    h2 = {'a'=>1}
+    h3 = {'a'=>1, :a=>1}
+
+    c = C.new
+    def c.method_missing(_, *args)
+      args
+    end
+    assert_equal([], c.call(**{}, &:m2))
+    assert_equal([], c.call(**kw, &:m2))
+    assert_equal([h], c.call(**h, &:m2))
+    assert_equal([h], c.call(h, **{}, &:m2))
+    assert_equal([h], c.call(a: 1, &:m2))
+    assert_equal([h2], c.call(**h2, &:m2))
+    assert_equal([h3], c.call(**h3, &:m2))
+    assert_equal([h3], c.call(a: 1, **h2, &:m2))
+
+    c.singleton_class.remove_method(:method_missing)
+    def c.method_missing(_) end
+    assert_nil(c.call(**{}, &:m2))
+    assert_nil(c.call(**kw, &:m2))
+    assert_raise(ArgumentError) { c.call(**h, &:m2) }
+    assert_raise(ArgumentError) { c.call(a: 1, &:m2) }
+    assert_raise(ArgumentError) { c.call(**h2, &:m2) }
+    assert_raise(ArgumentError) { c.call(**h3, &:m2) }
+    assert_raise(ArgumentError) { c.call(a: 1, **h2, &:m2) }
+
+    c.singleton_class.remove_method(:method_missing)
+    def c.method_missing(_, args)
+      args
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal(kw, c.call(**{}, &:m2))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal(kw, c.call(**kw, &:m2))
+    end
+    assert_equal(h, c.call(**h, &:m2))
+    assert_equal(h, c.call(a: 1, &:m2))
+    assert_equal(h2, c.call(**h2, &:m2))
+    assert_equal(h3, c.call(**h3, &:m2))
+    assert_equal(h3, c.call(a: 1, **h2, &:m2))
+
+    c.singleton_class.remove_method(:method_missing)
+    def c.method_missing(_, **args)
+      args
+    end
+    assert_equal(kw, c.call(**{}, &:m2))
+    assert_equal(kw, c.call(**kw, &:m2))
+    assert_equal(h, c.call(**h, &:m2))
+    assert_equal(h, c.call(a: 1, &:m2))
+    assert_equal(h2, c.call(**h2, &:m2))
+    assert_equal(h3, c.call(**h3, &:m2))
+    assert_equal(h3, c.call(a: 1, **h2, &:m2))
+
+    c.singleton_class.remove_method(:method_missing)
+    def c.method_missing(_, arg, **args)
+      [arg, args]
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([kw, kw], c.call(**{}, &:m2))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([kw, kw], c.call(**kw, &:m2))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([h, kw], c.call(**h, &:m2))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([h, kw], c.call(a: 1, &:m2))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([h2, kw], c.call(**h2, &:m2))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([h3, kw], c.call(**h3, &:m2))
+    end
+    assert_warn(/The keyword argument is passed as the last hash parameter.* for `method_missing'/m) do
+      assert_equal([h3, kw], c.call(a: 1, **h2, &:m2))
+    end
+
+    c.singleton_class.remove_method(:method_missing)
+    def c.method_missing(_, arg=1, **args)
+      [arg=1, args]
+    end
+    assert_equal([1, kw], c.call(**{}, &:m2))
+    assert_equal([1, kw], c.call(**kw, &:m2))
+    assert_equal([1, h], c.call(**h, &:m2))
+    assert_equal([1, h], c.call(a: 1, &:m2))
+    assert_equal([1, h2], c.call(**h2, &:m2))
+    assert_equal([1, h3], c.call(**h3, &:m2))
+    assert_equal([1, h3], c.call(a: 1, **h2, &:m2))
+  end
+end

vm_args.c

@@ -14,6 +14,7 @@ NORETURN(static void argument_kw_error(rb_execution_context_t *ec, const rb_iseq
 VALUE rb_keyword_error_new(const char *error, VALUE keys); /* class.c */
 static VALUE method_missing(VALUE obj, ID id, int argc, const VALUE *argv,
                             enum method_missing_reason call_status, int kw_splat);
+extern VALUE rb_adjust_argv_kw_splat(int *argc, const VALUE **argv, int *kw_splat);
 
 struct args_info {
     /* basic args info */
@@ -1051,6 +1052,9 @@ refine_sym_proc_call(RB_BLOCK_CALL_FUNC_ARGLIST(yielded_arg, callback_arg))
     rb_execution_context_t *ec;
     const VALUE symbol = RARRAY_AREF(callback_arg, 0);
     const VALUE refinements = RARRAY_AREF(callback_arg, 1);
+    int kw_splat = RB_PASS_CALLED_KEYWORDS;
+    VALUE v;
+    VALUE ret;
     VALUE klass;
 
     if (argc-- < 1) {
@@ -1071,10 +1075,15 @@ refine_sym_proc_call(RB_BLOCK_CALL_FUNC_ARGLIST(yielded_arg, callback_arg))
     if (!NIL_P(blockarg)) {
 	vm_passed_block_handler_set(ec, blockarg);
     }
+    v = rb_adjust_argv_kw_splat(&argc, &argv, &kw_splat);
     if (!me) {
-        return method_missing(obj, mid, argc, argv, MISSING_NOENTRY, VM_NO_KEYWORDS);
+        ret = method_missing(obj, mid, argc, argv, MISSING_NOENTRY, kw_splat);
     }
-    return rb_vm_call0(ec, obj, mid, argc, argv, me, VM_NO_KEYWORDS);
+    else {
+        ret = rb_vm_call0(ec, obj, mid, argc, argv, me, kw_splat);
+    }
+    rb_free_tmp_buffer(&v);
+    return ret;
 }
 
 static VALUE

vm_eval.c

@@ -236,8 +236,8 @@ vm_call0_body(rb_execution_context_t *ec, struct rb_calling_info *calling, const
 }
 
 /* Caller should keep the reference to the return value until argv becomes useless. */
-static VALUE
-add_empty_keyword(int *argc, const VALUE **argv, int *kw_splat)
+MJIT_FUNC_EXPORTED VALUE
+rb_adjust_argv_kw_splat(int *argc, const VALUE **argv, int *kw_splat)
 {
     if (*kw_splat == RB_PASS_CALLED_KEYWORDS || *kw_splat == RB_PASS_EMPTY_KEYWORDS) {
         if (*kw_splat == RB_PASS_EMPTY_KEYWORDS || rb_empty_keyword_given_p()) {
@@ -273,7 +273,7 @@ rb_vm_call(rb_execution_context_t *ec, VALUE recv, VALUE id, int argc, const VAL
 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)
 {
-    VALUE v = add_empty_keyword(&argc, &argv, &kw_splat);
+    VALUE v = rb_adjust_argv_kw_splat(&argc, &argv, &kw_splat);
     VALUE ret = rb_vm_call0(ec, recv, id, argc, argv, me, kw_splat);
     rb_free_tmp_buffer(&v);
     return ret;
@@ -298,7 +298,7 @@ vm_call_super(rb_execution_context_t *ec, int argc, const VALUE *argv, int kw_sp
     id = me->def->original_id;
     me = rb_callable_method_entry(klass, id);
 
-    v = add_empty_keyword(&argc, &argv, &kw_splat);
+    v = rb_adjust_argv_kw_splat(&argc, &argv, &kw_splat);
     if (!me) {
         ret = method_missing(recv, id, argc, argv, MISSING_SUPER, kw_splat);
     }
@@ -953,7 +953,7 @@ rb_funcallv(VALUE recv, ID mid, int argc, const VALUE *argv)
 VALUE
 rb_funcallv_kw(VALUE recv, ID mid, int argc, const VALUE *argv, int kw_splat)
 {
-    VALUE v = add_empty_keyword(&argc, &argv, &kw_splat);
+    VALUE v = rb_adjust_argv_kw_splat(&argc, &argv, &kw_splat);
     VALUE ret = rb_call(recv, mid, argc, argv, kw_splat ? CALL_FCALL_KW : CALL_FCALL);
     rb_free_tmp_buffer(&v);
     return ret;
@@ -1023,7 +1023,7 @@ rb_funcall_with_block_kw(VALUE recv, ID mid, int argc, const VALUE *argv, VALUE
         vm_passed_block_handler_set(GET_EC(), passed_procval);
     }
 
-    VALUE v = add_empty_keyword(&argc, &argv, &kw_splat);
+    VALUE v = rb_adjust_argv_kw_splat(&argc, &argv, &kw_splat);
     VALUE ret = rb_call(recv, mid, argc, argv, kw_splat ? CALL_PUBLIC_KW : CALL_PUBLIC);
     rb_free_tmp_buffer(&v);
     return ret;
@@ -1390,7 +1390,7 @@ rb_block_call_kw(VALUE obj, ID mid, int argc, const VALUE * argv,
 {
     struct iter_method_arg arg;
 
-    VALUE v = add_empty_keyword(&argc, &argv, &kw_splat);
+    VALUE v = rb_adjust_argv_kw_splat(&argc, &argv, &kw_splat);
     arg.obj = obj;
     arg.mid = mid;
     arg.argc = argc;

vm_insnhelper.c

@@ -2888,6 +2888,7 @@ vm_yield_with_cfunc(rb_execution_context_t *ec,
 {
     int is_lambda = FALSE; /* TODO */
     VALUE val, arg, blockarg;
+    int frame_flag;
     const struct vm_ifunc *ifunc = captured->code.ifunc;
 
     if (is_lambda) {
@@ -2902,9 +2903,18 @@ vm_yield_with_cfunc(rb_execution_context_t *ec,
 
     blockarg = rb_vm_bh_to_procval(ec, block_handler);
 
+    frame_flag = VM_FRAME_MAGIC_IFUNC | VM_FRAME_FLAG_CFRAME | (me ? VM_FRAME_FLAG_BMETHOD : 0);
+    switch(kw_splat) {
+        case 1:
+            frame_flag |= VM_FRAME_FLAG_CFRAME_KW;
+            break;
+        case 2:
+            frame_flag |= VM_FRAME_FLAG_CFRAME_EMPTY_KW;
+            break;
+    }
+
     vm_push_frame(ec, (const rb_iseq_t *)captured->code.ifunc,
-                  VM_FRAME_MAGIC_IFUNC | VM_FRAME_FLAG_CFRAME |
-                  (me ? VM_FRAME_FLAG_BMETHOD : 0) | (kw_splat ? VM_FRAME_FLAG_CFRAME_KW : 0),
+                  frame_flag,
 		  self,
 		  VM_GUARDED_PREV_EP(captured->ep),
                   (VALUE)me,
@@ -3060,9 +3070,17 @@ vm_invoke_ifunc_block(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp,
 {
     VALUE val;
     int argc;
+    int frame_flag = 0;
+    int kw_splat = calling->kw_splat;
     CALLER_SETUP_ARG(ec->cfp, calling, ci);
+    if (kw_splat && !calling->kw_splat) {
+        kw_splat = 2;
+    }
+    else {
+        kw_splat = calling->kw_splat;
+    }
     argc = calling->argc;
-    val = vm_yield_with_cfunc(ec, captured, captured->self, argc, STACK_ADDR_FROM_TOP(argc), calling->kw_splat, calling->block_handler, NULL);
+    val = vm_yield_with_cfunc(ec, captured, captured->self, argc, STACK_ADDR_FROM_TOP(argc), kw_splat, calling->block_handler, NULL);
     POPN(argc); /* TODO: should put before C/yield? */
     return val;
 }
@@ -3695,7 +3713,7 @@ vm_sendish(
     struct rb_calling_info calling;
 
     calling.block_handler = block_handler;
-    calling.kw_splat = IS_ARGS_KW_SPLAT(ci);
+    calling.kw_splat = IS_ARGS_KW_SPLAT(ci) > 0;
     calling.recv = recv;
     calling.argc = argc;