[ruby-core:94895] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?

[merch-redmine@jeremyevans.net]

Issue #16157 has been updated by jeremyevans0 (Jeremy Evans).


Dan0042 (Daniel DeLorme) wrote:
> > The only time you really need the `RUBY_VERSION` check is for complete argument delegation to arbitrary methods with arbitrary arguments.
> 
> Ah yes, I made a mistake there. So in my example if `to_json(*args)` outputs a warning, it's ok to change it to `to_json(*args,**kw)` even in 2.6 since it's very unlikely you'd be delegating to two different `to_json` methods, one with kwargs and one without.

Let's say the target method is `foo(*args, **kw)`, and the delegating method is `bar(*args) foo(*args) end`.  If you get a keyword argument separation warning when calling `foo` from `bar` in 2.7, then changing to `bar(*args, **kw) foo(*args, **kw) end` should fix the warning, be forward compatible with 3.0, and be backwards compatible back to 2.0.  You'll still need to fix callers of `bar` if they are passing a positional hash as the final positional argument and are not passing keyword arguments, but there will be separate warnings for that.

Note that a method that does not accept keyword arguments will never issue a warning.  For methods that do not accept keyword arguments, the keyword arguments are converted to a positional hash argument.  This is for backwards compatibility at least back to Ruby 1.6.  So you will only get a warning if the called method accepts keywords.

> So the migration procedure looks like this I think?
> ```
> if you get a warning
>    if you are delegating to a specific method
>       use (*args, **kw)
>    else
>       check RUBY_VERSION to delegate via (*args) or (*args, **kw)
> else
>    don't change anything, otherwise it will break on 2.6
> ```

That seems reasonable.  If you are only delegating to a specific method, then you only get the warning if the target method accepts keywords, in which case accepting and passing the keyword argument should work.

> > Most methods written in C do not care if they are called with keyword arguments or a positional hash argument and will work with either.
> 
> Wow, really? This is a bit off-topic but can you explain why C methods have no trouble with the hash/keyword ambiguity? I would have assumed it was the same as with ruby methods.

C functions for Ruby methods that accept keywords must accept the following arguments (since keywords are optional):

```
VALUE c_function_name(int argc, VALUE *argv, VALUE self)
```

`self` is the method receiver in Ruby, `argc` is the number of arguments passed in Ruby (with keywords counted as a positional hash), and `argv` is a pointer such that `argv[0]`...`argv[argc-1]` are the arguments passed in Ruby.

Prior to the keyword argument separation branch merge, it was not even possible for a C method to tell if it was called with keywords or called with a positional hash.  That is now possible via `rb_keyword_given_p`, as it is necessary for C methods if they are delegating arguments to another method (e.g. `Class#new`, `Method#call`, `Object#to_enum`).

> Well, I hope everyone has comprehensive test suites.

Agreed.

> I hope everyone will understand that just adding `**kw` can result in bugs on 2.6.

Me too.  The migration would have been much easier if `**kw` did not send an empty positional argument to methods that do not accept keyword arguments in older Ruby versions.  Alas, you cannot change the past.

> I hope this migration will go as smoothly as you think it will.

I didn't mean to imply I think the migration will go smoothly.  This is definitely going to require work, especially for large projects, and it may be error prone if the projects lack comprehensive testing.  However, the changes are necessary to fix the problems with keyword arguments.

Based on my experience so far (sample size=1), the hardest part of this migration will be convincing gem maintainers that it is worthwhile to have a version that will work correctly in 2.6, 2.7, and 3.0.  The actual work to update the code will be less difficult in comparison.

----------------------------------------
Misc #16157: What is the correct and *portable* way to do generic delegation?
https://bugs.ruby-lang.org/issues/16157#change-81510

* Author: Dan0042 (Daniel DeLorme)
* Status: Open
* Priority: Normal
* Assignee: 
----------------------------------------
With the keyword argument changes in 2.7 we must now specify keyword arguments explicitly when doing generic delegation. But this change is not compatible with 2.6, where it adds an empty hash to the argument list of methods that do not need/accept keyword arguments.

To illustrate the problem:

```ruby
class ProxyWithoutKW < BasicObject
  def initialize(target)
    @target = target
  end
  def method_missing(*a, &b)
    @target.send(*a, &b)
  end
end

class ProxyWithKW < BasicObject
  def initialize(target)
    @target = target
  end
  def method_missing(*a, **o, &b)
    @target.send(*a, **o, &b)
  end
end

class Test
  def args(*a)   a  end
  def arg(a)     a  end
  def opts(**o)  o  end
end
                                          # 2.6        2.7              3.0
ProxyWithoutKW.new(Test.new).args(42)     # [42]       [42]             [42]        ok
ProxyWithoutKW.new(Test.new).arg(42)      # 42         42               42          ok
ProxyWithoutKW.new(Test.new).opts(k: 42)  # {:k=>42}   {:k=>42} +warn   [{:k=>42}]  incompatible with >= 2.7
ProxyWithKW.new(Test.new).args(42)        # [42, {}]   [42]             [42]        incompatible with <= 2.6
ProxyWithKW.new(Test.new).arg(42)         # error      42               42          incompatible with <= 2.6
ProxyWithKW.new(Test.new).opts(k: 42)     # {:k=>42}   {:k=>42} +warn   {:k=>42}    must ignore warning? cannot use pass_positional_hash in 2.6
```

I don't know how to solve this, so I'm asking for the **official** correct way to write portable delegation code. And by **portable** I mean code that can be used in gems that target ruby 2.6 and above.




-- 
https://bugs.ruby-lang.org/