From 6b52959ef76f6f19e50c6f80f00c08bb0daf5c7c Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Wed, 25 Sep 2019 17:14:17 -0700 Subject: [PATCH] Fix keyword argument separation issues in Thread.new --- test/ruby/test_keyword.rb | 80 +++++++++++++++++++++++++++++++++++++++ thread.c | 13 +++++-- vm_core.h | 1 + 3 files changed, 91 insertions(+), 3 deletions(-) diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb index 1cfa982f0c12ef..dbced97b7e121f 100644 --- a/test/ruby/test_keyword.rb +++ b/test/ruby/test_keyword.rb @@ -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} diff --git a/thread.c b/thread.c index 09f413557f02ce..b6fd8e52b3b139 100644 --- a/thread.c +++ b/thread.c @@ -662,6 +662,8 @@ 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) { @@ -669,7 +671,8 @@ thread_do_start(rb_thread_t *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; @@ -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); } @@ -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; diff --git a/vm_core.h b/vm_core.h index 4c233fa27f061f..f1e74439269f2a 100644 --- a/vm_core.h +++ b/vm_core.h @@ -951,6 +951,7 @@ typedef struct rb_thread_struct { struct { VALUE proc; VALUE args; + int kw_splat; } proc; struct { VALUE (*func)(void *);