Skip to content

Commit

Permalink
Fix keyword argument separation issues in Thread.new
Browse files Browse the repository at this point in the history
  • Loading branch information
jeremyevans committed Sep 26, 2019
1 parent 47d4451 commit 6b52959
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 3 deletions.
80 changes: 80 additions & 0 deletions test/ruby/test_keyword.rb
Expand Up @@ -684,6 +684,86 @@ def test_lambda_method_kwsplat_call
assert_equal([1, h3], f[a: 1, **h2])
end

def test_Thread_new_kwsplat
Thread.report_on_exception = false
kw = {}
h = {:a=>1}
h2 = {'a'=>1}
h3 = {'a'=>1, :a=>1}

t = Thread
f = -> { true }
assert_equal(true, t.new(**{}, &f).value)
assert_equal(true, t.new(**kw, &f).value)
assert_raise(ArgumentError) { t.new(**h, &f).value }
assert_raise(ArgumentError) { t.new(a: 1, &f).value }
assert_raise(ArgumentError) { t.new(**h2, &f).value }
assert_raise(ArgumentError) { t.new(**h3, &f).value }

f = ->(a) { a }
assert_warn(/The keyword argument is passed as the last hash parameter/m) do
assert_equal(kw, t.new(**{}, &f).value)
end
assert_warn(/The keyword argument is passed as the last hash parameter/m) do
assert_equal(kw, t.new(**kw, &f).value)
end
assert_equal(h, t.new(**h, &f).value)
assert_equal(h, t.new(a: 1, &f).value)
assert_equal(h2, t.new(**h2, &f).value)
assert_equal(h3, t.new(**h3, &f).value)
assert_equal(h3, t.new(a: 1, **h2, &f).value)

f = ->(**x) { x }
assert_equal(kw, t.new(**{}, &f).value)
assert_equal(kw, t.new(**kw, &f).value)
assert_equal(h, t.new(**h, &f).value)
assert_equal(h, t.new(a: 1, &f).value)
assert_equal(h2, t.new(**h2, &f).value)
assert_equal(h3, t.new(**h3, &f).value)
assert_equal(h3, t.new(a: 1, **h2, &f).value)
assert_warn(/The last argument is used as the keyword parameter.*for method/m) do
assert_equal(h, t.new(h, &f).value)
end
assert_raise(ArgumentError) { t.new(h2, &f).value }
assert_warn(/The last argument is split into positional and keyword parameters.*for method/m) do
assert_raise(ArgumentError) { t.new(h3, &f).value }
end

f = ->(a, **x) { [a,x] }
assert_warn(/The keyword argument is passed as the last hash parameter/) do
assert_equal([{}, {}], t.new(**{}, &f).value)
end
assert_warn(/The keyword argument is passed as the last hash parameter/) do
assert_equal([{}, {}], t.new(**kw, &f).value)
end
assert_warn(/The keyword argument is passed as the last hash parameter/) do
assert_equal([h, {}], t.new(**h, &f).value)
end
assert_warn(/The keyword argument is passed as the last hash parameter/) do
assert_equal([h, {}], t.new(a: 1, &f).value)
end
assert_warn(/The keyword argument is passed as the last hash parameter/) do
assert_equal([h2, {}], t.new(**h2, &f).value)
end
assert_warn(/The keyword argument is passed as the last hash parameter/) do
assert_equal([h3, {}], t.new(**h3, &f).value)
end
assert_warn(/The keyword argument is passed as the last hash parameter/) do
assert_equal([h3, {}], t.new(a: 1, **h2, &f).value)
end

f = ->(a=1, **x) { [a, x] }
assert_equal([1, kw], t.new(**{}, &f).value)
assert_equal([1, kw], t.new(**kw, &f).value)
assert_equal([1, h], t.new(**h, &f).value)
assert_equal([1, h], t.new(a: 1, &f).value)
assert_equal([1, h2], t.new(**h2, &f).value)
assert_equal([1, h3], t.new(**h3, &f).value)
assert_equal([1, h3], t.new(a: 1, **h2, &f).value)
ensure
Thread.report_on_exception = true
end

def test_Class_new_kwsplat_call
kw = {}
h = {:a=>1}
Expand Down
13 changes: 10 additions & 3 deletions thread.c
Expand Up @@ -662,14 +662,17 @@ rb_vm_proc_local_ep(VALUE proc)
}
}

extern VALUE rb_adjust_argv_kw_splat(int *argc, const VALUE **argv, int *kw_splat);

static void
thread_do_start(rb_thread_t *th)
{
native_set_thread_name(th);

if (th->invoke_type == thread_invoke_type_proc) {
VALUE args = th->invoke_arg.proc.args;
long args_len = RARRAY_LEN(args);
int args_len = RARRAY_LEN(args);
int kw_splat = th->invoke_arg.proc.kw_splat;
const VALUE *args_ptr;
VALUE procval = th->invoke_arg.proc.proc;
rb_proc_t *proc;
Expand All @@ -691,9 +694,10 @@ thread_do_start(rb_thread_t *th)
args_ptr = RARRAY_CONST_PTR(args);
}

rb_adjust_argv_kw_splat(&args_len, &args_ptr, &kw_splat);
th->value = rb_vm_invoke_proc(th->ec, proc,
(int)args_len, args_ptr,
VM_NO_KEYWORDS, VM_BLOCK_HANDLER_NONE);
args_len, args_ptr,
kw_splat, VM_BLOCK_HANDLER_NONE);

EXEC_EVENT_HOOK(th->ec, RUBY_EVENT_THREAD_END, th->self, 0, 0, 0, Qundef);
}
Expand Down Expand Up @@ -841,6 +845,9 @@ thread_create_core(VALUE thval, VALUE args, VALUE (*fn)(void *))
th->invoke_type = thread_invoke_type_proc;
th->invoke_arg.proc.proc = rb_block_proc();
th->invoke_arg.proc.args = args;
th->invoke_arg.proc.kw_splat = rb_empty_keyword_given_p() ?
RB_PASS_EMPTY_KEYWORDS :
rb_keyword_given_p();
}

th->priority = current_th->priority;
Expand Down
1 change: 1 addition & 0 deletions vm_core.h
Expand Up @@ -951,6 +951,7 @@ typedef struct rb_thread_struct {
struct {
VALUE proc;
VALUE args;
int kw_splat;
} proc;
struct {
VALUE (*func)(void *);
Expand Down

0 comments on commit 6b52959

Please sign in to comment.