Bug #20008
closedf(**kw, &block) calls block.to_proc before kw.to_hash
Description
The evaluation order expectation is that kw.to_hash
is called first in this case. Ruby previously called kw.to_hash
before block.to_proc
in Ruby 2, but stopped doing so in Ruby 3.0. In master, f(*a, **kw, &block)
is also affected. However, that is not true in Ruby 3.2 and earlier (though Ruby 2.0 and 2.1 called kw.to_hash
before a.to_a
).
The reason for the current behavior is that vm_caller_setup_arg_block
calls block.to_proc
before vm_sendish
is called. kw.to_hash
is not called until CALLER_SETUP_ARG
or setup_parameters_complex
.
I have a pull request that fixes this (https://github.com/ruby/ruby/pull/8877), by adding a splatkw
VM instruction and calling it directly before any send instructions that have both VM_CALL_ARGS_BLOCKARG
and VM_CALL_KW_SPLAT
and not VM_CALL_KW_SPLAT_MUT
. The splatkw
instruction calls kw.to_hash
if kw
is not already a Hash. The splatkw
instruction is not needed for mutable keyword splats, as kw.to_hash
would have already been called by earlier instructions in that case.
It is possible to fix this without a new VM instruction, by adding the logic to vm_caller_setup_arg_block
or rb_vm_send
. However, then an additional check is needed in every method call, instead of only before method calls that actually need it. I would guess that approach would be slower, though as I have not benchmarked that approach, it is not an educated guess.
Currently, the splatkw
instruction is inserted using the peephole optimizer. I'm not sure if using the optimizer to fix bugs is acceptable. If not, it could probably be moved to the compiler proper.
I do not believe we should backport this fix, as I consider a new VM instruction invasive, and the benefit of backporting seems small.
Example code:
o = Object.new
o.define_singleton_method(:to_hash) {p :called_to_hash; {}}
o.define_singleton_method(:to_proc) {p :called_to_proc; lambda{}}
puts "p(**o, &o)"
p(**o, &o)
Actual Output:
p(**o, &o)
:called_to_proc
:called_to_hash
Expected Output:
p(**o, &o)
:called_to_hash
:called_to_proc
Updated by jeremyevans0 (Jeremy Evans) 8 months ago
This was reviewed during yesterday's dev meeting and was found to be mostly OK, except for the use of the peephole optimizer to fix the issue. I moved the fix from the peephole optimizer to the compiler, which should make this acceptable. I found the same issue affected super
calls with explicit arguments, so I fixed those as well.
This does not affect yield
, because you cannot pass a block to yield
. It does not affect zsuper calls, as zsuper does not currently respect reassigning the block parameter. zsuper always uses the block passed to the original method, and does not call to_proc
on it.
This issue affects op asgn calls (e.g. foo[**kw, &block] = bar
, but those are compiled incorrectly anyway, passing **kw
as a kw
positional argument (no to_hash
conversion) to the []
and []=
methods, allowing code such as [1][**0] += 2
to work. I'll file a separate bug report for that.
Updated by jeremyevans0 (Jeremy Evans) 8 months ago
- Related to Bug #20051: Op asgn calls handle keywords and keyword splats as positional arguments added
Updated by jeremyevans0 (Jeremy Evans) 8 months ago
- Status changed from Open to Closed