ruby-changes:59610
From: Jeremy <ko1@a...>
Date: Fri, 3 Jan 2020 11:41:08 +0900 (JST)
Subject: [ruby-changes:59610] beae6cbf0f (master): Fully separate positional arguments and keyword arguments
https://git.ruby-lang.org/ruby.git/commit/?id=beae6cbf0f From beae6cbf0fd8b6619e5212552de98022d4c4d4d4 Mon Sep 17 00:00:00 2001 From: Jeremy Evans <code@j...> Date: Fri, 4 Oct 2019 12:51:57 -0700 Subject: Fully separate positional arguments and keyword arguments This removes the warnings added in 2.7, and changes the behavior so that a final positional hash is not treated as keywords or vice-versa. To handle the arg_setup_block splat case correctly with keyword arguments, we need to check if we are taking a keyword hash. That case didn't have a test, but it affects real-world code, so add a test for it. This removes rb_empty_keyword_given_p() and related code, as that is not needed in Ruby 3. The empty keyword case is the same as the no keyword case in Ruby 3. This changes rb_scan_args to implement keyword argument separation for C functions when the : character is used. For backwards compatibility, it returns a duped hash. This is a bad idea for performance, but not duping the hash breaks at least Enumerator::ArithmeticSequence#inspect. Instead of having RB_PASS_CALLED_KEYWORDS be a number, simplify the code by just making it be rb_keyword_given_p(). diff --git a/class.c b/class.c index bc64ff7..e42ee08 100644 --- a/class.c +++ b/class.c @@ -1970,7 +1970,6 @@ rb_scan_args_parse(int kw_flag, int argc, const VALUE *argv, const char *fmt, st https://github.com/ruby/ruby/blob/trunk/class.c#L1970 const char *p = fmt; VALUE *tmp_buffer = arg->tmp_buffer; int keyword_given = 0; - int empty_keyword_given = 0; int last_hash_keyword = 0; memset(arg, 0, sizeof(*arg)); @@ -1979,16 +1978,11 @@ rb_scan_args_parse(int kw_flag, int argc, const VALUE *argv, const char *fmt, st https://github.com/ruby/ruby/blob/trunk/class.c#L1978 switch (kw_flag) { case RB_SCAN_ARGS_PASS_CALLED_KEYWORDS: - if (!(keyword_given = rb_keyword_given_p())) { - empty_keyword_given = rb_empty_keyword_given_p(); - } + keyword_given = rb_keyword_given_p(); break; case RB_SCAN_ARGS_KEYWORDS: keyword_given = 1; break; - case RB_SCAN_ARGS_EMPTY_KEYWORDS: - empty_keyword_given = 1; - break; case RB_SCAN_ARGS_LAST_HASH_KEYWORDS: last_hash_keyword = 1; break; @@ -2023,67 +2017,13 @@ rb_scan_args_parse(int kw_flag, int argc, const VALUE *argv, const char *fmt, st https://github.com/ruby/ruby/blob/trunk/class.c#L2017 } arg->n_mand = arg->n_lead + arg->n_trail; - /* capture an option hash - phase 1: pop */ - /* Ignore final positional hash if empty keywords given */ - if (argc > 0 && !(arg->f_hash && empty_keyword_given)) { + if (arg->f_hash && argc > 0) { VALUE last = argv[argc - 1]; - if (arg->f_hash && arg->n_mand < argc) { - if (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 { - arg->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 (!arg->f_var && arg->n_mand + arg->n_opt < argc) { - rb_warn("The last argument is nil, treating as empty keywords"); - argc--; - } - } - else { - arg->hash = rb_check_hash_type(last); - } - - /* Ruby 3: Remove if branch, as it will not attempt to split hashes */ - if (!NIL_P(arg->hash)) { - VALUE opts = rb_extract_keywords(&arg->hash); - - if (!(arg->last_hash = arg->hash)) { - if (!keyword_given && !last_hash_keyword) { - /* Warn if treating positional as keyword, as in Ruby 3, - this will be an error */ - rb_warn("Using the last argument as keyword parameters is deprecated"); - } - argc--; - } - else { - /* Warn 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"); - arg->last_idx = argc - 1; - } - arg->hash = opts ? opts : Qnil; - } + if (keyword_given || (last_hash_keyword && RB_TYPE_P(last, T_HASH))) { + arg->hash = rb_hash_dup(last); + argc--; } - else if (arg->f_hash && keyword_given && arg->n_mand == argc) { - /* Warn if treating keywords as positional, as in Ruby 3, this will be an error */ - rb_warn("Passing the keyword argument as the last hash parameter is deprecated"); - } - } - if (arg->f_hash && arg->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("Passing the keyword argument as the last hash parameter is deprecated"); } arg->argc = argc; diff --git a/cont.c b/cont.c index 2365406..d4a2bf9 100644 --- a/cont.c +++ b/cont.c @@ -1790,8 +1790,6 @@ rb_fiber_new(rb_block_call_func_t func, VALUE obj) https://github.com/ruby/ruby/blob/trunk/cont.c#L1790 static void rb_fiber_terminate(rb_fiber_t *fiber, int need_interrupt); -#define PASS_KW_SPLAT (rb_empty_keyword_given_p() ? RB_PASS_EMPTY_KEYWORDS : rb_keyword_given_p()) - void rb_fiber_start(void) { @@ -1809,7 +1807,6 @@ rb_fiber_start(void) https://github.com/ruby/ruby/blob/trunk/cont.c#L1807 rb_context_t *cont = &VAR_FROM_MEMORY(fiber)->cont; int argc; const VALUE *argv, args = cont->value; - int kw_splat = cont->kw_splat; GetProcPtr(fiber->first_proc, proc); argv = (argc = cont->argc) > 1 ? RARRAY_CONST_PTR(args) : &args; cont->value = Qnil; @@ -1818,8 +1815,7 @@ rb_fiber_start(void) https://github.com/ruby/ruby/blob/trunk/cont.c#L1815 th->ec->root_svar = Qfalse; EXEC_EVENT_HOOK(th->ec, RUBY_EVENT_FIBER_SWITCH, th->self, 0, 0, 0, Qnil); - rb_adjust_argv_kw_splat(&argc, &argv, &kw_splat); - cont->value = rb_vm_invoke_proc(th->ec, proc, argc, argv, kw_splat, VM_BLOCK_HANDLER_NONE); + cont->value = rb_vm_invoke_proc(th->ec, proc, argc, argv, cont->kw_splat, VM_BLOCK_HANDLER_NONE); } EC_POP_TAG(); @@ -2163,7 +2159,7 @@ rb_fiber_alive_p(VALUE fiber_value) https://github.com/ruby/ruby/blob/trunk/cont.c#L2159 static VALUE rb_fiber_m_resume(int argc, VALUE *argv, VALUE fiber) { - return rb_fiber_resume_kw(fiber, argc, argv, PASS_KW_SPLAT); + return rb_fiber_resume_kw(fiber, argc, argv, rb_keyword_given_p()); } /* @@ -2249,7 +2245,7 @@ rb_fiber_m_transfer(int argc, VALUE *argv, VALUE fiber_value) https://github.com/ruby/ruby/blob/trunk/cont.c#L2245 { rb_fiber_t *fiber = fiber_ptr(fiber_value); fiber->transferred = 1; - return fiber_switch(fiber, argc, argv, 0, PASS_KW_SPLAT); + return fiber_switch(fiber, argc, argv, 0, rb_keyword_given_p()); } /* @@ -2265,7 +2261,7 @@ rb_fiber_m_transfer(int argc, VALUE *argv, VALUE fiber_value) https://github.com/ruby/ruby/blob/trunk/cont.c#L2261 static VALUE rb_fiber_s_yield(int argc, VALUE *argv, VALUE klass) { - return rb_fiber_yield_kw(argc, argv, PASS_KW_SPLAT); + return rb_fiber_yield_kw(argc, argv, rb_keyword_given_p()); } /* diff --git a/enumerator.c b/enumerator.c index 79081806..fa30a59 100644 --- a/enumerator.c +++ b/enumerator.c @@ -384,8 +384,6 @@ enumerator_allocate(VALUE klass) https://github.com/ruby/ruby/blob/trunk/enumerator.c#L384 return enum_obj; } -#define PASS_KW_SPLAT (rb_empty_keyword_given_p() ? RB_PASS_EMPTY_KEYWORDS : rb_keyword_given_p()) - static VALUE enumerator_init(VALUE enum_obj, VALUE obj, VALUE meth, int argc, const VALUE *argv, rb_enumerator_size_func *size_fn, VALUE size, int kw_splat) { @@ -480,7 +478,7 @@ enumerator_initialize(int argc, VALUE *argv, VALUE obj) https://github.com/ruby/ruby/blob/trunk/enumerator.c#L478 meth = *argv++; --argc; } - kw_splat = PASS_KW_SPLAT; + kw_splat = rb_keyword_given_p(); } return enumerator_init(obj, recv, meth, argc, argv, 0, size, kw_splat); @@ -535,10 +533,10 @@ rb_enumeratorize_with_size(VALUE obj, VALUE meth, int argc, const VALUE *argv, r https://github.com/ruby/ruby/blob/trunk/enumerator.c#L533 /* Similar effect as calling obj.to_enum, i.e. dispatching to either Kernel#to_enum vs Lazy#to_enum */ if (RTEST(rb_obj_is_kind_of(obj, rb_cLazy))) - return lazy_to_enum_i(obj, meth, argc, argv, size_fn, PASS_KW_SPLAT); + return lazy_to_enum_i(obj, meth, argc, argv, size_fn, rb_keyword_given_p()); else return enumerator_init(enumerator_allocate(rb_cEnumerator), - obj, meth, argc, argv, size_fn, Qnil, PASS_KW_SPLAT); + obj, meth, argc, argv, size_fn, Qnil, rb_keyword_given_p()); } VALUE @@ -1892,7 +1890,7 @@ lazy_add_method(VALUE obj, int argc, VALUE *argv, VALUE args, VALUE memo, https://github.com/ruby/ruby/blob/trunk/enumerator.c#L1890 static VALUE enumerable_lazy(VALUE obj) { - VALUE result = lazy_to_enum_i(obj, sym_each, 0, 0, lazyenum_size, PASS_KW_SPLAT); + VALUE result = lazy_to_enum_i(obj, sym_each, 0, 0, lazyenum_size, rb_keyword_given_p()); /* Qfalse indicates that the Enumerator: (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/