Backport #8072

Method#arity for keyword arguments

Added by Marc-Andre Lafortune about 1 year ago. Updated 2 months ago.

[ruby-core:53298]
Status:Closed
Priority:Normal
Assignee:Marc-Andre Lafortune

Description

I would expect the following two methods to have the same arity:

def old_way(req, options = {}); end
def new_way(req, **options); end

method(:new_way).arity # => 1, should be -2

Related issues

Related to Backport21 - Backport #9299: Required keyowrd arguments and arity Open 12/26/2013

Associated revisions

Revision 45138
Added by Yui NARUSE 2 months ago

merge revision(s) 44412,44413,44414,44420,44421: [Backport #9298]

test_method.rb, test_proc.rb: suppress warnings

* test/ruby/test_method.rb: suppress warnings in verbose mode.

* test/ruby/test_proc.rb: ditto.
* proc.c (rb_iseq_min_max_arity): maximum argument is unlimited if
  having rest keywords argument.   [Bug #8072]

* iseq.c (rb_iseq_parameters): push argument type symbol only for
  unnamed rest keywords argument.

* compile.c (iseq_set_arguments): set arg_keyword_check from
  nd_cflag, which is set by parser.  internal ID is used for
  unnamed keyword rest argument, which should be separated from no
  keyword check.

* iseq.c (rb_iseq_parameters): if no keyword check, keyword rest is
  present.

* parse.y (new_args_tail_gen): set keywords check to nd_cflag, which
  equals to that keyword rest is not present.

Revision 45231
Added by Yui NARUSE about 1 month ago

merge revision(s) 44432: [Backport #9299]

* proc.c: Having optional keyword arguments makes maximum arity +1,
  not unlimited [#8072]

History

#1 Updated by Boris Stitnicky 11 months ago

+1

Ruby has outgrown #arity. Today, plain #arity should only return positive value,
when the function has only compulsory ordered arguments. If there are any other
arguments (optional ordered, named, splat-collected...), #arity should return a
negative value, meaning "Sorry, I give up, the negative integer I am returning
does not fully describe this function's arguments."

How about giving #arity an optional argument with terminology like in #parameters?

f = -> a, b, c=1, *d, e: 3, f: 4, **g {}
f.parameters
#=> [[:req, :a], [:req, :b], [:opt, :c], [:rest, :d], [:key, :e], [:key, :f], [:keyrest, :g]]
f.arity #=> -3
f.arity( :req ) #=> 2
f.arity( :opt ) #=> 1
f.arity( :rest ) #=> 1
f.arity( :key ) #=> 2
f.arity( :keyrest ) #=> 1

g = -> compulsory_ordered_argument, **splat_collected_named_arguments {}
g.arity #=> -2
g.arity( :req ) #=> 1
g.arity( :opt ) #=> 0
g.arity( :rest ) #=> 0
g.arity( :key ) #=> 0
g.arity( :keyrest ) #=> 1

I also think, that #arity behavior for lambdas and procs should be the same.

#2 Updated by Nobuyoshi Nakada 4 months ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r44413.
Marc-Andre, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


proc.c: fix arity of rest keywords argument

  • proc.c (rbiseqminmaxarity): maximum argument is unlimited if having rest keywords argument. [Bug #8072]

#3 Updated by Tomoyuki Chikanaga 4 months ago

  • Backport set to 1.9.3: DONTNEED, 2.0.0: REQUIRED

#4 Updated by Marc-Andre Lafortune 4 months ago

  • Status changed from Closed to Open
  • Assignee changed from Yukihiro Matsumoto to Yui NARUSE
  • Backport changed from 1.9.3: DONTNEED, 2.0.0: REQUIRED to 1.9.3: DONTNEED, 2.0.0: REQUIRED, 2.1.0: REQUIRED

I don't think this fix is correct. A method/proc having keyword arguments should be considered as having one optional argument, not infinitely many. Among other things:

proc{|options = {}|}.arity == proc{|**options|}.arity # => should be true

I've committed r44432 to fix this.

#5 Updated by Marc-Andre Lafortune 4 months ago

  • Tracker changed from Bug to Backport
  • Project changed from ruby-trunk to Backport21
  • Category deleted (core)
  • Target version deleted (2.1.0)

#6 Updated by Yui NARUSE 4 months ago

  • Assignee changed from Yui NARUSE to Yukihiro Matsumoto

About the behavior, the right return value is not clear for me.
It needs matz' decision.

#7 Updated by Yui NARUSE 2 months ago

  • Status changed from Open to Feedback
  • Assignee changed from Yukihiro Matsumoto to Marc-Andre Lafortune

#8 Updated by Yui NARUSE 2 months ago

  • Status changed from Feedback to Closed

Applied in changeset r45138.


merge revision(s) 44412,44413,44414,44420,44421: [Backport #9298]

test_method.rb, test_proc.rb: suppress warnings

* test/ruby/test_method.rb: suppress warnings in verbose mode.

* test/ruby/test_proc.rb: ditto.
* proc.c (rb_iseq_min_max_arity): maximum argument is unlimited if
  having rest keywords argument.   [Bug #8072]

* iseq.c (rb_iseq_parameters): push argument type symbol only for
  unnamed rest keywords argument.

* compile.c (iseq_set_arguments): set arg_keyword_check from
  nd_cflag, which is set by parser.  internal ID is used for
  unnamed keyword rest argument, which should be separated from no
  keyword check.

* iseq.c (rb_iseq_parameters): if no keyword check, keyword rest is
  present.

* parse.y (new_args_tail_gen): set keywords check to nd_cflag, which
  equals to that keyword rest is not present.

Also available in: Atom PDF