Project

General

Profile

Actions

Bug #20008

closed

f(**kw, &block) calls block.to_proc before kw.to_hash

Added by jeremyevans0 (Jeremy Evans) about 1 year ago. Updated 11 months ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 3.3.0dev (2023-11-12 master 60e19a0b5f) [x86_64-openbsd]
[ruby-core:115412]

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

Related issues 1 (0 open1 closed)

Related to Ruby master - Bug #20051: Op asgn calls handle keywords and keyword splats as positional argumentsClosedActions
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0