Skip to content

Commit

Permalink
Ensure f(**kw, &block) calls kw.to_hash before block.to_proc
Browse files Browse the repository at this point in the history
Previously, block.to_proc was called first, by vm_caller_setup_arg_block.
kw.to_hash was called later inside CALLER_SETUP_ARG or setup_parameters_complex.

This adds a splatkw instruction that is inserted before sends with
ARGS_BLOCKARG and KW_SPLAT and without KW_SPLAT_MUT. This is not needed in the
KW_SPLAT_MUT case, because then you know the value is a hash, and you don't
need to call to_hash on it.

The splatkw instruction checks whether the second to top block is a hash,
and if not, replaces it with the value of calling to_hash on it (using
rb_to_hash_type).  As it is always before a send with ARGS_BLOCKARG and
KW_SPLAT, second to top is the keyword splat, and top is the passed block.
  • Loading branch information
jeremyevans committed Dec 9, 2023
1 parent c0b6ea7 commit a950f23
Show file tree
Hide file tree
Showing 4 changed files with 203 additions and 170 deletions.
3 changes: 3 additions & 0 deletions compile.c
Expand Up @@ -8827,6 +8827,9 @@ compile_call(rb_iseq_t *iseq, LINK_ANCHOR *const ret, const NODE *const node, co
flag |= VM_CALL_FCALL;
}

if ((flag & VM_CALL_ARGS_BLOCKARG) && (flag & VM_CALL_KW_SPLAT) && !(flag & VM_CALL_KW_SPLAT_MUT)) {
ADD_INSN(ret, line_node, splatkw);
}
ADD_SEND_R(ret, line_node, mid, argc, parent_block, INT2FIX(flag), keywords);

qcall_branch_end(iseq, ret, else_label, branches, node, line_node);
Expand Down
11 changes: 11 additions & 0 deletions insns.def
Expand Up @@ -527,6 +527,17 @@ splatarray
obj = vm_splat_array(flag, ary);
}

/* call to_hash on hash to keyword splat before converting block */
DEFINE_INSN
splatkw
()
(VALUE hash, VALUE block)
(VALUE obj, VALUE block)
// attr bool leaf = false; /* has rb_to_hash_type() */
{
obj = rb_to_hash_type(hash);
}

/* put new Hash from n elements. n must be an even number. */
DEFINE_INSN
newhash
Expand Down
17 changes: 17 additions & 0 deletions test/ruby/test_call.rb
Expand Up @@ -147,6 +147,23 @@ def self.f(*a, **kw)
assert_equal Hash, f(*[], **o).class
end

def test_kwsplat_block_order
o = Object.new
ary = []
o.define_singleton_method(:to_a) {ary << :to_a; []}
o.define_singleton_method(:to_hash) {ary << :to_hash; {}}
o.define_singleton_method(:to_proc) {ary << :to_proc; lambda{}}

def self.t(...) end

t(**o, &o)
assert_equal([:to_hash, :to_proc], ary)

ary.clear
t(*o, **o, &o)
assert_equal([:to_a, :to_hash, :to_proc], ary)
end

OVER_STACK_LEN = (ENV['RUBY_OVER_STACK_LEN'] || 150).to_i # Greater than VM_ARGC_STACK_MAX
OVER_STACK_ARGV = OVER_STACK_LEN.times.to_a.freeze

Expand Down

0 comments on commit a950f23

Please sign in to comment.