From: "jeremyevans0 (Jeremy Evans) via ruby-core" Date: 2023-12-19T17:09:48+00:00 Subject: [ruby-core:115796] [Ruby master Bug#20008] f(**kw, &block) calls block.to_proc before kw.to_hash Issue #20008 has been updated by jeremyevans0 (Jeremy Evans). Status changed from Open to Closed Fixed by commit:a950f230788d51e13d16596e37cb77e4cc6e2311 and commit:f64357540eabad0f1bfaa6be60710d153325b064 ---------------------------------------- Bug #20008: f(**kw, &block) calls block.to_proc before kw.to_hash https://bugs.ruby-lang.org/issues/20008#change-105740 * Author: jeremyevans0 (Jeremy Evans) * Status: Closed * Priority: Normal * ruby -v: ruby 3.3.0dev (2023-11-12 master 60e19a0b5f) [x86_64-openbsd] * Backport: 3.0: UNKNOWN, 3.1: UNKNOWN, 3.2: UNKNOWN ---------------------------------------- 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: ```ruby 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 ``` -- https://bugs.ruby-lang.org/ ______________________________________________ ruby-core mailing list -- ruby-core@ml.ruby-lang.org To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org ruby-core info -- https://ml.ruby-lang.org/mailman3/postorius/lists/ruby-core.ml.ruby-lang.org/