From fc33559c40e08e4ae0a98821a679abddc4bb247c Mon Sep 17 00:00:00 2001 From: Koichi Sasada Date: Thu, 13 Jun 2024 11:48:48 +0900 Subject: [PATCH] clear `kw_flag` if given hash is nil https://bugs.ruby-lang.org/issues/20570 is caused I missed to clear the `kw_flag` even if `keyword_hash` is nil. --- test/ruby/test_keyword.rb | 18 ++++++++++++++++++ vm_args.c | 21 +++++++++++++-------- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/test/ruby/test_keyword.rb b/test/ruby/test_keyword.rb index 34a80c3729..7a9cd74b40 100644 --- a/test/ruby/test_keyword.rb +++ b/test/ruby/test_keyword.rb @@ -4523,6 +4523,24 @@ class TestKeywordArgumentsSymProcRefinements < Test::Unit::TestCase assert_equal({one: 1, two: 2}, f.call(one:, two:)) end + def m_bug20570(*a, **nil) + a + end + + def test_splat_arg_with_prohibited_keyword + assert_equal([], m_bug20570(*[])) + assert_equal([1], m_bug20570(*[1])) + assert_equal([1, 2], m_bug20570(*[1, 2])) + h = nil + assert_equal([], m_bug20570(*[], **h)) + assert_equal([1], m_bug20570(*[1], **h)) + assert_equal([1, 2], m_bug20570(*[1, 2], **h)) + + assert_equal([], m_bug20570(*[], **nil)) + assert_equal([1], m_bug20570(*[1], **nil)) + assert_equal([1, 2], m_bug20570(*[1, 2], **nil)) + end + private def one 1 end diff --git a/vm_args.c b/vm_args.c index 0c1a26cfad..5593f95695 100644 --- a/vm_args.c +++ b/vm_args.c @@ -524,10 +524,9 @@ static inline int ignore_keyword_hash_p(VALUE keyword_hash, const rb_iseq_t * const iseq, unsigned int * kw_flag, VALUE * converted_keyword_hash) { if (keyword_hash == Qnil) { - return 1; + goto ignore; } - - if (!RB_TYPE_P(keyword_hash, T_HASH)) { + else if (!RB_TYPE_P(keyword_hash, T_HASH)) { keyword_hash = rb_to_hash_type(keyword_hash); } else if (UNLIKELY(ISEQ_BODY(iseq)->param.flags.anon_kwrest)) { @@ -543,9 +542,17 @@ ignore_keyword_hash_p(VALUE keyword_hash, const rb_iseq_t * const iseq, unsigned keyword_hash = rb_hash_dup(keyword_hash); } *converted_keyword_hash = keyword_hash; - return !(ISEQ_BODY(iseq)->param.flags.has_kw) && - !(ISEQ_BODY(iseq)->param.flags.has_kwrest) && - RHASH_EMPTY_P(keyword_hash); + + if (!(ISEQ_BODY(iseq)->param.flags.has_kw) && + !(ISEQ_BODY(iseq)->param.flags.has_kwrest) && + RHASH_EMPTY_P(keyword_hash)) { + ignore: + *kw_flag &= ~(VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT); + return 1; + } + else { + return 0; + } } static VALUE @@ -699,7 +706,6 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co rb_ary_pop(args->rest); } given_argc--; - kw_flag &= ~(VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT); } else { if (rest_last != converted_keyword_hash) { @@ -730,7 +736,6 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co if (ignore_keyword_hash_p(last_arg, iseq, &kw_flag, &converted_keyword_hash)) { args->argc--; given_argc--; - kw_flag &= ~(VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT); } else { if (!(kw_flag & VM_CALL_KW_SPLAT_MUT) && !ISEQ_BODY(iseq)->param.flags.has_kw) {