Add Module#ruby2_keywords for passing keywords through regular argument splats

This approach uses a flag bit on the final hash object in the regular splat,
as opposed to a previous approach that used a VM frame flag.  The hash flag
approach is less invasive, and handles some cases that the VM frame flag
approach does not, such as saving the argument splat array and splatting it
later:

  ruby2_keywords def foo(*args)
    @args = args
    bar
  end
  def bar
    baz(*@args)
  end
  def baz(*args, **kw)
    [args, kw]
  end
  foo(a:1)    #=> [[], {a: 1}]
  foo({a: 1}, **{}) #=> [[{a: 1}], {}]

  foo({a: 1}) #=> 2.7: [[], {a: 1}] # and warning
  foo({a: 1}) #=> 3.0: [[{a: 1}], {}]

It doesn't handle some cases that the VM frame flag handles, such as when
the final hash object is replaced using Hash#merge, but those cases are
probably less common and are unlikely to properly support keyword
argument separation.

Use ruby2_keywords to handle argument delegation in the delegate library.

1 files changed, 27 insertions, 17 deletions

diff --git a/vm_insnhelper.c b/vm_insnhelper.c
index 56767a4a62..49e865d96f 100644
--- a/vm_insnhelper.c
+++ b/vm_insnhelper.c
@@ -1770,15 +1770,21 @@ rb_iseq_only_kwparam_p(const rb_iseq_t *iseq)
 
 
 static inline void
-CALLER_SETUP_ARG_WITHOUT_KW_SPLAT(struct rb_control_frame_struct *restrict cfp,
-                                  struct rb_calling_info *restrict calling,
-                                  const struct rb_call_info *restrict ci)
+CALLER_SETUP_ARG(struct rb_control_frame_struct *restrict cfp,
+                 struct rb_calling_info *restrict calling,
+                 const struct rb_call_info *restrict ci)
 {
     if (UNLIKELY(IS_ARGS_SPLAT(ci))) {
         /* This expands the rest argument to the stack.
          * So, ci->flag & VM_CALL_ARGS_SPLAT is now inconsistent.
          */
         vm_caller_setup_arg_splat(cfp, calling);
+        if (!IS_ARGS_KW_OR_KW_SPLAT(ci) &&
+                calling->argc > 0 &&
+                RB_TYPE_P(*(cfp->sp - 1), T_HASH) &&
+                (((struct RHash *)*(cfp->sp - 1))->basic.flags & RHASH_PASS_AS_KEYWORDS)) {
+            calling->kw_splat = 1;
+        }
     }
     if (UNLIKELY(IS_ARGS_KEYWORD(ci))) {
         /* This converts VM_CALL_KWARG style to VM_CALL_KW_SPLAT style
@@ -1790,12 +1796,10 @@ CALLER_SETUP_ARG_WITHOUT_KW_SPLAT(struct rb_control_frame_struct *restrict cfp,
 }
 
 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)
+CALLER_REMOVE_EMPTY_KW_SPLAT(struct rb_control_frame_struct *restrict cfp,
+                             struct rb_calling_info *restrict calling,
+                             const struct rb_call_info *restrict ci)
 {
-    CALLER_SETUP_ARG_WITHOUT_KW_SPLAT(cfp, calling, ci);
-
     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.
@@ -1920,6 +1924,7 @@ vm_callee_setup_arg(rb_execution_context_t *ec, struct rb_calling_info *calling,
         if (LIKELY(rb_simple_iseq_p(iseq))) {
             rb_control_frame_t *cfp = ec->cfp;
             CALLER_SETUP_ARG(cfp, calling, ci);
+            CALLER_REMOVE_EMPTY_KW_SPLAT(cfp, calling, ci);
 
             if (calling->argc != iseq->body->param.lead_num) {
                 argument_arity_error(ec, iseq, calling->argc, iseq->body->param.lead_num, iseq->body->param.lead_num);
@@ -1931,6 +1936,7 @@ vm_callee_setup_arg(rb_execution_context_t *ec, struct rb_calling_info *calling,
         else if (rb_iseq_only_optparam_p(iseq)) {
             rb_control_frame_t *cfp = ec->cfp;
             CALLER_SETUP_ARG(cfp, calling, ci);
+            CALLER_REMOVE_EMPTY_KW_SPLAT(cfp, calling, ci);
 
             const int lead_num = iseq->body->param.lead_num;
             const int opt_num = iseq->body->param.opt_num;
@@ -2285,10 +2291,12 @@ 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;
+    int empty_kw_splat;
     RB_DEBUG_COUNTER_INC(ccf_cfunc);
 
     CALLER_SETUP_ARG(reg_cfp, calling, ci);
+    empty_kw_splat = calling->kw_splat;
+    CALLER_REMOVE_EMPTY_KW_SPLAT(reg_cfp, calling, ci);
     if (empty_kw_splat && calling->kw_splat) {
         empty_kw_splat = 0;
     }
@@ -2333,7 +2341,7 @@ vm_call_bmethod(rb_execution_context_t *ec, rb_control_frame_t *cfp, struct rb_c
     VALUE *argv;
     int argc;
 
-    CALLER_SETUP_ARG_WITHOUT_KW_SPLAT(cfp, calling, ci);
+    CALLER_SETUP_ARG(cfp, calling, ci);
     argc = calling->argc;
     argv = ALLOCA_N(VALUE, argc);
     MEMCPY(argv, cfp->sp - argc, VALUE, argc);
@@ -2363,7 +2371,7 @@ vm_call_opt_send(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp, struct
     struct rb_call_info_with_kwarg ci_entry;
     struct rb_call_cache cc_entry, *cc;
 
-    CALLER_SETUP_ARG_WITHOUT_KW_SPLAT(reg_cfp, calling, orig_ci);
+    CALLER_SETUP_ARG(reg_cfp, calling, orig_ci);
 
     i = calling->argc - 1;
 
@@ -2468,7 +2476,7 @@ vm_call_method_missing(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp,
     struct rb_call_cache cc_entry, *cc;
     unsigned int argc;
 
-    CALLER_SETUP_ARG_WITHOUT_KW_SPLAT(reg_cfp, calling, orig_ci);
+    CALLER_SETUP_ARG(reg_cfp, calling, orig_ci);
     argc = calling->argc+1;
 
     ci_entry.flag = VM_CALL_FCALL | VM_CALL_OPT_SEND | (calling->kw_splat ? VM_CALL_KW_SPLAT : 0);
@@ -2673,12 +2681,12 @@ vm_call_method_each_type(rb_execution_context_t *ec, rb_control_frame_t *cfp, st
 	return vm_call_cfunc(ec, cfp, calling, ci, cc);
 
       case VM_METHOD_TYPE_ATTRSET:
+        CALLER_SETUP_ARG(cfp, calling, ci);
         if (calling->argc == 1 && calling->kw_splat && RHASH_EMPTY_P(cfp->sp[-1])) {
-            CALLER_SETUP_ARG_WITHOUT_KW_SPLAT(cfp, calling, ci);
             rb_warn_keyword_to_last_hash(calling, ci, NULL);
         }
         else {
-            CALLER_SETUP_ARG(cfp, calling, ci);
+            CALLER_REMOVE_EMPTY_KW_SPLAT(cfp, calling, ci);
         }
 
 	rb_check_arity(calling->argc, 1, 1);
@@ -2688,6 +2696,7 @@ vm_call_method_each_type(rb_execution_context_t *ec, rb_control_frame_t *cfp, st
 
       case VM_METHOD_TYPE_IVAR:
         CALLER_SETUP_ARG(cfp, calling, ci);
+        CALLER_REMOVE_EMPTY_KW_SPLAT(cfp, calling, ci);
 	rb_check_arity(calling->argc, 0, 0);
 	cc->aux.index = 0;
         CC_SET_FASTPATH(cc, vm_call_ivar, !(ci->flag & VM_CALL_ARGS_SPLAT));
@@ -2998,12 +3007,12 @@ vm_callee_setup_block_arg(rb_execution_context_t *ec, struct rb_calling_info *ca
 	rb_control_frame_t *cfp = ec->cfp;
 	VALUE arg0;
 
+        CALLER_SETUP_ARG(cfp, calling, ci);
         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);
+            CALLER_REMOVE_EMPTY_KW_SPLAT(cfp, calling, ci);
         }
 
 	if (arg_setup_type == arg_setup_block &&
@@ -3088,7 +3097,7 @@ vm_invoke_symbol_block(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp,
 {
     VALUE val;
     int argc;
-    CALLER_SETUP_ARG_WITHOUT_KW_SPLAT(ec->cfp, calling, ci);
+    CALLER_SETUP_ARG(ec->cfp, calling, ci);
     argc = calling->argc;
     val = vm_yield_with_symbol(ec, symbol, argc, STACK_ADDR_FROM_TOP(argc), calling->kw_splat, calling->block_handler);
     POPN(argc);
@@ -3104,6 +3113,7 @@ vm_invoke_ifunc_block(rb_execution_context_t *ec, rb_control_frame_t *reg_cfp,
     int argc;
     int kw_splat = calling->kw_splat;
     CALLER_SETUP_ARG(ec->cfp, calling, ci);
+    CALLER_REMOVE_EMPTY_KW_SPLAT(ec->cfp, calling, ci);
     if (kw_splat && !calling->kw_splat) {
         kw_splat = 2;
     }