Skip to content

Instantly share code, notes, and snippets.

@mame mame/README.md Secret
Last active Jul 23, 2018

Embed
What would you like to do?
Ruby 3's keyword argument separation. https://bugs.ruby-lang.org/issues/14183

NG: passing a keyword argument to a normal parameter

$ ./miniruby -w -e '
def foo(h)
end
foo(k: 1)
'
-e:4: warning: The keyword argument for `foo' is used as the last parameter

OK: receving it as a keyword rest argument

$ ./miniruby -w -e '
def foo(**h)
end
foo(k: 1)
'

NG: passing a normal hash argument to a keyword argument

$ ./miniruby -w -e '
def foo(k: 1)
end
h = {k: 42}
foo(h)
'
-e:5: warning: The last argument for `foo' is used as the keyword parameter

OK: passing the hash as keyword argument by using **

$ ./miniruby -w -e '
def foo(k: 1)
end
h = {k: 42}
foo(**h)
'

A current issue: methods written in C handles keyword argument as a normal parameter

$ ./miniruby -w -e '
class C
  def initialize(k: 1)
  end  
end
C.new(k: 1)
'
-e:6: warning: The keyword argument for `new' is used as the last parameter
-e:6: warning: The last argument for `initialize' is used as the keyword parameter
diff --git a/compile.c b/compile.c
index 5a458200ec..2084725809 100644
--- a/compile.c
+++ b/compile.c
@@ -3748,7 +3748,7 @@ compile_array_keyword_arg(rb_iseq_t *iseq, LINK_ANCHOR *const ret,
{
if (kw_arg_ptr == NULL) return FALSE;
- if (nd_type(root_node) == NODE_HASH && root_node->nd_head && nd_type(root_node->nd_head) == NODE_ARRAY) {
+ if (nd_type(root_node) == NODE_HASH && !root_node->nd_alen && root_node->nd_head && nd_type(root_node->nd_head) == NODE_ARRAY) {
const NODE *node = root_node->nd_head;
while (node) {
@@ -3756,7 +3756,7 @@ compile_array_keyword_arg(rb_iseq_t *iseq, LINK_ANCHOR *const ret,
assert(nd_type(node) == NODE_ARRAY);
if (!key_node) {
- if (flag && !root_node->nd_alen) *flag |= VM_CALL_KW_SPLAT;
+ if (flag) *flag |= VM_CALL_KW_SPLAT;
return FALSE;
}
else if (nd_type(key_node) == NODE_LIT && RB_TYPE_P(key_node->nd_lit, T_SYMBOL)) {
diff --git a/vm_args.c b/vm_args.c
index 3c1a2a3d75..9b2f05b011 100644
--- a/vm_args.c
+++ b/vm_args.c
@@ -514,6 +514,7 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
VALUE keyword_hash = Qnil;
VALUE * const orig_sp = ec->cfp->sp;
unsigned int i;
+ int k2n_warned = 0;
/*
* Extend SP for GC.
@@ -586,6 +587,12 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
/* argc check */
if (given_argc < min_argc) {
if (given_argc == min_argc - 1 && args->kw_argv) {
+ /* Warn the following:
+ * def foo(a, k:1) p [a, k] end
+ * foo(k:42) #=> [{:k=>42}, 1]
+ */
+ rb_warning("The keyword argument for `%s' is used as the last parameter", rb_id2name(ci->mid));
+ k2n_warned = TRUE;
args_stored_kw_argv_to_hash(args);
given_argc = args_argc(args);
}
@@ -609,9 +616,26 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
(kw_splat && given_argc > max_argc)) &&
args->kw_argv == NULL) {
if (args_pop_keyword_hash(args, &keyword_hash)) {
+ if (!(ci->flag & (VM_CALL_KWARG | VM_CALL_KW_SPLAT))) {
+ /* Warn the following:
+ * def foo(k:1) p [k]; end
+ * foo({k:42}) #=> 42
+ */
+ rb_warning("The last argument for `%s' is used as the keyword parameter", rb_id2name(ci->mid));
+ }
given_argc--;
}
}
+ else if (!k2n_warned &&
+ !((ci->flag & VM_CALL_KWARG) && iseq->body->param.flags.has_kw) &&
+ given_argc == min_argc &&
+ (ci->flag & (VM_CALL_KWARG | VM_CALL_KW_SPLAT))) {
+ /* Warn the following:
+ * def foo(x, opt=1) p [x]; end
+ * foo(k:42) #=> [{:k=>42}]
+ */
+ rb_warning("The keyword argument for `%s' is used as the last parameter", rb_id2name(ci->mid));
+ }
if (given_argc > max_argc && max_argc != UNLIMITED_ARGUMENTS) {
if (arg_setup_type == arg_setup_block) {
@@ -792,6 +816,12 @@ vm_caller_setup_arg_kw(rb_control_frame_t *cfp, struct rb_calling_info *calling,
VALUE *sp = cfp->sp;
int i;
+ /* Warn the following:
+ * def foo(x) p [x] end
+ * foo(k:42) #=> [{:k=>42}]
+ */
+ rb_warning("The keyword argument for `%s' is used as the last parameter", rb_id2name(ci->mid));
+
for (i=0; i<kw_len; i++) {
rb_hash_aset(h, passed_keywords[i], (sp - kw_len)[i]);
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.