Make rb_scan_args handle keywords more similar to Ruby methods

Cfuncs that use rb_scan_args with the : entry suffer similar keyword
argument separation issues that Ruby methods suffer if the cfuncs
accept optional or variable arguments.

This makes the following changes to : handling.

* If optional or variable arguments are also requested, treat
  the option handling similar to Ruby keyword option handling
  (keyword mode).  Otherwise, if only mandatory arguments are
  requested other than this option hash, treat it similarly to
  an optional argument (hash mode).

* In keyword mode, do not look for an option hash if empty
  keywords are provided.  For backwards compatibility, in both modes,
  treat an empty keyword splat as a empty mandatory positional hash
  argument, but emit a a warning, as this behavior should be removed
  in Ruby 3.  The argument number check needs to be moved lower so it
  can correctly handle an empty positional argument being added.

* In both modes, if the last argument is nil and it is necessary to
  treat it as an option hash in order to make sure all arguments are
  processed, continue to treat the last argument as the option hash.
  Emit a warning in this case, as this behavior should be removed
  in Ruby 3.

* In both modes, if splitting the option hash into two hashes, issue
  a warning, as we will not be splitting hashes in Ruby 3.

* In keyword mode, if keyword arguments are not provided and the
  last argument is a hash, emit a warning, as in Ruby 3 it will be
  treated as a positional argument instead of the option hash.

* In keyword mode, if the keyword argument is required to fill a
  mandatory positional argument, continue to do so, but emit a
  warning as this behavior will be going away in Ruby 3.

* In keyword mode, if keyword arguments are provided an the last
  argument is not a hash, that indicates something wrong. This can
  happen if a cfunc is calling rb_scan_args multiple times, and
  providing arguments that were not passed to it from Ruby.  Such
  functions will need to be updated, and there is a new feature
  to handle this.  Instead of calling rb_keyword_given_p() and
  rb_empty_keyword_given_p(), allow an 'k', 'e', or 'n' prefix
  to the rb_scan_args string:

  * k: Treat as if the keyword given flag is set (last argument
    should be a hash.

  * e: Treat as if the empty keyword given flag is set (in
    keyword mode, do not look for a last positional hash).

  * n: In keyword mode, assume the call did not set the keyword
    or empty keyword flags, but do not issue a warning if the
    last argument is a hash that is treated as keywords.

  For external Cfuncs, you can be backwards compatible with Ruby
  <2.7 by using a macro. On 2.7+, the macro can be defined to
  "k", "e", or "n", and on <2.7, it can be defined to "";

This commit fixes all warnings caused by the changes above.

It switches some function calls to *_kw versions with appropriate
kw_splat flags. If delegating arguments, RB_PASS_CALLED_KEYWORDS
is used.  If creating new arguments, RB_PASS_KEYWORDS is used if
the last argument is a hash to be treated as keywords.

In open_key_args in io.c, use the 'n' prefix to rb_scan_args.
In this case, the arguments provided come from another C
function, not Ruby.  The last argument may or may not be a hash,
so we can't set keyword argument mode.  However, if it is a
hash, we don't want to warn when treating it as keywords.

In Ruby files, make sure to appropriately use keyword splats
or literal keywords when calling Cfuncs that now issue keyword
argument separation warnings through rb_scan_args.  Also, make
sure not to pass nil in place of an option hash.

Work around Kernel#warn warnings due to problems in the Rubygems
override of the method.  There is an open pull request to fix
these issues in Rubygems, but part of the Rubygems tests for
their override fail on ruby-head due to rb_scan_args not
recognizing empty keyword splats, which this commit fixes.

class.c

@@ -1954,7 +1954,30 @@ rb_scan_args(int argc, const VALUE *argv, const char *fmt, ...)
     int f_var = 0, f_hash = 0, f_block = 0;
     int n_lead = 0, n_opt = 0, n_trail = 0, n_mand;
     int argi = 0, last_idx = -1;
+    int f_kw = 0;
+    int keyword_given = 0;
+    int empty_keyword_given = 0;
+    int allow_hash_to_keyword = 0;
     VALUE hash = Qnil, last_hash = 0;
+    VALUE tmp_buffer = 0;
+
+    if (*p == 'k') {
+        keyword_given = 1;
+        p++;
+    }
+    else if (*p == 'e') {
+        empty_keyword_given = 1;
+        p++;
+    }
+    else if (*p == 'n') {
+        allow_hash_to_keyword = 1;
+        p++;
+    }
+    else {
+        if(!(keyword_given = rb_keyword_given_p())) {
+            empty_keyword_given = rb_empty_keyword_given_p();
+        }
+    }
 
     if (ISDIGIT(*p)) {
 	n_lead = *p - '0';
@@ -1974,6 +1997,9 @@ rb_scan_args(int argc, const VALUE *argv, const char *fmt, ...)
     }
     if (*p == ':') {
 	f_hash = 1;
+        /* Optional or variable arguments turn on keyword argument mode */
+        /* If all arguments are mandatory, operate in regular argument mode */
+        f_kw = f_var || n_opt;
 	p++;
     }
     if (*p == '&') {
@@ -1985,32 +2011,79 @@ rb_scan_args(int argc, const VALUE *argv, const char *fmt, ...)
     }
     n_mand = n_lead + n_trail;
 
-    if (argc < n_mand)
-	goto argc_error;
-
     va_start(vargs, fmt);
 
     /* capture an option hash - phase 1: pop */
-    if (f_hash && n_mand < argc) {
-	VALUE last = argv[argc - 1];
-
-	if (NIL_P(last)) {
-	    /* nil is taken as an empty option hash only if it is not
-	       ambiguous; i.e. '*' is not specified and arguments are
-	       given more than sufficient */
-	    if (!f_var && n_mand + n_opt < argc)
-		argc--;
-	}
-	else {
-	    hash = rb_check_hash_type(last);
-	    if (!NIL_P(hash)) {
-		VALUE opts = rb_extract_keywords(&hash);
-		if (!(last_hash = hash)) argc--;
-		else last_idx = argc - 1;
-		hash = opts ? opts : Qnil;
-	    }
-	}
+    /* In keyword arugment mode, ignore final positional hash if empty keywords given */
+    if (argc > 0 && !(f_kw && empty_keyword_given)) {
+        VALUE last = argv[argc - 1];
+
+        /* Ruby 3: if (f_hash && (f_kw && keyword_given) || n_mand < argc)) { */
+        if (f_hash && n_mand < argc) {
+            if (f_kw && keyword_given) {
+                if (!RB_TYPE_P(last, T_HASH)) {
+                    rb_warn("Keyword flag set when calling rb_scan_args, but last entry is not a hash");
+                }
+                else {
+                    hash = last;
+                }
+            }
+            else if (NIL_P(last)) {
+                /* For backwards compatibility, nil is taken as an empty
+                   option hash only if it is not ambiguous; i.e. '*' is
+                   not specified and arguments are given more than sufficient.
+                   This will be removed in Ruby 3. */
+                if (!f_var && n_mand + n_opt < argc) {
+                    rb_warn("The last argument is nil, treating as empty keywords");
+                    argc--;
+                }
+            }
+            else {
+                hash = rb_check_hash_type(last);
+            }
+
+            /* Ruby 3: Remove if branch, as it will not attempt to split hashes */
+            if (!NIL_P(hash)) {
+                VALUE opts = rb_extract_keywords(&hash);
+
+                if (!(last_hash = hash)) {
+                    if (f_kw && !keyword_given && !allow_hash_to_keyword) {
+                        /* Warn in keyword argument mode if treating positional
+                           as keyword, as in Ruby 3, this will be an error */
+                        rb_warn("The last argument is used as the keyword parameter");
+                    }
+                    argc--;
+                }
+                else {
+                    /* Warn in keyword argument mode if splitting either positional hash
+                       to keywords or keywords to positional hash, as in Ruby 3,
+                       no splitting will be done */
+                    rb_warn("The last argument is split into positional and keyword parameters");
+                    last_idx = argc - 1;
+                }
+                hash = opts ? opts : Qnil;
+            }
+        }
+        else if (f_kw && keyword_given && n_mand == argc) {
+            /* Warn in keyword argument mode if treating keywords as positional,
+               as in Ruby 3, this will be an error */
+            rb_warn("The keyword argument is passed as the last hash parameter");
+        }
     }
+    if (f_hash && n_mand == argc+1 && empty_keyword_given) {
+        VALUE *ptr = rb_alloc_tmp_buffer2(&tmp_buffer, argc+1, sizeof(VALUE));
+        memcpy(ptr, argv, sizeof(VALUE)*argc);
+        ptr[argc] = rb_hash_new();
+        argc++;
+        *(&argv) = ptr;
+        rb_warn("The keyword argument is passed as the last hash parameter");
+    }
+
+    if (argc < n_mand) {
+        va_end(vargs);
+        goto argc_error;
+    }
+
     /* capture leading mandatory arguments */
     for (i = n_lead; i-- > 0; ) {
 	var = va_arg(vargs, VALUE *);
@@ -2070,9 +2143,11 @@ rb_scan_args(int argc, const VALUE *argv, const char *fmt, ...)
 
     if (argi < argc) {
       argc_error:
+        if(tmp_buffer) rb_free_tmp_buffer(&tmp_buffer);
 	rb_error_arity(argc, n_mand, f_var ? UNLIMITED_ARGUMENTS : n_mand + n_opt);
     }
 
+    if(tmp_buffer) rb_free_tmp_buffer(&tmp_buffer);
     return argc;
 }
 

error.c

@@ -341,7 +341,7 @@ rb_warn_m(int argc, VALUE *argv, VALUE exc)
     VALUE opts, location = Qnil;
 
     if (!NIL_P(ruby_verbose) && argc > 0 &&
-	    (argc = rb_scan_args(argc, argv, "*:", NULL, &opts)) > 0) {
+            (argc = rb_scan_args(argc, argv, "*:", NULL, &opts)) > 0) {
 	VALUE str = argv[0], uplevel = Qnil;
 	if (!NIL_P(opts)) {
 	    static ID kwds[1];
@@ -1595,7 +1595,7 @@ nometh_err_initialize(int argc, VALUE *argv, VALUE self)
     priv = (argc > 3) && (--argc, RTEST(argv[argc]));
     args = (argc > 2) ? argv[--argc] : Qnil;
     if (!NIL_P(options)) argv[argc++] = options;
-    rb_call_super(argc, argv);
+    rb_call_super_kw(argc, argv, RB_PASS_CALLED_KEYWORDS);
     return nometh_err_init_attr(self, args, priv);
 }
 

ext/-test-/scan_args/scan_args.c

@@ -250,6 +250,33 @@ scan_args_lead_opt_var_trail_hash(int argc, VALUE *argv, VALUE self)
     return rb_ary_new_from_values(numberof(args), args);
 }
 
+static VALUE
+scan_args_k_lead_opt_hash(int argc, VALUE *argv, VALUE self)
+{
+    VALUE args[4];
+    int n = rb_scan_args(argc, argv, "k11:", args+1, args+2, args+3);
+    args[0] = INT2NUM(n);
+    return rb_ary_new_from_values(numberof(args), args);
+}
+
+static VALUE
+scan_args_e_lead_opt_hash(int argc, VALUE *argv, VALUE self)
+{
+    VALUE args[4];
+    int n = rb_scan_args(argc, argv, "e11:", args+1, args+2, args+3);
+    args[0] = INT2NUM(n);
+    return rb_ary_new_from_values(numberof(args), args);
+}
+
+static VALUE
+scan_args_n_lead_opt_hash(int argc, VALUE *argv, VALUE self)
+{
+    VALUE args[4];
+    int n = rb_scan_args(argc, argv, "n11:", args+1, args+2, args+3);
+    args[0] = INT2NUM(n);
+    return rb_ary_new_from_values(numberof(args), args);
+}
+
 void
 Init_scan_args(void)
 {
@@ -282,5 +309,7 @@ Init_scan_args(void)
     rb_define_singleton_method(module, "lead_var_trail_hash", scan_args_lead_var_trail_hash, -1);
     rb_define_singleton_method(module, "opt_var_trail_hash", scan_args_opt_var_trail_hash, -1);
     rb_define_singleton_method(module, "lead_opt_var_trail_hash", scan_args_lead_opt_var_trail_hash, -1);
+    rb_define_singleton_method(module, "k_lead_opt_hash", scan_args_k_lead_opt_hash, -1);
+    rb_define_singleton_method(module, "e_lead_opt_hash", scan_args_e_lead_opt_hash, -1);
+    rb_define_singleton_method(module, "n_lead_opt_hash", scan_args_n_lead_opt_hash, -1);
 }
-

ext/pathname/pathname.c

@@ -393,7 +393,7 @@ path_read(int argc, VALUE *argv, VALUE self)
 
     args[0] = get_strpath(self);
     n = rb_scan_args(argc, argv, "03", &args[1], &args[2], &args[3]);
-    return rb_funcallv(rb_cFile, id_read, 1+n, args);
+    return rb_funcallv_kw(rb_cFile, id_read, 1+n, args, RB_PASS_CALLED_KEYWORDS);
 }
 
 /*
@@ -1097,12 +1097,12 @@ path_s_glob(int argc, VALUE *argv, VALUE klass)
 
     n = rb_scan_args(argc, argv, "12", &args[0], &args[1], &args[2]);
     if (rb_block_given_p()) {
-        return rb_block_call(rb_cDir, id_glob, n, args, s_glob_i, klass);
+        return rb_block_call_kw(rb_cDir, id_glob, n, args, s_glob_i, klass, RB_PASS_CALLED_KEYWORDS);
     }
     else {
         VALUE ary;
         long i;
-        ary = rb_funcallv(rb_cDir, id_glob, n, args);
+        ary = rb_funcallv_kw(rb_cDir, id_glob, n, args, RB_PASS_CALLED_KEYWORDS);
         ary = rb_convert_type(ary, T_ARRAY, "Array", "to_ary");
         for (i = 0; i < RARRAY_LEN(ary); i++) {
             VALUE elt = RARRAY_AREF(ary, i);
@@ -1145,12 +1145,12 @@ path_glob(int argc, VALUE *argv, VALUE self)
     n = 3;
 
     if (rb_block_given_p()) {
-        return rb_block_call(rb_cDir, id_glob, n, args, glob_i, self);
+        return rb_block_call_kw(rb_cDir, id_glob, n, args, glob_i, self, RB_PASS_KEYWORDS);
     }
     else {
         VALUE ary;
         long i;
-        ary = rb_funcallv(rb_cDir, id_glob, n, args);
+        ary = rb_funcallv_kw(rb_cDir, id_glob, n, args, RB_PASS_KEYWORDS);
         ary = rb_convert_type(ary, T_ARRAY, "Array", "to_ary");
         for (i = 0; i < RARRAY_LEN(ary); i++) {
             VALUE elt = RARRAY_AREF(ary, i);

ext/stringio/stringio.c

@@ -385,7 +385,7 @@ strio_finalize(VALUE self)
 static VALUE
 strio_s_open(int argc, VALUE *argv, VALUE klass)
 {
-    VALUE obj = rb_class_new_instance(argc, argv, klass);
+    VALUE obj = rb_class_new_instance_kw(argc, argv, klass, RB_PASS_CALLED_KEYWORDS);
     if (!rb_block_given_p()) return obj;
     return rb_ensure(rb_yield, obj, strio_finalize, obj);
 }
@@ -400,7 +400,7 @@ strio_s_new(int argc, VALUE *argv, VALUE klass)
 	rb_warn("%"PRIsVALUE"::new() does not take block; use %"PRIsVALUE"::open() instead",
 		cname, cname);
     }
-    return rb_class_new_instance(argc, argv, klass);
+    return rb_class_new_instance_kw(argc, argv, klass, RB_PASS_CALLED_KEYWORDS);
 }
 
 /*