ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
* [ruby-core:94860] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
@ 2019-09-09 14:10 ` daniel
  2019-09-09 14:22 ` [ruby-core:94861] " merch-redmine
                   ` (23 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: daniel @ 2019-09-09 14:10 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been reported by Dan0042 (Daniel DeLorme).

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94861] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
  2019-09-09 14:10 ` [ruby-core:94860] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation? daniel
@ 2019-09-09 14:22 ` merch-redmine
  2019-09-09 17:09 ` [ruby-core:94865] " daniel
                   ` (22 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: merch-redmine @ 2019-09-09 14:22 UTC (permalink / raw)
  To: ruby-core

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


I haven't tested this, but I think it should work the same between 2.0 - 3.0 (assuming #16154 is accepted):

```ruby
class ProxyWithKW < BasicObject
  def initialize(target)
    @target = target
  end
  def method_missing(*a, **o, &b)
    if o.empty?
      @target.send(*a, &b)
    else
      @target.send(*a, **o, &b)
    end
  end
  pass_positional_hash(:method_missing) if respond_to?(:pass_positional_hash, false)
end
```

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94865] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
  2019-09-09 14:10 ` [ruby-core:94860] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation? daniel
  2019-09-09 14:22 ` [ruby-core:94861] " merch-redmine
@ 2019-09-09 17:09 ` daniel
  2019-09-09 19:36 ` [ruby-core:94872] " merch-redmine
                   ` (21 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: daniel @ 2019-09-09 17:09 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been updated by Dan0042 (Daniel DeLorme).


I've compiled your delegate-keyword-argument-separation branch to test this. BTW in your example it should be `respond_to?(:pass_positional_hash, true)`

The portability result is... Almost there, but not quite. In 2.6 and 2.7, `o.empty?` cannot distinguish between `foo()` and `foo({})`. This results in the discrepancies found below via this test script.

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

class ProxyNew < BasicObject
  def initialize(target)
    @target = target
  end
  def method_missing(*a, **o, &b)
    if o.empty?
      @target.send(*a, &b)
    else
      @target.send(*a, **o, &b)
    end rescue $!
  end
  pass_positional_hash(:method_missing) if respond_to?(:pass_positional_hash, true)
end

class Test
  def args(*a)   a  end
  def arg(a)     a  end
  def opts(**o)  o  end
  def arg0o(a=nil, **o) [a,o] end
  def arg1o(a, **o)     [a,o] end
end

e = Object.new
def e.write(*args) $stdout.print("warn! ") end
$stderr = e

opt = {}
hash = {k:42}
proxy = (ARGV.first=="old" ? ProxyOld : ProxyNew).new(Test.new)
p proxy.args(42)          
p proxy.arg(42)           
p proxy.arg({k:42})       
p proxy.arg({})           
p proxy.opts(k: 42)       
p proxy.arg0o(hash)       
p proxy.arg0o(hash, **{}) 
p proxy.arg0o(hash, **opt)
```

I've highlighted the two cases where we have the correct behavior in 3.0 and 2.6 ProxyOld, but not the two others.

| testcase                  | 2.6 ProxyOld    | 2.6 ProxyNew        | 2.7 ProxyNew                    | 3.0 ProxyNew   |
|---------------------------|-----------------|---------------------|---------------------------------|----------------|
| proxy.args(42)            | [42]            | [42]                | [42]                            | [42]           |
| proxy.arg(42)             | 42              | 42                  | 42                              | 42             |
| proxy.arg({k:42})         | {:k=>42}        | {:k=>42}            | {:k=>42}                        | {:k=>42}       |
| proxy.arg({})             | {}              | **error**           | **error**                       | {}             |
| proxy.opts(k: 42)         | {:k=>42}        | {:k=>42}            | {:k=>42}                        | {:k=>42}       |
| proxy.arg0o(hash)         | [nil, {:k=>42}] | [nil, {:k=>42}]     | warn! warn! [nil, {:k=>42}]     | [{:k=>42}, {}] |
| proxy.arg0o(hash, **{})   | [nil, {:k=>42}] | [nil, {:k=>42}]     | warn! warn! [nil, {:k=>42}]     | [{:k=>42}, {}] |
| proxy.arg0o(hash, **opt)  | [{:k=>42}, {}]  | **[nil, {:k=>42}]** | **warn! warn! [nil, {:k=>42}]** | [{:k=>42}, {}] |


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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94872] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (2 preceding siblings ...)
  2019-09-09 17:09 ` [ruby-core:94865] " daniel
@ 2019-09-09 19:36 ` merch-redmine
  2019-09-10 14:41 ` [ruby-core:94888] " daniel
                   ` (20 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: merch-redmine @ 2019-09-09 19:36 UTC (permalink / raw)
  To: ruby-core

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


Dan0042 (Daniel DeLorme) wrote:
> I've compiled your delegate-keyword-argument-separation branch to test this. BTW in your example it should be `respond_to?(:pass_positional_hash, true)`
> 
> The portability result is... Almost there, but not quite. In 2.6 and 2.7, `o.empty?` cannot distinguish between `foo()` and `foo({})`. This results in the discrepancies found below via this test script.

Thank you for working on this and providing the detailed example.  I think your analysis is correct.  We could handle this in 2.7 by introducing a method to check for whether positional->keyword hash conversion was done, but as we cannot change the behavior in 2.0-2.6, it looks like we cannot use the same method definition to delegate to all possible target method definitions with all possible arguments and be backwards compatible with <2.7.  For best compatibility, you will need separate method definitions for <2.7 and 2.7+:

```ruby
class Proxy < BasicObject
  def initialize(target)
    @target = target
  end
  if ::RUBY_VERSION < '2.7'
    def method_missing(*a, &b)
      @target.send(*a, &b) rescue $!
    end
  else
    def method_missing(*a, **o, &b)
      @target.send(*a, **o, &b) rescue $!
    end
    pass_positional_hash(:method_missing) if respond_to?(:pass_positional_hash, true)
  end
end

class Test
  def args(*a)   a  end
  def arg(a)     a  end
  def opts(**o)  o  end
  def arg0o(a=nil, **o) [a,o] end
  def arg1o(a, **o)     [a,o] end
end

e = Object.new
def e.write(*args) $stdout.print("warn! ") end
$stderr = e

opt = {}
hash = {k:42}
proxy =  Proxy.new(Test.new)
p proxy.args(42)
p proxy.arg(42)
p proxy.arg({k:42})
p proxy.arg({})
p proxy.opts(k: 42)
p proxy.arg0o(hash)
p proxy.arg0o(hash, **{})
p proxy.arg0o(hash, **opt)
```

This results in the following behavior for 2.6.4 and 2.7.0 with the delegate-keyword-argument-separation branch:

```
ruby 2.6.4p104    │ ruby 2.7.0-delegate-keyword-argument-separation
[42]              │ [42]
42                │ 42
{:k=>42}          │ {:k=>42}
{}                │ {}
{:k=>42}          │ {:k=>42}
[nil, {:k=>42}]   │ warn! warn! [nil, {:k=>42}]
[nil, {:k=>42}]   │ [{:k=>42}, {}]
[{:k=>42}, {}]    │ [{:k=>42}, {}]
```

The only behavior difference is for `**{}`, which was ignored by the parser in 2.6, and is not likely to appear in production code.  The only warning in 2.7 is for `proxy.arg0o(hash)`, which correctly warns in 2.7 as the behavior will change in 3.0 to be the same as `proxy.arg0o(hash, **opt)` in 2.6.

Note that could use metaprogramming to reduce the duplication:

```ruby
class Proxy < BasicObject
  def initialize(target)
    @target = target
  end

  kw = ", **kw" if ::RUBY_VERSION >= '2.7'
  class_eval(<<-END, __FILE__, __LINE__+1)
    def method_missing(*a#{kw}, &b)
      @target.send(*a#{kw}, &b) rescue $!
    end
  END
  pass_positional_hash(:method_missing) if respond_to?(:pass_positional_hash, true)
end
```

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94888] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (3 preceding siblings ...)
  2019-09-09 19:36 ` [ruby-core:94872] " merch-redmine
@ 2019-09-10 14:41 ` daniel
  2019-09-10 16:37 ` [ruby-core:94889] " merch-redmine
                   ` (19 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: daniel @ 2019-09-10 14:41 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been updated by Dan0042 (Daniel DeLorme).


Hmm, ok, that's what I was afraid of. I mean, it's not exactly a pretty solution. And it's not limited to method_missing; any method that accepts `*args` and forwards it to another method may have to be changed. Even if it doesn't have to be changed, it has to be checked, which is possibly even more work than a simple find-and-replace.

One example that comes to mind is in the stdlib; all the files in json/add/*.rb have something like this which may need to be updated:
```ruby
  def to_json(*args)
    as_json.to_json(*args)
  end
```
Of course since that's the stdlib we don't need the RUBY_VERSION check, but for rubygems.... the issue is really how much is there to fix? If it's a small amount, even an ugly-ish solution can be good enough.

Actually, this could be checked lexically to some extent...

[workworkwork]

I've compiled a list of the most popular gems by number of downloads. https://pastebin.com/MurhpP2j
And then installed the top 500. Including pre-installed gems and dependencies, this results in 679 gems (including a few multiple versions). https://pastebin.com/KajXEt7A
Then I search through all .rb files for methods that should be checked and/or updated to use the RUBY_VERSION check if they want to stay compatible with 2.6. https://pastebin.com/cmmjMDW8

Result:
in `gems/*/lib`:   1949 matches in 1095 files of 225 gems: https://pastebin.com/HNU1cZcD
in `gems/*/others`: 256 matches in 167 files of 63 gems: https://pastebin.com/eTciQAc0

That's... a **lot** more than I expected. And only with the top 500 gems. If you can check my methodology...

But if I'm correct, I think that the migration required for syntax-based keyword separation goes beyond "challenging" and into the realm of "unrealistic". Of course that's a decision for Matz to take...


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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94889] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (4 preceding siblings ...)
  2019-09-10 14:41 ` [ruby-core:94888] " daniel
@ 2019-09-10 16:37 ` merch-redmine
  2019-09-10 19:14 ` [ruby-core:94894] " daniel
                   ` (18 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: merch-redmine @ 2019-09-10 16:37 UTC (permalink / raw)
  To: ruby-core

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


Note that the `RUBY_VERSION` check is only needed in a subset of the cases.  In cases where the target method does not accept keyword arguments, no changes are needed (no need to introduce keyword arguments at all).  In cases where the last positional argument is not a hash (and even in many of those cases), the simpler code that doesn't require `RUBY_VERSION` will work.  The only time you really need the `RUBY_VERSION` check is for complete argument delegation to arbitrary methods with arbitrary arguments.

I took a brief look at some things caught by your methodology.  I decided to look at ActiveSupport since that is one of the more popular gems.  Here are a few examples of delegation and a discussion of whether changes are needed.

```ruby
# lib/active_support/values/time_zone.rb
    def local(*args)
      time = Time.utc(*args)
      ActiveSupport::TimeWithZone.new(nil, self, time)
    end

    def at(*args)
      Time.at(*args).utc.in_time_zone(self)
    end
```

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.  So these cases probably do not need to be updated.

```ruby
# lib/active_support/time_with_zone.rb
    def method_missing(sym, *args, &block)
      wrap_with_time_zone time.__send__(sym, *args, &block)
    rescue NoMethodError => e
      raise e, e.message.sub(time.inspect, inspect), e.backtrace
    end
```

This is pure delegation code to arbitrary methods.  However, I think `time` is an instance of `Time`, this will not require changes.  Now, users can define their own methods on Time in Ruby that accept keyword arguments, and if you want to handle that, you may want to update the code to handle keyword arguments.

```ruby
#lib/active_support/testing/setup_and_teardown.rb
        # Add a callback, which runs before <tt>TestCase#setup</tt>.
        def setup(*args, &block)
          set_callback(:setup, :before, *args, &block)
        end

        # Add a callback, which runs after <tt>TestCase#teardown</tt>.
        def teardown(*args, &block)
          set_callback(:teardown, :after, *args, &block)
        end
```

`set_callback` doesn't accept keyword arguments, it treats the positional arguments as an array.  So no changes are needed here.

```ruby
#lib/active_support/tagged_logging.rb
      def tagged(*tags)
        new_tags = push_tags(*tags)
        yield self
      ensure
        pop_tags(new_tags.size)
      end

    def tagged(*tags)
      formatter.tagged(*tags) { yield self }
    end
```

Here `tagged` at the bottom delegates all arguments to `tagged` at the top, which delegates all arguments to `push_tags`.  However, as `push_tags` does not accept keyword arguments, no changes are needed.

I tried your scanner using Sequel (in the 500 top gems).  143 matches in 63 files, none of which actually required changes.  There were a few changes needed in Sequel which the scanner didn't catch, involving passing option hashes to CSV using `**` instead of as a positional argument.

In general, I would not recommend using a lexical scanner to try to find cases to fix. Run the tests for the program/library, and fix any warnings.  Even for decent sized programs/libraries, it does not take very long to fix the related issues.  For many cases where you would have to fix issues, there are already other existing issues due to the implicit conversion between keyword and positional arguments.

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94894] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (5 preceding siblings ...)
  2019-09-10 16:37 ` [ruby-core:94889] " merch-redmine
@ 2019-09-10 19:14 ` daniel
  2019-09-10 20:09 ` [ruby-core:94895] " merch-redmine
                   ` (17 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: daniel @ 2019-09-10 19:14 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been updated by Dan0042 (Daniel DeLorme).


> Note that the `RUBY_VERSION` check is only needed in a subset of the cases.  In cases where the target method does not accept keyword arguments, no changes are needed

Yes, I know, that's exactly what I was saying.

But my point was that "Even if it doesn't have to be *changed*, it has to be *checked*, which is possibly even more work". Although now I realize that checking is easy if the tests are comprehensive enough. And by that I mean the tests have to cover not only `foo(42)` and `foo(42, flag:true)` but also (less obviously) `wrapfoo(42)` and `wrapfoo(42, flag:true)`. But even without tests you can get the warnings from production logs. The lexical analysis was just intended to get a rough idea but I think I got a bit too caught up.

> 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.

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
```

> 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.

---

Well, I hope everyone has comprehensive test suites.

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

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

Disclaimer: I may be a worrywart.

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94895] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (6 preceding siblings ...)
  2019-09-10 19:14 ` [ruby-core:94894] " daniel
@ 2019-09-10 20:09 ` merch-redmine
  2019-09-11 15:36 ` [ruby-core:94904] " daniel
                   ` (16 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: merch-redmine @ 2019-09-10 20:09 UTC (permalink / raw)
  To: ruby-core

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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94904] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (7 preceding siblings ...)
  2019-09-10 20:09 ` [ruby-core:94895] " merch-redmine
@ 2019-09-11 15:36 ` daniel
  2019-09-11 21:25 ` [ruby-core:94909] " merch-redmine
                   ` (15 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: daniel @ 2019-09-11 15:36 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been updated by Dan0042 (Daniel DeLorme).


> 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's the same situation as ruby methods used to be in. So *theoretically* C methods are susceptible to the same problem. But very very few C methods have a signature that would allow to trigger the bug. However, feast your eyes on the behavior of `warn()` (on ruby master)

```
>> warn({x:1})
ArgumentError (unknown keyword: :x)

>> warn({x:1}, uplevel:0)
(irb):4: warning: {:x=>1}

>> warn({x:1}, **{})

>> warn({x:1}, {})

>> warn({x:1}, {}, {})
{:x=>1}

>> warn({x:1}, {}, {}, {})
{:x=>1}
{}

>> warn({x:1}, {y:2}, {})
ArgumentError (unknown keyword: :y)

>> warn({x:1}, {y:2}, {}, {})
{:x=>1}
{:y=>2}
```

Of course this is all *extremely* unconventional usage and doesn't really deserve a fix. But I thought it was weird/interesting.

> The only time you really need the RUBY_VERSION check is for complete argument delegation to arbitrary methods with arbitrary arguments.

For reference, searching for only arbitrary delegation (`send`/`__send__` with `*args`) gives 179 matches in 158 files of 86 gems (not all of which will require kwargs) https://pastebin.com/HX3w5ngK
 
> The only behavior difference is for **{}, which was ignored by the parser in 2.6, and is not likely to appear in production code. 

Confirmed, not a single appearance in the 679 gems I installed.

> However, the changes are necessary to fix the problems with keyword arguments.

I still disagree with that, but that's another argument, another story ;-)




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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94909] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (8 preceding siblings ...)
  2019-09-11 15:36 ` [ruby-core:94904] " daniel
@ 2019-09-11 21:25 ` merch-redmine
  2019-09-11 23:20 ` [ruby-core:94912] " merch-redmine
                   ` (14 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: merch-redmine @ 2019-09-11 21:25 UTC (permalink / raw)
  To: ruby-core

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


After discussion with some other committers, I have an alternative approach in https://github.com/ruby/ruby/pull/2449.  This allows you to do the following:

```ruby
class Foo
  def foo(*args, &block)
    bar(*args, &block)
  end
  pass_keywords :foo if respond_to?(:pass_keywords, true)
end
```

Then if you call `foo` with keywords, keywords will be passed to `bar`.  This should allow for more backwards compatible behavior with older versions of Ruby.

Using a modified version of your example:

```ruby
class Proxy < BasicObject
  def initialize(target)
    @target = target
  end
  def method_missing(*a, &b)
    @target.send(*a, &b) rescue $!
  end
  pass_keywords(:method_missing) if respond_to?(:pass_keywords, true)
end

class Test
  def noarg() end
  def noarg_o(**o) o end
  def arg(a) a  end
  def arg_o(a, **o) [a,o] end
  def opt_arg(a=nil) a end
  def opt_arg_o(a=nil, **o) [a,o] end
  def args(*a) a end
  def args_o(*a, **o) [a, o] end
end

e = Object.new
def e.write(*args) $stdout.print("warn! ") end
$stderr = e

opt = {}
hash = {k:42}
proxy = Proxy.new(Test.new)
%i'noarg arg opt_arg args'.each do |m|
  [m, :"#{m}_o"].each do |meth|
    p ["#{meth}(opt)", proxy.send(meth, opt)]
    p ["#{meth}(hash)", proxy.send(meth, hash)]
    p ["#{meth}(**hash)", proxy.send(meth, **opt)]
    p ["#{meth}(**opt)", proxy.send(meth, **opt)]
    p ["#{meth}(hash, **opt)", proxy.send(meth, hash, **opt)]
    p ["#{meth}(hash, **hash)", proxy.send(meth, hash, **hash)]
  end
end
```

Here's a table describing the differences and warnings (differences in ArgumentError messages are ignored).  All differences are due to empty keyword splats not being passed as positional arguments in 2.7, and I would consider them at least undesired behavior in 2.6, if not an outright bug.

```
    Code                           2.6              2.7-pass_keywords
-----------------------------------------------------------------------------
  | noarg(opt)              | #<ArgE:(1:0)>   | #<ArgE:(1:0)>
  | noarg(hash)             | #<ArgE:(1:0)>   | #<ArgE:(1:0)>
 d| noarg(**opt)            | #<ArgE:(1:0)>   | nil
  | noarg(**hash)           | #<ArgE:(1:0)>   | #<ArgE:unknown keyword: :k>
  | noarg(hash, **opt)      | #<ArgE:(2:0)>   | #<ArgE:(1:0)>
  | noarg(hash, **hash)     | #<ArgE:(2:0)>   | #<ArgE:(1:0)>
w | noarg_o(opt)            | opt             | opt
w | noarg_o(hash)           | hash            | hash
  | noarg_o(**opt)          | opt             | opt
  | noarg_o(**hash)         | hash            | hash
  | noarg_o(hash, **opt)    | #<ArgE:(1:0)>   | #<ArgE:(1:0)>
  | noarg_o(hash, **hash)   | #<ArgE:(1:0)>   | #<ArgE:(1:0)>
  | arg(opt)                | opt             | opt
  | arg(hash)               | hash            | hash
w | arg(**opt)              | opt             | opt
  | arg(**hash)             | opt             | hash
 d| arg(hash, **opt)        | #<ArgE:(2:1)>   | hash
  | arg(hash, **hash)       | #<ArgE:(2:1)>   | #<ArgE:unknown keyword: :k>
  | arg_o(opt)              | [opt, opt]      | [opt, opt]
  | arg_o(hash)             | [hash, opt]     | [hash, opt]
w | arg_o(**opt)            | [opt, opt]      | [opt, opt]
w | arg_o(**hash)           | [hash, opt]     | [hash, opt]
  | arg_o(hash, **opt)      | [hash, opt]     | [hash, opt]
  | arg_o(hash, **hash)     | [hash, hash]    | [hash, hash]
  | opt_arg(opt)            | opt             | opt
  | opt_arg(hash)           | hash            | hash
 d| opt_arg(**opt)          | opt             | nil
  | opt_arg(**hash)         | hash            | hash
 d| opt_arg(hash, **opt)    | #<ArgE:(2:0..1)>| hash
  | opt_arg(hash, **hash)   | #<ArgE:(2:0..1)>| #<ArgE:unknown keyword: :k>
w | opt_arg_o(opt)          | [nil, opt]      | [nil, opt]
w | opt_arg_o(hash)         | [nil, hash]     | [nil, hash]
  | opt_arg_o(**opt)        | [nil, opt]      | [nil, opt]
  | opt_arg_o(**hash)       | [nil, hash]     | [nil, hash]
  | opt_arg_o(hash, **opt)  | [hash, opt]     | [hash, opt]
  | opt_arg_o(hash, **hash) | [hash, hash]    | [hash, hash]
  | args(opt)               | [opt]           | [opt]
  | args(hash)              | [hash]          | [hash]
 d| args(**opt)             | [opt]           | []
  | args(**hash)            | [hash]          | [hash]
 d| args(hash, **opt)       | [hash, opt]     | [hash]
  | args(hash, **hash)      | [hash, hash]    | [hash, hash]
w | args_o(opt)             | [[], opt]       | [[], opt]
w | args_o(hash)            | [[], hash]      | [[], hash]
  | args_o(**hash)          | [[], opt]       | [[], opt]
  | args_o(**opt)           | [[], opt]       | [[], opt]
  | args_o(hash, **opt)     | [[hash], opt]   | [[hash], opt]
  | args_o(hash, **hash)    | [[hash], hash]  | [[hash], hash]
```

Every time a warning is issued, behavior is the same in 2.7 as in 2.6. For each warning, here will be the behavior in 3.0:

```
    Code                 2.6 & 2.7           3.0
-------------------------------------------------------
w | noarg_o(opt)    | opt             | #<ArgE:(1:0)>
w | noarg_o(hash)   | hash            | #<ArgE:(1:0)>
w | arg(**opt)      | opt             | #<ArgE:(0:1)>
w | arg_o(**opt)    | [opt, opt]      | #<ArgE:(0:1)>
w | arg_o(**hash)   | [hash, opt]     | #<ArgE:(0:1)>
w | opt_arg_o(opt)  | [nil, opt]      | [opt, opt]
w | opt_arg_o(hash) | [nil, hash]     | [hash, opt]
w | args_o(opt)     | [[], opt]       | [[opt], opt]
w | args_o(hash)    | [[], hash]      | [[hash], opt]
```

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94912] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (9 preceding siblings ...)
  2019-09-11 21:25 ` [ruby-core:94909] " merch-redmine
@ 2019-09-11 23:20 ` merch-redmine
  2019-09-12  2:05 ` [ruby-core:94916] " daniel
                   ` (13 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: merch-redmine @ 2019-09-11 23:20 UTC (permalink / raw)
  To: ruby-core

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


Dan0042 (Daniel DeLorme) wrote:
> Of course this is all *extremely* unconventional usage and doesn't really deserve a fix. But I thought it was weird/interesting.

Very interesting actually.  However, while you think this method is written in C, it's actually not, at least not the version you are calling:

```
$ ruby --disable-gems -e 'warn({x: 1})'
$ ruby -e 'warn({x: 1})'
-e:1: warning: The last argument is used as the keyword parameter
/home/jeremy/tmp/ruby/lib/rubygems/core_ext/kernel_warn.rb:15: warning: for method defined here
Traceback (most recent call last):
        1: from -e:1:in `<main>'
/home/jeremy/tmp/ruby/lib/rubygems/core_ext/kernel_warn.rb:15:in `block in <module:Kernel>': unknown keyword: :x (ArgumentError)
```

Rubygems overwrites `Kernel#warn`! This is part of the implementation:

```ruby
    module_function define_method(:warn) {|*messages, uplevel: nil|
      unless uplevel
        return original_warn.call(*messages)
      end
      # filter callers
      original_warn.call(*messages, uplevel: uplevel)
    end
```

So the C version is loose with the keywords, in the sense that invalid keywords will be ignored. The Rubygems version is strict with the keywords, in that passing keywords other than :uplevel will result in an error.  The Rubygems version will then call the original version without keywords arguments.  Combined, this causes the very weird behavior you see.  Let's go through each example:

> ```
> >> warn({x:1})
> ArgumentError (unknown keyword: :x)
> ```

Happens when the Rubygems version is called, because :x is not a valid keyword.

> ```
> >> warn({x:1}, uplevel:0)
> (irb):4: warning: {:x=>1}
> ```

Rubygems version recognizes uplevel keyword and passes it to C version


> ```
> >> warn({x:1}, **{})
> ```

Passes no keywords to the Rubygems version, so Rubygems version passes `{x: 1}` as the sole argument to the C version, which is treated as keyword arguments and ignored.

> ```
> >> warn({x:1}, {})
> ```

This emits a keyword argument separation warning in the master branch, as the hash is treated as keywords to the Rubygems version.  The Rubygems version then passes `{x: 1}` as the sole argument to the C version, which is treated as keyword arguments and ignored.


> ```
> >> warn({x:1}, {}, {})
> {:x=>1}
> ```

Last hash passed to Rubygems version as keyword arguments with keyword argument separation warning emitted.  First two arguments passed to C version. Second argument treated as keyword argument by C version, and first argument treated as warning message.

> ```
> >> warn({x:1}, {}, {}, {})
> {:x=>1}
> {}
> ```

Same as previous, except there is an extra argument emitted as a warning.

> ```
> >> warn({x:1}, {y:2}, {})
> ArgumentError (unknown keyword: :y)
> ```

Last hash passed to Rubygems version as keyword arguments with keyword argument separation warning emitted.  First two arguments passed to C version. Second argument treated as keyword argument by C version, and ArgumentError raised as C version doesn't support the :y keyword.

> ```
> >> warn({x:1}, {y:2}, {}, {})
> {:x=>1}
> {:y=>2}
> ```

Same as the `warn({x:1}, {}, {}, {})` case other than the elements of the 2nd argument.

I sent a pull request to Rubygems to fix this behavior (https://github.com/rubygems/rubygems/pull/2911).  Personally, I think it would be better for them to drop the override of `Kernel#warn`.

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94916] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (10 preceding siblings ...)
  2019-09-11 23:20 ` [ruby-core:94912] " merch-redmine
@ 2019-09-12  2:05 ` daniel
  2019-09-12 12:53 ` [ruby-core:94922] " daniel
                   ` (12 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: daniel @ 2019-09-12  2:05 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been updated by Dan0042 (Daniel DeLorme).


> Then if you call `foo` with keywords, keywords will be passed to `bar`.  This should allow for more backwards compatible behavior with older versions of Ruby.

Ah, great! So we no longer need the RUBY_VERSION check!

I have a feeling the `pass_keywords` behavior would be appropriate for 99% of methods with an argument splat but no keywords. In that case maybe it would make things simpler to turn on the behavior by default and turn it off via `do_not_pass_keywords` or something? Just a thought.

> All differences are due to empty keyword splats not being passed as positional arguments in 2.7, and I would consider them at least undesired behavior in 2.6, if not an outright bug.

Maybe often undesired, but not always. Without that behavior, `opt_arg_o(hash, **opt)` would pass `hash` as kwarg in 2.6.


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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94922] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (11 preceding siblings ...)
  2019-09-12  2:05 ` [ruby-core:94916] " daniel
@ 2019-09-12 12:53 ` daniel
  2019-09-12 15:40 ` [ruby-core:94923] " merch-redmine
                   ` (11 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: daniel @ 2019-09-12 12:53 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been updated by Dan0042 (Daniel DeLorme).


After some more thinking I believe the `pass_keywords` behavior would be appropriate for 100% of methods with an argument splat but no keywords.

* If forwarding to a method with no keyword arguments
   * The kwarg will be converted to positional hash anyway
* If forwarding to a method with keyword arguments
   * If the intention is to pass a kwarg
      * Will work as expected
   * If the intention is to pass a positional hash
      * This behavior is currently impossible in 2.6, so there would be no compatibility problem. In order to activate the new behavior of passing a positional hash, use `pass_keywords_as_hash` or whatever the name would be.
     

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94923] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (12 preceding siblings ...)
  2019-09-12 12:53 ` [ruby-core:94922] " daniel
@ 2019-09-12 15:40 ` merch-redmine
  2019-09-12 19:57 ` [ruby-core:94924] " daniel
                   ` (10 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: merch-redmine @ 2019-09-12 15:40 UTC (permalink / raw)
  To: ruby-core

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


Dan0042 (Daniel DeLorme) wrote:
> After some more thinking I believe the `pass_keywords` behavior would be appropriate for 100% of methods with an argument splat but no keywords.

We do not want this as the default behavior.  Consider:

```ruby
def foo(*args)
  bar(*quux)
  baz(*args)
end
```

Here in the call to `bar`, the purpose is not to pass arguments through, but to call the method `quux` and splat the arguments received from that method as positional arguments.  If `bar` accepts a positional argument splat and keyword arguments, and `quux` returns an array where the last element is a hash, then the last element will be treated as keywords to `bar`, even though that is not what we want (these are the types of bugs keyword argument separation is designed to fix).

`pass_keywords` is designed only for arbitrary delegation methods, and only where backwards compatibility with Ruby <2.7 is desired.  It should eventually be removed after Ruby <2.7 is EOL.  As the implementation is based on VM frame flags, it requires extra internal work in the VM for every method call.

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94924] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (13 preceding siblings ...)
  2019-09-12 15:40 ` [ruby-core:94923] " merch-redmine
@ 2019-09-12 19:57 ` daniel
  2019-09-13 14:55 ` [ruby-core:94927] " daniel
                   ` (9 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: daniel @ 2019-09-12 19:57 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been updated by Dan0042 (Daniel DeLorme).


Ok, I misunderstood what `pass_keywords` was doing; I thought it would only apply to `baz(*args)` in this case.

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94927] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (14 preceding siblings ...)
  2019-09-12 19:57 ` [ruby-core:94924] " daniel
@ 2019-09-13 14:55 ` daniel
  2019-09-13 15:15 ` [ruby-core:94928] " merch-redmine
                   ` (8 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: daniel @ 2019-09-13 14:55 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been updated by Dan0042 (Daniel DeLorme).


There's something I'd like to clarify.

With 2.7, people will be asked to migrate their generic delegations to use `pass_keywords`. Then at some point in the future when 2.6 is no longer supported, `pass_keywords` will be deprecated and they'll be asked to migrate their generic delegations a second time, to use `(*args, **kw)`

Is that the idea?

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94928] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (15 preceding siblings ...)
  2019-09-13 14:55 ` [ruby-core:94927] " daniel
@ 2019-09-13 15:15 ` merch-redmine
  2019-09-13 17:00 ` [ruby-core:94931] " daniel
                   ` (7 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: merch-redmine @ 2019-09-13 15:15 UTC (permalink / raw)
  To: ruby-core

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


Dan0042 (Daniel DeLorme) wrote:
> With 2.7, people will be asked to migrate their generic delegations to use `pass_keywords`. Then at some point in the future when 2.6 is no longer supported, `pass_keywords` will be deprecated and they'll be asked to migrate their generic delegations a second time, to use `(*args, **kw)`
> 
> Is that the idea?

`pass_keywords` is proposed only to make it easier to continue to support older versions of Ruby with the same method definition.  I don't think we will be pushing people to use `pass_keywords`, merely making it available as an option.

You are correct that `pass_keywords` is currently envisioned as a temporary solution, not a permanent one.  However, there is nothing technically preventing it from being a permanent solution.

For 2.7+, delegation using `(*args, **kw)` should work the same way `(*args)` works in Ruby <2.7.  If you only want to migrate once and still want to support backwards compatibility, you can certainly switch to having separate definitions, using the current one for <2.7 and adding one for 2.7+.  Then you could drop the <2.7 definition when you drop support for Ruby <2.7.

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94931] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (16 preceding siblings ...)
  2019-09-13 15:15 ` [ruby-core:94928] " merch-redmine
@ 2019-09-13 17:00 ` daniel
  2019-09-13 17:54 ` [ruby-core:94933] " merch-redmine
                   ` (6 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: daniel @ 2019-09-13 17:00 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been updated by Dan0042 (Daniel DeLorme).


In that case I'd like to make one last suggestion.

For a method with `*args`, if the method is called with keyword arguments, flag `args` in some way (instance variable?) so that, in that method only, a call with `*args` would result in the last argument being passed as a keyword argument. Yes, it's a hack, but it's a better hack than `pass_keywords` imho.

This would make the migration less tedious, less confusing, less error-prone, and postpone this whole delegation mess to a date when it's easier to deal with. I really think this migration should be split into "should be done now" and "easier to do once 2.6 is no longer supported". My 2¢

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

* 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/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:94933] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (17 preceding siblings ...)
  2019-09-13 17:00 ` [ruby-core:94931] " daniel
@ 2019-09-13 17:54 ` merch-redmine
  2019-10-14 18:10 ` [ruby-core:95317] " merch-redmine
                   ` (5 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: merch-redmine @ 2019-09-13 17:54 UTC (permalink / raw)
  To: ruby-core

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


Dan0042 (Daniel DeLorme) wrote:
> In that case I'd like to make one last suggestion.
> 
> For a method with `*args`, if the method is called with keyword arguments, flag `args` in some way (instance variable?) so that, in that method only, a call with `*args` would result in the last argument being passed as a keyword argument. Yes, it's a hack, but it's a better hack than `pass_keywords` imho.

You can't make this an instance-specific flag and easily contain the scope to that method only.  The user could pass `args` (not `*args`) as a argument to another method and splat `args` in that method.

Also, using an instance-specific flag doesn't handle this type of delegation:

```ruby
def foo(*args)
  bar(*args.map{|arg| something(arg)})
end
```

> This would make the migration less tedious, less confusing, less error-prone, and postpone this whole delegation mess to a date when it's easier to deal with. I really think this migration should be split into "should be done now" and "easier to do once 2.6 is no longer supported". My 2¢

The approach you are proposing handles the simplest case fine and opens another Pandora's box of issues for other cases.  And if you want to split the migration into "should be done now" vs. "easier to do once 2.6 is no longer supported", that's exactly what `pass_keywords` allows.  You can use `pass_keywords` now, and once 2.6 is no longer supported, you can switch all delegation to `(*args, **kw)`.

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

* 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/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:95317] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (18 preceding siblings ...)
  2019-09-13 17:54 ` [ruby-core:94933] " merch-redmine
@ 2019-10-14 18:10 ` merch-redmine
  2019-10-15  5:37 ` [ruby-core:95326] " eregontp
                   ` (4 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: merch-redmine @ 2019-10-14 18:10 UTC (permalink / raw)
  To: ruby-core

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

Status changed from Open to Closed

`pass_keywords` was renamed to `ruby2_keywords`. The correct and portable way to handle generic delegation is now:

```ruby
class Proxy < BasicObject
  def initialize(target)
    @target = target
  end
  def method_missing(*a, &b)
    @target.send(*a, &b)
  end
  ruby2_keywords :method_missing if respond_to?(:ruby2_keywords, true)
end
```

Basically, keep your existing method definition, and after it add `ruby2_keywords :method_name if respond_to?(:ruby2_keywords, true)`.

That will work in all ruby versions until `ruby2_keywords` is removed. That is currently planned for after EOL of Ruby 2.6, but potentially it could stay longer.

I fixed `ruby2_keywords` so it works with `define_method`.  Note that generic delegation through procs is not currently implemented, but it is not difficult to add `Proc#ruby2_keywords` if it is needed.  Please request that as a feature if you want it.

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

* Author: Dan0042 (Daniel DeLorme)
* Status: Closed
* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:95326] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (19 preceding siblings ...)
  2019-10-14 18:10 ` [ruby-core:95317] " merch-redmine
@ 2019-10-15  5:37 ` eregontp
  2019-10-15  7:23 ` [ruby-core:95328] " eregontp
                   ` (3 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: eregontp @ 2019-10-15  5:37 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been updated by Eregon (Benoit Daloze).


@jeremyevans0 That is not future proof and will break as soon as `ruby2_keywords` is removed.

I don't think we want to keep using `ruby2_keywords` forever, i.e., does it make sense to use `ruby2_keywords` in Ruby 4?
From the name and my understanding of it, `ruby2_keywords` should only be used in 2.7 for transition, 3.0 later should explicitly pass kwargs.
I think using `ruby2_keywords` is also really not pretty, so I hope once we drop 2.x support we don't need such a workaround for delegation anymore.

So I believe this is the currently complete and future-proof way to do delegation that works on Ruby 2.x - 3.0+:

```ruby
class Proxy < BasicObject
  def initialize(target)
    @target = target
  end

  if RUBY_VERSION >= 3.0
    def method_missing(*args, **kwargs, &block)
      @target.send(*args, **kwargs, &block)
    end
  else
    def method_missing(*args, &block)
      @target.send(*args, &block)
    end
    ruby2_keywords :method_missing if respond_to?(:ruby2_keywords, true) # For 2.7
  end
end
```

We need to discuss when to remove `ruby2_keywords` before closing this issue, as it affects how we do the version check, so I reopen this.

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

* Author: Dan0042 (Daniel DeLorme)
* Status: Closed
* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:95328] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (20 preceding siblings ...)
  2019-10-15  5:37 ` [ruby-core:95326] " eregontp
@ 2019-10-15  7:23 ` eregontp
  2019-10-15 15:16 ` [ruby-core:95337] " merch-redmine
                   ` (2 subsequent siblings)
  24 siblings, 0 replies; 25+ messages in thread
From: eregontp @ 2019-10-15  7:23 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been updated by Eregon (Benoit Daloze).


At the end of my comment in https://bugs.ruby-lang.org/issues/16188#note-5 I discussed why I believe `ruby2_keywords` is a non-trivial cost on performance, and so should be IMHO only in Ruby 2.7.
I also propose there an explicit way to mark call sites converting the Hash flagged by `ruby2_keywords` to keywords again with `send_keyword_hash`.
That I think would make it easier to understand by being less magic and limit the performance cost to only where it's needed.

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:95337] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (21 preceding siblings ...)
  2019-10-15  7:23 ` [ruby-core:95328] " eregontp
@ 2019-10-15 15:16 ` merch-redmine
  2019-10-15 15:49 ` [ruby-core:95339] " eregontp
  2019-10-15 16:28 ` [ruby-core:95341] " merch-redmine
  24 siblings, 0 replies; 25+ messages in thread
From: merch-redmine @ 2019-10-15 15:16 UTC (permalink / raw)
  To: ruby-core

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


Eregon (Benoit Daloze) wrote:
> @jeremyevans0 That is not future proof and will break as soon as `ruby2_keywords` is removed.

I did say: `That will work in all ruby versions until ruby2_keywords is removed`.

> I don't think we want to keep using `ruby2_keywords` forever, i.e., does it make sense to use `ruby2_keywords` in Ruby 4?

It certainly could.  I believe the idea of removing it after Ruby 2.6 is EOL is that gems that work on supported Ruby versions could continue to have a single method definition.  That method definition may need to change if `ruby2_keywords` is removed, though.

> From the name and my understanding of it, `ruby2_keywords` should only be used in 2.7 for transition, 3.0 later should explicitly pass kwargs.

The idea behind `ruby2_keywords` is to be able to use a single method definition for delegation methods, that will work for older ruby versions (where `ruby2_keywords` is not defined and won't be called), and also newer versions (where `ruby2_keywords` is defined and will be called).  It is designed to require the fewest possible changes to existing code in order to get it to work in Ruby 2.7 without warnings and in Ruby 3.0 without errors.

> I think using `ruby2_keywords` is also really not pretty, so I hope once 2.x support is dropped libraries will not use the `ruby2_keywords` workaround for delegation anymore.
> 
> So I believe this is the currently complete and future-proof way to do delegation that works on Ruby 2.x - 3.0+:
> 
> ```ruby
> class Proxy < BasicObject
>   def initialize(target)
>     @target = target
>   end
> 
>   if RUBY_VERSION >= "3.0"
>     def method_missing(*args, **kwargs, &block)
>       @target.send(*args, **kwargs, &block)
>     end
>   else
>     def method_missing(*args, &block)
>       @target.send(*args, &block)
>     end
>     ruby2_keywords :method_missing if respond_to?(:ruby2_keywords, true) # For 2.7
>   end
> end
> ```

If you have an existing method:

```ruby
  def method_missing(*args, &block)
    @target.send(*args, &block)
  end
```

The idea with `ruby2_keywords` is that can currently just add a line so it works in 2.7+:

```ruby
  def method_missing(*args, &block)
    @target.send(*args, &block)
  end
  ruby2_keywords :method_missing if respond_to?(:ruby2_keywords, true)
```

If at some point we do remove `ruby2_keywords`, you could then switch it to:

```ruby
  def method_missing(*args, **kwargs, &block)
    @target.send(*args, **kwargs, &block)
  end
```

The assumption is by that point, the code will no longer need to support Ruby <2.7.  So at no point does the code need two method definitions to work with all supported Ruby versions.

> At the end of my comment in https://bugs.ruby-lang.org/issues/16188#note-5 I discussed why I believe `ruby2_keywords` is a non-trivial cost on performance, and so should be IMHO only in Ruby 2.7.

I'll try to respond to that today, but it is quite long.

> I also propose there an explicit way to mark call sites converting the Hash flagged by `ruby2_keywords` to keywords again with `send_keyword_hash`.

CRuby's C-API supports this.  It's not difficult to implement such a method.  However, this explicit approach requires more changes to existing code, which is sort of the opposite goal of `ruby2_keywords`.

> That I think would make it easier to understand by being less magic and limit the performance cost to only where it's needed.

It's more explicit at the call site where the conversion of hash to keywords takes place.  I'm not sure it is any easier to understand for the typical Ruby programmer.  

It also requires modifying the method definition.  The point of `ruby2_keywords` is you do not have to modify an existing method definition, you can just add `ruby2_keywords :method_name if respond_to?(:ruby2_keywords, true)` after the method definition, and things will continue to work as they did before.

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:95339] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (22 preceding siblings ...)
  2019-10-15 15:16 ` [ruby-core:95337] " merch-redmine
@ 2019-10-15 15:49 ` eregontp
  2019-10-15 16:28 ` [ruby-core:95341] " merch-redmine
  24 siblings, 0 replies; 25+ messages in thread
From: eregontp @ 2019-10-15 15:49 UTC (permalink / raw)
  To: ruby-core

Issue #16157 has been updated by Eregon (Benoit Daloze).


jeremyevans0 (Jeremy Evans) wrote:
> It certainly could.  I believe the idea of removing it after Ruby 2.6 is EOL is that gems that work on supported Ruby versions could continue to have a single method definition.  That method definition may need to change if `ruby2_keywords` is removed, though.

As we've seen countless times on bug trackers of many gems, many gems need to support Ruby versions which are EOL'd, because many applications exist and are not updated to the latest Ruby on every release.
In other words, I believe MRI version EOL is not a good deadline for this.
After all, there are other distributions than MRI which might EOL at different times or explicitly support older Ruby versions.

I believe removing `ruby2_keywords` is going to break those gems if we recommend them to entirely rely on `ruby2_keywords`.
And since currently nobody knows when `ruby2_keywords` would be removed, it's a time bomb that would need every such gem to change to fix it.
I doubt anyone wants that.
I'd bet most people would prefer a bit more verbose code that will work in the future than changing code for 2.7, knowing it will break again in the future.

Unless we never remove `ruby2_keywords`, but that doesn't make sense to me:
* It's a workaround for compatibility, we should not leave it in forever.
* It is modifying the behavior of things like `*args` which is inconsistent with other methods using `*args` but not using `ruby2_keywords`, creating inconsistency.
* We should encourage new code for Ruby 3.0+ to not use such a workaround, by simply not be able to use it.
* It's slowing down every single call with a `*rest` argument and without explicit keywords.

I think it's not a good idea to propose a migration path that we know will need to change again in the future.

To the best of our abilities, I believe it is our responsibility to provide a clear replacement that will work in all future Ruby versions.
So Rubyists don't need to modify their code a second time and they don't have to drop compatibility with Ruby < 3 at the same time as changing the code for the second time.

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [ruby-core:95341] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation?
       [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
                   ` (23 preceding siblings ...)
  2019-10-15 15:49 ` [ruby-core:95339] " eregontp
@ 2019-10-15 16:28 ` merch-redmine
  24 siblings, 0 replies; 25+ messages in thread
From: merch-redmine @ 2019-10-15 16:28 UTC (permalink / raw)
  To: ruby-core

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


Eregon (Benoit Daloze) wrote:
> jeremyevans0 (Jeremy Evans) wrote:
> > It certainly could.  I believe the idea of removing it after Ruby 2.6 is EOL is that gems that work on supported Ruby versions could continue to have a single method definition.  That method definition may need to change if `ruby2_keywords` is removed, though.
> 
> As we've seen countless times on bug trackers of many gems, many gems need to support Ruby versions which are EOL'd, because many applications exist and are not updated to the latest Ruby on every release.
> In other words, I believe MRI version EOL is not a good deadline for this.
> After all, there are other distributions than MRI which might EOL at different times or explicitly support older Ruby versions.

That's a possible argument for keeping `ruby2_keywords` longer or indefinitely. :)

> I believe removing `ruby2_keywords` is going to break those gems if we recommend them to entirely rely on `ruby2_keywords`.
> And since currently nobody knows when `ruby2_keywords` would be removed, it's a time bomb that would need every such gem to change to fix it.
> I doubt anyone wants that.
> I'd bet most people would prefer a bit more verbose code that will work in the future than changing code for 2.7, knowing it will break again in the future.

Certainly people that want that can already write separate method definitions depending on Ruby version.

As a counterpoint, most popular gems are well maintained and many of those only aim for compatibility with supported Ruby versions.  So using `ruby2_keywords` when it is available and switching away from it if it is removed may be easier for them.

Gems that aren't maintained may have issues in Ruby 3 anyway, due to keyword argument separation. Do you expect there will be a high percentage of gems that are maintained enough to add `ruby2_keywords` but will not be maintained enough to switch if it is removed?

> Unless we never remove `ruby2_keywords`, but that doesn't make sense to me:
> * It's a workaround for compatibility, we should not leave it in forever.

This depends on how much you value compatibility with older versions.  If you want compatibility with older versions, and don't want to define separate methods per version, you would want to keep `ruby2_keywords` defined.

> * It is modifying the behavior of things like `*args` which is inconsistent with other methods using `*args` but not using `ruby2_keywords`, creating inconsistency.

It is true that it results in inconsistency, and can result in behavior changes far away from the method using `ruby2_keywords`.  This is one reason it should be explicit and not the default behavior as some other people have proposed.

> * We should encourage new code for Ruby 3.0+ to not use such a workaround, by simply not be able to use it.

Then you are forcing separate method definitions per version if you want to support Ruby 2.6 and 3.0.  `ruby2_keywords` is designed to avoid that.

> * It's slowing down every single call with a `*rest` argument and without explicit keywords.

I believe the slow down is insignificant compared to all the other processing going on (in CRuby).  If you have benchmark results that prove otherwise, I would like to review them.

> I think it's not a good idea to propose a migration path that we know will need to change again in the future.
>
> To the best of our abilities, I believe it is our responsibility to provide a clear replacement that will work in all future Ruby versions.
> So Rubyists don't need to modify their code a second time and they don't have to drop compatibility with Ruby < 3 at the same time as changing the code for the second time.

I think that is fair.  However, the best solution if we don't want to change again in the future is to support `ruby2_keywords` indefinitely.  Any other approach requires separate methods definitions depending on the version.

I should also point out that the `ruby2_keywords` approach is even better at backwards compatibility then your proposed separate method definitions, since it works probably back to at least 1.6, whereas with the separate method definitions, it only works as far back as 2.0 (since keyword arguments are a syntax error in Ruby <2).

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

* 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/

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2019-10-15 16:28 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <redmine.issue-16157.20190909141042@ruby-lang.org>
2019-09-09 14:10 ` [ruby-core:94860] [Ruby master Misc#16157] What is the correct and *portable* way to do generic delegation? daniel
2019-09-09 14:22 ` [ruby-core:94861] " merch-redmine
2019-09-09 17:09 ` [ruby-core:94865] " daniel
2019-09-09 19:36 ` [ruby-core:94872] " merch-redmine
2019-09-10 14:41 ` [ruby-core:94888] " daniel
2019-09-10 16:37 ` [ruby-core:94889] " merch-redmine
2019-09-10 19:14 ` [ruby-core:94894] " daniel
2019-09-10 20:09 ` [ruby-core:94895] " merch-redmine
2019-09-11 15:36 ` [ruby-core:94904] " daniel
2019-09-11 21:25 ` [ruby-core:94909] " merch-redmine
2019-09-11 23:20 ` [ruby-core:94912] " merch-redmine
2019-09-12  2:05 ` [ruby-core:94916] " daniel
2019-09-12 12:53 ` [ruby-core:94922] " daniel
2019-09-12 15:40 ` [ruby-core:94923] " merch-redmine
2019-09-12 19:57 ` [ruby-core:94924] " daniel
2019-09-13 14:55 ` [ruby-core:94927] " daniel
2019-09-13 15:15 ` [ruby-core:94928] " merch-redmine
2019-09-13 17:00 ` [ruby-core:94931] " daniel
2019-09-13 17:54 ` [ruby-core:94933] " merch-redmine
2019-10-14 18:10 ` [ruby-core:95317] " merch-redmine
2019-10-15  5:37 ` [ruby-core:95326] " eregontp
2019-10-15  7:23 ` [ruby-core:95328] " eregontp
2019-10-15 15:16 ` [ruby-core:95337] " merch-redmine
2019-10-15 15:49 ` [ruby-core:95339] " eregontp
2019-10-15 16:28 ` [ruby-core:95341] " merch-redmine

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).