mirror of
https://github.com/ruby/ruby.git
synced 2022-11-09 12:17:21 -05:00
Dup splat array in certain cases where there is a block argument
This makes: ```ruby args = [1, 2, -> {}]; foo(*args, &args.pop) ``` call `foo` with 1, 2, and the lambda, in addition to passing the lambda as a block. This is different from the previous behavior, which passed the lambda as a block but not as a regular argument, which goes against the expected left-to-right evaluation order. This is how Ruby already compiled arguments if using leading arguments, trailing arguments, or keywords in the same call. This works by disabling the optimization that skipped duplicating the array during the splat (splatarray instruction argument switches from false to true). In the above example, the splat call duplicates the array. I've tested and cases where a local variable or symbol are used do not duplicate the array, so I don't expect this to decrease the performance of most Ruby programs. However, programs such as: ```ruby foo(*args, &bar) ``` could see a decrease in performance, if `bar` is a method call and not a local variable. This is not a perfect solution, there are ways to get around this: ```ruby args = Struct.new(:a).new([:x, :y]) def args.to_a; a; end def args.to_proc; a.pop; ->{}; end foo(*args, &args) # calls foo with 1 argument (:x) # not 2 arguments (:x and :y) ``` A perfect solution would require completely disabling the optimization. Fixes [Bug #16504] Fixes [Bug #16500]
This commit is contained in:
parent
6439c939db
commit
aae8223c70
Notes:
git
2020-06-19 00:19:56 +09:00
3 changed files with 39 additions and 10 deletions
|
@ -5172,12 +5172,12 @@ setup_args(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
|
||||||
{
|
{
|
||||||
VALUE ret;
|
VALUE ret;
|
||||||
if (argn && nd_type(argn) == NODE_BLOCK_PASS) {
|
if (argn && nd_type(argn) == NODE_BLOCK_PASS) {
|
||||||
|
unsigned int dup_rest = 1;
|
||||||
DECL_ANCHOR(arg_block);
|
DECL_ANCHOR(arg_block);
|
||||||
INIT_ANCHOR(arg_block);
|
INIT_ANCHOR(arg_block);
|
||||||
NO_CHECK(COMPILE(arg_block, "block", argn->nd_body));
|
NO_CHECK(COMPILE(arg_block, "block", argn->nd_body));
|
||||||
|
|
||||||
*flag |= VM_CALL_ARGS_BLOCKARG;
|
*flag |= VM_CALL_ARGS_BLOCKARG;
|
||||||
ret = setup_args_core(iseq, args, argn->nd_head, 0, flag, keywords);
|
|
||||||
|
|
||||||
if (LIST_INSN_SIZE_ONE(arg_block)) {
|
if (LIST_INSN_SIZE_ONE(arg_block)) {
|
||||||
LINK_ELEMENT *elem = FIRST_ELEMENT(arg_block);
|
LINK_ELEMENT *elem = FIRST_ELEMENT(arg_block);
|
||||||
|
@ -5186,8 +5186,10 @@ setup_args(rb_iseq_t *iseq, LINK_ANCHOR *const args, const NODE *argn,
|
||||||
if (iobj->insn_id == BIN(getblockparam)) {
|
if (iobj->insn_id == BIN(getblockparam)) {
|
||||||
iobj->insn_id = BIN(getblockparamproxy);
|
iobj->insn_id = BIN(getblockparamproxy);
|
||||||
}
|
}
|
||||||
|
dup_rest = 0;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
ret = setup_args_core(iseq, args, argn->nd_head, dup_rest, flag, keywords);
|
||||||
ADD_SEQ(args, arg_block);
|
ADD_SEQ(args, arg_block);
|
||||||
}
|
}
|
||||||
else {
|
else {
|
||||||
|
|
|
@ -421,18 +421,36 @@ describe "Invoking a method" do
|
||||||
specs.rest_len(0,*a,4,*5,6,7,*c,-1).should == 11
|
specs.rest_len(0,*a,4,*5,6,7,*c,-1).should == 11
|
||||||
end
|
end
|
||||||
|
|
||||||
it "expands the Array elements from the splat after executing the arguments and block if no other arguments follow the splat" do
|
ruby_version_is ""..."2.8" do
|
||||||
def self.m(*args, &block)
|
it "expands the Array elements from the splat after executing the arguments and block if no other arguments follow the splat" do
|
||||||
[args, block]
|
def self.m(*args, &block)
|
||||||
|
[args, block]
|
||||||
|
end
|
||||||
|
|
||||||
|
args = [1, nil]
|
||||||
|
m(*args, &args.pop).should == [[1], nil]
|
||||||
|
|
||||||
|
args = [1, nil]
|
||||||
|
order = []
|
||||||
|
m(*(order << :args; args), &(order << :block; args.pop)).should == [[1], nil]
|
||||||
|
order.should == [:args, :block]
|
||||||
end
|
end
|
||||||
|
end
|
||||||
|
|
||||||
args = [1, nil]
|
ruby_version_is "2.8" do
|
||||||
m(*args, &args.pop).should == [[1], nil]
|
it "expands the Array elements from the splat before applying block argument operations" do
|
||||||
|
def self.m(*args, &block)
|
||||||
|
[args, block]
|
||||||
|
end
|
||||||
|
|
||||||
args = [1, nil]
|
args = [1, nil]
|
||||||
order = []
|
m(*args, &args.pop).should == [[1, nil], nil]
|
||||||
m(*(order << :args; args), &(order << :block; args.pop)).should == [[1], nil]
|
|
||||||
order.should == [:args, :block]
|
args = [1, nil]
|
||||||
|
order = []
|
||||||
|
m(*(order << :args; args), &(order << :block; args.pop)).should == [[1, nil], nil]
|
||||||
|
order.should == [:args, :block]
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
it "evaluates the splatted arguments before the block if there are other arguments after the splat" do
|
it "evaluates the splatted arguments before the block if there are other arguments after the splat" do
|
||||||
|
|
|
@ -99,4 +99,13 @@ class TestCall < Test::Unit::TestCase
|
||||||
ary = [1, 2]
|
ary = [1, 2]
|
||||||
assert_equal([0, 1, 2, 1], aaa(0, *ary, ary.shift), bug12860)
|
assert_equal([0, 1, 2, 1], aaa(0, *ary, ary.shift), bug12860)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_call_block_order
|
||||||
|
bug16504 = '[ruby-core:96769] [Bug# 16504]'
|
||||||
|
b = proc{}
|
||||||
|
ary = [1, 2, b]
|
||||||
|
assert_equal([1, 2, b], aaa(*ary, &ary.pop), bug16504)
|
||||||
|
ary = [1, 2, b]
|
||||||
|
assert_equal([0, 1, 2, b], aaa(0, *ary, &ary.pop), bug16504)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in a new issue