Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fully separate positional arguments and keyword arguments #2794

Merged
merged 3 commits into from
Jan 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 5 additions & 65 deletions class.c
Original file line number Diff line number Diff line change
Expand Up @@ -1970,7 +1970,6 @@ rb_scan_args_parse(int kw_flag, int argc, const VALUE *argv, const char *fmt, st
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));
Expand All @@ -1979,16 +1978,11 @@ rb_scan_args_parse(int kw_flag, int argc, const VALUE *argv, const char *fmt, st

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;
Expand Down Expand Up @@ -2023,67 +2017,13 @@ rb_scan_args_parse(int kw_flag, int argc, const VALUE *argv, const char *fmt, st
}
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;
Expand Down
12 changes: 4 additions & 8 deletions cont.c
Original file line number Diff line number Diff line change
Expand Up @@ -1790,8 +1790,6 @@ rb_fiber_new(rb_block_call_func_t func, VALUE obj)

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)
{
Expand All @@ -1809,7 +1807,6 @@ rb_fiber_start(void)
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;
Expand All @@ -1818,8 +1815,7 @@ rb_fiber_start(void)
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();

Expand Down Expand Up @@ -2163,7 +2159,7 @@ rb_fiber_alive_p(VALUE fiber_value)
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());
}

/*
Expand Down Expand Up @@ -2249,7 +2245,7 @@ rb_fiber_m_transfer(int argc, VALUE *argv, VALUE fiber_value)
{
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());
}

/*
Expand All @@ -2265,7 +2261,7 @@ rb_fiber_m_transfer(int argc, VALUE *argv, VALUE fiber_value)
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());
}

/*
Expand Down
14 changes: 6 additions & 8 deletions enumerator.c
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,6 @@ enumerator_allocate(VALUE klass)
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)
{
Expand Down Expand Up @@ -480,7 +478,7 @@ enumerator_initialize(int argc, VALUE *argv, VALUE obj)
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);
Expand Down Expand Up @@ -535,10 +533,10 @@ rb_enumeratorize_with_size(VALUE obj, VALUE meth, int argc, const VALUE *argv, r
/* 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
Expand Down Expand Up @@ -1892,7 +1890,7 @@ lazy_add_method(VALUE obj, int argc, VALUE *argv, VALUE args, VALUE memo,
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::Lazy has no method name */
rb_ivar_set(result, id_method, Qfalse);
return result;
Expand Down Expand Up @@ -1940,7 +1938,7 @@ lazy_to_enum(int argc, VALUE *argv, VALUE self)
if (RTEST((super_meth = rb_hash_aref(lazy_use_super_method, meth)))) {
meth = super_meth;
}
lazy = lazy_to_enum_i(self, meth, argc, argv, 0, PASS_KW_SPLAT);
lazy = lazy_to_enum_i(self, meth, argc, argv, 0, rb_keyword_given_p());
if (rb_block_given_p()) {
enumerator_ptr(lazy)->size = rb_block_proc();
}
Expand Down Expand Up @@ -3318,7 +3316,7 @@ rb_arith_seq_new(VALUE obj, VALUE meth, int argc, VALUE const *argv,
VALUE beg, VALUE end, VALUE step, int excl)
{
VALUE aseq = enumerator_init(enumerator_allocate(rb_cArithSeq),
obj, meth, argc, argv, size_fn, Qnil, PASS_KW_SPLAT);
obj, meth, argc, argv, size_fn, Qnil, rb_keyword_given_p());
rb_ivar_set(aseq, id_begin, beg);
rb_ivar_set(aseq, id_end, end);
rb_ivar_set(aseq, id_step, step);
Expand Down
8 changes: 0 additions & 8 deletions eval.c
Original file line number Diff line number Diff line change
Expand Up @@ -924,14 +924,6 @@ rb_keyword_given_p(void)
return rb_vm_cframe_keyword_p(GET_EC()->cfp);
}

/* -- Remove In 3.0 -- */
int rb_vm_cframe_empty_keyword_p(const rb_control_frame_t *cfp);
int
rb_empty_keyword_given_p(void)
{
return rb_vm_cframe_empty_keyword_p(GET_EC()->cfp);
}

VALUE rb_eThreadError;

/*! Declares that the current method needs a block.
Expand Down
10 changes: 0 additions & 10 deletions ext/-test-/scan_args/scan_args.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,15 +259,6 @@ scan_args_k_lead_opt_hash(int argc, VALUE *argv, VALUE self)
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_kw(RB_SCAN_ARGS_EMPTY_KEYWORDS, argc, argv, "11:", 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)
{
Expand Down Expand Up @@ -310,6 +301,5 @@ Init_scan_args(void)
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);
}
78 changes: 4 additions & 74 deletions include/ruby/ruby.h
Original file line number Diff line number Diff line change
Expand Up @@ -1901,7 +1901,6 @@ VALUE rb_funcall_with_block_kw(VALUE, ID, int, const VALUE*, VALUE, int);
int rb_scan_args(int, const VALUE*, const char*, ...);
#define RB_SCAN_ARGS_PASS_CALLED_KEYWORDS 0
#define RB_SCAN_ARGS_KEYWORDS 1
#define RB_SCAN_ARGS_EMPTY_KEYWORDS 2 /* Will be removed in 3.0 */
#define RB_SCAN_ARGS_LAST_HASH_KEYWORDS 3
int rb_scan_args_kw(int, int, const VALUE*, const char*, ...);
VALUE rb_call_super(int, const VALUE*);
Expand Down Expand Up @@ -1976,8 +1975,7 @@ VALUE rb_yield_splat_kw(VALUE, int);
VALUE rb_yield_block(RB_BLOCK_CALL_FUNC_ARGLIST(yielded_arg, callback_arg)); /* rb_block_call_func */
#define RB_NO_KEYWORDS 0
#define RB_PASS_KEYWORDS 1
#define RB_PASS_EMPTY_KEYWORDS 2 /* Will be removed in 3.0 */
#define RB_PASS_CALLED_KEYWORDS 3
#define RB_PASS_CALLED_KEYWORDS rb_keyword_given_p()
int rb_keyword_given_p(void);
int rb_block_given_p(void);
void rb_need_block(void);
Expand Down Expand Up @@ -2331,9 +2329,6 @@ unsigned long ruby_strtoul(const char *str, char **endptr, int base);
PRINTF_ARGS(int ruby_snprintf(char *str, size_t n, char const *fmt, ...), 3, 4);
int ruby_vsnprintf(char *str, size_t n, char const *fmt, va_list ap);

/* -- Remove In 3.0, Only public for rb_scan_args optimized version -- */
int rb_empty_keyword_given_p(void);

#if defined(HAVE_BUILTIN___BUILTIN_CHOOSE_EXPR_CONSTANT_P) && defined(HAVE_VA_ARGS_MACRO) && defined(__OPTIMIZE__)
# define rb_scan_args(argc,argvp,fmt,...) \
__builtin_choose_expr(__builtin_constant_p(fmt), \
Expand Down Expand Up @@ -2525,78 +2520,13 @@ rb_scan_args_set(int argc, const VALUE *argv,
int i, argi = 0, vari = 0, last_idx = -1;
VALUE *var, hash = Qnil, last_hash = 0;
const int n_mand = n_lead + n_trail;
int keyword_given = rb_keyword_given_p();
int empty_keyword_given = 0;
VALUE tmp_buffer = 0;

if (!keyword_given) {
empty_keyword_given = rb_empty_keyword_given_p();
if (f_hash && argc > 0 && rb_keyword_given_p()) {
hash = rb_hash_dup(argv[argc - 1]);
argc--;
}

/* capture an option hash - phase 1: pop */
/* Ignore final positional hash if empty keywords given */
if (argc > 0 && !(f_hash && empty_keyword_given)) {
VALUE last = argv[argc - 1];

if (f_hash && 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 {
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 (!keyword_given) {
/* 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");
last_idx = argc - 1;
}
hash = opts ? opts : Qnil;
}
}
else if (f_hash && keyword_given && 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 (f_hash && n_mand > 0 && n_mand == argc+1 && empty_keyword_given) {
VALUE *ptr = (VALUE *)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");
}


if (argc < n_mand) {
goto argc_error;
}
Expand Down
1 change: 0 additions & 1 deletion internal/vm.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ VALUE rb_lambda_call(VALUE obj, ID mid, int argc, const VALUE *argv,

MJIT_SYMBOL_EXPORT_BEGIN
VALUE rb_vm_call0(struct rb_execution_context_struct *ec, VALUE recv, ID id, int argc, const VALUE *argv, const struct rb_callable_method_entry_struct *me, int kw_splat);
VALUE rb_adjust_argv_kw_splat(int *argc, const VALUE **argv, int *kw_splat);
VALUE rb_vm_call_kw(struct rb_execution_context_struct *ec, VALUE recv, VALUE id, int argc, const VALUE *argv, const struct rb_callable_method_entry_struct *me, int kw_splat);
VALUE rb_make_no_method_exception(VALUE exc, VALUE format, VALUE obj, int argc, const VALUE *argv, int priv);
MJIT_SYMBOL_EXPORT_END
Expand Down
4 changes: 1 addition & 3 deletions object.c
Original file line number Diff line number Diff line change
Expand Up @@ -4086,9 +4086,7 @@ rb_obj_dig(int argc, VALUE *argv, VALUE obj, VALUE notfound)
}
return rb_check_funcall_with_hook_kw(obj, id_dig, argc, argv,
no_dig_method, obj,
rb_empty_keyword_given_p() ?
RB_PASS_EMPTY_KEYWORDS :
RB_NO_KEYWORDS);
RB_NO_KEYWORDS);
}
return obj;
}
Expand Down
Loading