mirror/ruby
1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2026-01-26 12:14:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix allocationless anonymous splat keyword argument check

Browse Source 
Previously, if an argument splat and keywords are provided by
the caller, it did not check whether the method/proc accepted
keywords before avoiding the allocation. This is incorrect,
because if the method/proc does not accept keywords, the
keywords passed by the caller are added as a positional
argument, so there must be an allocation to avoid mutating
the positional splat argument.

Add a check that if the caller passes keywords, the
method/proc must accept keywords in order to optimize.
If the caller passes a keyword splat, either the
method/proc must accept keywords, or the keyword splat must
be empty in order to optimize.

If keywords are explicitly disallowed via `**nil`, the
optimization should be skipped, because the array is mutated
before the ArgumentError exception is raised.

In addition to a test for the correct behavior, add an
allocation test for a method that accepts an anonymous splat
without keywords.

Fixes [Bug #21757]
This commit is contained in:
Jeremy Evans 2025-12-02 17:34:36 -08:00
parent bd0d08b6d2
commit 6409715212
Notes: git 2025-12-09 20:45:22 +00:00 
Merged: https://github.com/ruby/ruby/pull/15383
3 changed files with 102 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
test/ruby/test_allocation.rb
View File

@ -527,6 +527,59 @@ class TestAllocation < Test::Unit::TestCase
RUBY
end
def test_anonymous_splat_parameter
only_block = block.empty? ? block : block[2..]
check_allocations(<<~RUBY)
def self.anon_splat(*#{block}); end
check_allocations(1, 1, "anon_splat(1, a: 2#{block})")
check_allocations(1, 1, "anon_splat(1, *empty_array, a: 2#{block})")
check_allocations(1, 1, "anon_splat(1, a:2, **empty_hash#{block})")
check_allocations(1, 1, "anon_splat(1, **empty_hash, a: 2#{block})")
check_allocations(1, 0, "anon_splat(1, **nil#{block})")
check_allocations(1, 0, "anon_splat(1, **empty_hash#{block})")
check_allocations(1, 1, "anon_splat(1, **hash1#{block})")
check_allocations(1, 1, "anon_splat(1, *empty_array, **hash1#{block})")
check_allocations(1, 1, "anon_splat(1, **hash1, **empty_hash#{block})")
check_allocations(1, 1, "anon_splat(1, **empty_hash, **hash1#{block})")
check_allocations(1, 0, "anon_splat(1, *empty_array#{block})")
check_allocations(1, 0, "anon_splat(1, *empty_array, *empty_array, **empty_hash#{block})")
check_allocations(1, 1, "anon_splat(*array1, a: 2#{block})")
check_allocations(0, 0, "anon_splat(*nil, **nill#{block})")
check_allocations(0, 0, "anon_splat(*array1, **nill#{block})")
check_allocations(0, 0, "anon_splat(*array1, **empty_hash#{block})")
check_allocations(1, 1, "anon_splat(*array1, **hash1#{block})")
check_allocations(1, 1, "anon_splat(*array1, *empty_array, **hash1#{block})")
check_allocations(1, 0, "anon_splat(*array1, *empty_array#{block})")
check_allocations(1, 0, "anon_splat(*array1, *empty_array, **empty_hash#{block})")
check_allocations(1, 1, "anon_splat(*array1, *empty_array, a: 2, **empty_hash#{block})")
check_allocations(1, 1, "anon_splat(*array1, *empty_array, **hash1, **empty_hash#{block})")
check_allocations(0, 0, "anon_splat(#{only_block})")
check_allocations(1, 1, "anon_splat(a: 2#{block})")
check_allocations(0, 0, "anon_splat(**empty_hash#{block})")
check_allocations(1, 1, "anon_splat(1, *empty_array, a: 2, **empty_hash#{block})")
check_allocations(1, 1, "anon_splat(1, *empty_array, **hash1, **empty_hash#{block})")
check_allocations(1, 1, "anon_splat(*array1, **empty_hash, a: 2#{block})")
check_allocations(1, 1, "anon_splat(*array1, **hash1, **empty_hash#{block})")
unless defined?(RubyVM::YJIT.enabled?) && RubyVM::YJIT.enabled?
check_allocations(0, 0, "anon_splat(*array1, **nil#{block})")
check_allocations(1, 0, "anon_splat(*r2k_empty_array#{block})")
check_allocations(1, 1, "anon_splat(*r2k_array#{block})")
check_allocations(1, 0, "anon_splat(*r2k_empty_array1#{block})")
check_allocations(1, 1, "anon_splat(*r2k_array1#{block})")
end
RUBY
end
def test_anonymous_splat_and_anonymous_keyword_splat_parameters
only_block = block.empty? ? block : block[2..]
check_allocations(<<~RUBY)

29
test/ruby/test_call.rb
View File

@ -423,6 +423,35 @@ class TestCall < Test::Unit::TestCase
assert_equal([1, 2, {}], s(*r2ka))
end
def test_anon_splat_mutated_bug_21757
args = [1, 2]
kw = {bug: true}
def self.m(*); end
m(*args, bug: true)
assert_equal(2, args.length)
proc = ->(*) { }
proc.(*args, bug: true)
assert_equal(2, args.length)
def self.m2(*); end
m2(*args, **kw)
assert_equal(2, args.length)
proc = ->(*) { }
proc.(*args, **kw)
assert_equal(2, args.length)
def self.m3(*, **nil); end
assert_raise(ArgumentError) { m3(*args, bug: true) }
assert_equal(2, args.length)
proc = ->(*, **nil) { }
assert_raise(ArgumentError) { proc.(*args, bug: true) }
assert_equal(2, args.length)
end
def test_kwsplat_block_eval_order
def self.t(**kw, &b) [kw, b] end

26
vm_args.c
View File

@ -638,12 +638,26 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
given_argc == ISEQ_BODY(iseq)->param.lead_num + (kw_flag ? 2 : 1) &&
!ISEQ_BODY(iseq)->param.flags.has_opt &&
!ISEQ_BODY(iseq)->param.flags.has_post &&
!ISEQ_BODY(iseq)->param.flags.ruby2_keywords &&
(!kw_flag ||
!ISEQ_BODY(iseq)->param.flags.has_kw ||
!ISEQ_BODY(iseq)->param.flags.has_kwrest ||
!ISEQ_BODY(iseq)->param.flags.accepts_no_kwarg)) {
args->rest_dupped = true;
!ISEQ_BODY(iseq)->param.flags.ruby2_keywords) {
if (kw_flag) {
if (ISEQ_BODY(iseq)->param.flags.has_kw ||
ISEQ_BODY(iseq)->param.flags.has_kwrest) {
args->rest_dupped = true;
}
else if (kw_flag & VM_CALL_KW_SPLAT) {
VALUE kw_hash = locals[args->argc - 1];
if (kw_hash == Qnil ||
(RB_TYPE_P(kw_hash, T_HASH) && RHASH_EMPTY_P(kw_hash))) {
args->rest_dupped = true;
}
}
}
else if (!ISEQ_BODY(iseq)->param.flags.has_kw &&
!ISEQ_BODY(iseq)->param.flags.has_kwrest &&
!ISEQ_BODY(iseq)->param.flags.accepts_no_kwarg) {
args->rest_dupped = true;
}
}
}