Issue #16154 has been updated by fieldtensor (Patrick Rutkowski).


jeremyevans0 (Jeremy Evans) wrote:
> The hash-flag approach for ruby2_keywords has been merged at 3b302ea8c95d34d5ef072d7e3b326f28a611e479.  That commit uses ruby2_keywords in delegate, fixing the keyword argument separation issues.

I ran into a warning when calling #write_nonblock on a Tempfile instance. Some digging led me to this issue page. When I saw this merge I assumed that the warning would go away if I switched my rbenv from 2.7.0 to 2.7.0-dev, but switching to 2.7.0-dev did even worse, it crashed:

```
$ RBENV_VERSION="2.6.5" ruby -v -r tempfile -e 'Tempfile.new.write_nonblock("hello\n", exception: false)'; echo; echo
ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-darwin18]


$ RBENV_VERSION="2.7.0" ruby -v -r tempfile -e 'Tempfile.new.write_nonblock("hello\n", exception: false)'; echo; echo
ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-darwin18]
/rbenv/versions/2.7.0/lib/ruby/2.7.0/delegate.rb:336: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
<internal:io>:120: warning: The called method `write_nonblock' is defined here


$ RBENV_VERSION="2.7.0-dev" ruby -v -r tempfile -e 'Tempfile.new.write_nonblock("hello\n", exception: false)'; echo; echo
ruby 2.8.0dev (2020-01-20T00:50:03Z master f31b90f2b9) [x86_64-darwin18]
Traceback (most recent call last):
    2: from -e:1:in `<main>'
        1: from /rbenv/versions/2.7.0-dev/lib/ruby/2.8.0/delegate.rb:336:in `block in delegating_block'
<internal:io>:120:in `write_nonblock': wrong number of arguments (given 2, expected 1) (ArgumentError)
```

Is this crash related to the issue here? Should this issue be reopened? Should I file a separate bug report?

----------------------------------------
Bug #16154: lib/delegate.rb issues keyword argument warnings
https://bugs.ruby-lang.org/issues/16154#change-83978

* Author: jeremyevans0 (Jeremy Evans)
* Status: Closed
* Priority: Normal
* Assignee: 
* Target version: 
* ruby -v: 
* Backport: 2.5: DONTNEED, 2.6: DONTNEED
----------------------------------------
In the current master branch, `lib/delegate.rb` (e.g. `Delegator`/`SimpleDelegator`/`DelegateClass`) does not delegate keyword arguments, leading to keyword argument warnings when using it.  It is simple to fix this by changing it to delegate keyword arguments, and I have a commit that does this (https://github.com/ruby/ruby/pull/2439/commits/30a70491b11d5235877a13d1a1d3fe0157960a64).

However, one issue with this approach, that you can notice at the bottom of the commit, is that it warns in a case where the behavior will not change between 2.6 and 3.0:

```ruby
foo = Object.new
def foo.bar(*args)
  args
end
d = SimpleDelegator.new(foo)
d.bar({a: 1})
# warns in 2.7, even though 2.6, 2.7, and 3.0 will return [{a: 1}]
```

The reason it warns even though the behavior will not change is that there is a behavior change, but the behavior change is inside the delegator.  When you call `d.bar`, the positional hash is converted to a keyword argument, which generates a warning.  When `d.bar` calls `foo.bar`, the keyword splat is converted back to a positional hash.  Because of this hash->keyword->hash conversion, the net result is behavior does not change outside the delegator, and therefore it would be great if we can somehow flag this particular case as not warning, while still warning in the case where `foo.bar` would accept keyword arguments (as in that case, behavior will change).

I came up with an implementation that makes delegate not emit the warning in the case where behavior does not change, but still emit the warning in the case where behavior will change (https://github.com/ruby/ruby/pull/2439/commits/6b7bd4f89143d692932dc89fadc341980816ee14).  This works by flagging a method that accepts keyword arguments.  If the flag is set for the method, and the method is passed a positional hash that is converted to a keyword argument, no warning is emitted, and instead a VM frame flag is set in the next frame so that keyword splats inside that method are converted to positional arguments.  The method flag is enabled by calling a `Module#pass_positional_hash` method, and this method would only exist in 2.7, since it is only for skipping this 2.7-specific warning.

I'm not sure this implementation approach is best, and would be open to any approach that allows delegate to not emit a warning for the code above, as long as it doesn't change the default keyword behavior in 2.7.

Attached is a patch that implements this, and there is also a pull request that has passed CI for it (https://github.com/ruby/ruby/pull/2439).

---Files--------------------------------
delegate-keyword-argument-separation.patch (19.1 KB)


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