зеркало из https://github.com/github/ruby.git
Fix keyword splat passing as regular argument
Since Ruby 3.0, Ruby has passed a keyword splat as a regular argument in the case of a call to a Ruby method where the method does not accept keyword arguments, if the method call does not contain an argument splat: ```ruby def self.f(obj) obj end def self.fs(*obj) obj[0] end h = {a: 1} f(**h).equal?(h) # Before: true; After: false fs(**h).equal?(h) # Before: true; After: false a = [] f(*a, **h).equal?(h) # Before and After: false fs(*a, **h).equal?(h) # Before and After: false ``` The fact that the behavior differs when passing an empty argument splat makes it obvious that something is not working the way it is intended. Ruby 2 always copied the keyword splat hash, and that is the expected behavior in Ruby 3. This bug is because of a missed check in setup_parameters_complex. If the keyword splat passed is not mutable, then it points to an existing object and not a new object, and therefore it must be copied. Now, there are 3 specs for the broken behavior of directly using the keyword splatted hash. Fix two specs and add a new version guard. Do not keep the specs for the broken behavior for earlier Ruby versions, in case this fix is backported. For the ruby2_keywords spec, just remove the related line, since that line is unrelated to what the spec is testing. Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org>
This commit is contained in:
Родитель
e6a6ea9dcf
Коммит
ca204a2023
|
@ -22,7 +22,6 @@ describe "Module#ruby2_keywords" do
|
|||
end
|
||||
|
||||
h = {a: 1}
|
||||
obj.regular(**h).should.equal?(h)
|
||||
|
||||
last = mark(**h).last
|
||||
Hash.ruby2_keywords_hash?(last).should == true
|
||||
|
|
|
@ -220,15 +220,17 @@ describe "The ** operator" do
|
|||
h.should == { one: 1, two: 2 }
|
||||
end
|
||||
|
||||
it "does not copy when calling a method taking a positional Hash" do
|
||||
ruby_bug "#20012", ""..."3.3" do
|
||||
it "makes a copy when calling a method taking a positional Hash" do
|
||||
def m(h)
|
||||
h.delete(:one); h
|
||||
end
|
||||
|
||||
h = { one: 1, two: 2 }
|
||||
m(**h).should == { two: 2 }
|
||||
m(**h).should.equal?(h)
|
||||
h.should == { two: 2 }
|
||||
m(**h).should_not.equal?(h)
|
||||
h.should == { one: 1, two: 2 }
|
||||
end
|
||||
end
|
||||
|
||||
ruby_version_is "3.1" do
|
||||
|
|
|
@ -87,13 +87,16 @@ describe "Keyword arguments" do
|
|||
end
|
||||
|
||||
context "**" do
|
||||
it "does not copy a non-empty Hash for a method taking (*args)" do
|
||||
ruby_version_is "3.3" do
|
||||
it "copies a non-empty Hash for a method taking (*args)" do
|
||||
def m(*args)
|
||||
args[0]
|
||||
end
|
||||
|
||||
h = {a: 1}
|
||||
m(**h).should.equal?(h)
|
||||
m(**h).should_not.equal?(h)
|
||||
h.should == {a: 1}
|
||||
end
|
||||
end
|
||||
|
||||
it "copies the given Hash for a method taking (**kwargs)" do
|
||||
|
|
|
@ -237,16 +237,16 @@ class TestKeywordArguments < Test::Unit::TestCase
|
|||
assert_equal(true, Hash.ruby2_keywords_hash?(marked))
|
||||
end
|
||||
|
||||
def test_keyword_splat_new
|
||||
kw = {}
|
||||
h = {a: 1}
|
||||
|
||||
def self.assert_equal_not_same(kw, res)
|
||||
def assert_equal_not_same(kw, res)
|
||||
assert_instance_of(Hash, res)
|
||||
assert_equal(kw, res)
|
||||
assert_not_same(kw, res)
|
||||
end
|
||||
|
||||
def test_keyword_splat_new
|
||||
kw = {}
|
||||
h = {a: 1}
|
||||
|
||||
def self.yo(**kw) kw end
|
||||
m = method(:yo)
|
||||
assert_equal(false, yo(**{}).frozen?)
|
||||
|
@ -459,6 +459,20 @@ class TestKeywordArguments < Test::Unit::TestCase
|
|||
assert_equal_not_same h, yo(*a, **h)
|
||||
end
|
||||
|
||||
def test_keyword_splat_to_non_keyword_method
|
||||
h = {a: 1}.freeze
|
||||
|
||||
def self.yo(kw) kw end
|
||||
assert_equal_not_same(h, yo(**h))
|
||||
assert_equal_not_same(h, method(:yo).(**h))
|
||||
assert_equal_not_same(h, :yo.to_proc.(self, **h))
|
||||
|
||||
def self.yoa(*kw) kw[0] end
|
||||
assert_equal_not_same(h, yoa(**h))
|
||||
assert_equal_not_same(h, method(:yoa).(**h))
|
||||
assert_equal_not_same(h, :yoa.to_proc.(self, **h))
|
||||
end
|
||||
|
||||
def test_regular_kwsplat
|
||||
kw = {}
|
||||
h = {:a=>1}
|
||||
|
|
|
@ -609,6 +609,10 @@ setup_parameters_complex(rb_execution_context_t * const ec, const rb_iseq_t * co
|
|||
kw_flag &= ~(VM_CALL_KW_SPLAT | VM_CALL_KW_SPLAT_MUT);
|
||||
}
|
||||
else {
|
||||
if (!(kw_flag & VM_CALL_KW_SPLAT_MUT)) {
|
||||
converted_keyword_hash = rb_hash_dup(converted_keyword_hash);
|
||||
}
|
||||
|
||||
if (last_arg != converted_keyword_hash) {
|
||||
last_arg = converted_keyword_hash;
|
||||
args->argv[args->argc-1] = last_arg;
|
||||
|
|
Загрузка…
Ссылка в новой задаче