Project

General

Profile

Actions

Bug #16339

closed

Forwardable#def_delegator warns and is incorrect on trunk when passed keyword arguments

Added by Eregon (Benoit Daloze) over 4 years ago. Updated over 4 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 2.7.0dev (2019-11-10T05:56:38Z master 4570284ce1) [x86_64-linux]
[ruby-core:95785]

Description

Because it uses *args, &block-style delegation.

require 'forwardable'

class C
  extend Forwardable
  def_delegator :self, :target, :delegate
  def target(*args, **kwargs)
    [args, kwargs]
  end
end

p C.new.target(1, b: 2)   # => [[1], {:b=>2}]
p C.new.delegate(1, b: 2)
# ruby-trunk/lib/ruby/2.7.0/forwardable.rb:231: warning: The last argument is used as the keyword parameter
# del.rb:6: warning: for `target' defined here
# => [[1], {:b=>2}]

p C.new.target({}, **{}) # => [[{}], {}]
p C.new.delegate({}, **{})
# ruby-trunk/lib/ruby/2.7.0/forwardable.rb:231: warning: The last argument is used as the keyword parameter
# del.rb:6: warning: for `target' defined here
# => [[], {}]

Which also illustrates we're missing important tests/specs for Forwardable.

Updated by jeremyevans0 (Jeremy Evans) over 4 years ago

forwardable should be fixed to use ruby2_keywords. I'm guessing the reason this wasn't done initially is because I worked on forwardable keyword argument support before I developed ruby2_keywords.

Updated by Eregon (Benoit Daloze) over 4 years ago

2.6 behaves like:

p C.new.target({}, **{})   # => [[], {}]
p C.new.delegate({}, **{}) # => [[], {}]

So I'm not sure what's "correct" for that last case.

jeremyevans0 (Jeremy Evans) wrote:

forwardable should be fixed to use ruby2_keywords.

Right, or ... maybe.

I'm guessing the reason this wasn't done initially is because I worked on forwardable keyword argument support before I developed ruby2_keywords.

Didn't forwardable always support keyword arguments since it just did (*args, &block) delegation?

Updated by jeremyevans0 (Jeremy Evans) over 4 years ago

Looks like the forwardable keyword argument support was never merged. My previous attempt, developed before ruby2_keywords, added **kw to method definition and method calls on 2.7+. I've updated the pull request to use ruby2_keywords: https://github.com/ruby/forwardable/pull/5 . One failing CI test, but it looks unrelated. This should probably be merged soon, before preview3.

Updated by jeremyevans0 (Jeremy Evans) over 4 years ago

  • Status changed from Open to Closed
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0