Bug #5694: Proc#arity doesn't take optional arguments into account.

• Status: Closed
• Assignee: nobu (Nobuyoshi Nakada)
• Target version: -
• ruby -v: -

[marcandre (Marc-Andre Lafortune)]

Currently:

->(foo = 42){}.arity # => 0, should be -1

This is contrary to the documentation and to what we should expect from the equivalent method definition.

Fixed in trunk, requesting backport for the 1.9 line.

[marcandre (Marc-Andre Lafortune)]

• Status changed from Open to Closed

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

---

• proc.c (rb_proc_arity): Fix Proc#arity in case of optional arguments
  [bug #5694] [rubyspec:b8b259]

[marcandre (Marc-Andre Lafortune)]

• Tracker changed from Bug to Backport
• Project changed from Ruby master to Backport193
• Category deleted (core)
• Status changed from Closed to Open
• Assignee deleted (marcandre (Marc-Andre Lafortune))
• Target version deleted (2.0.0)

[naruse (Yui NARUSE)]

• Tracker changed from Backport to Bug
• Project changed from Backport193 to Ruby master
• Status changed from Open to Assigned
• Assignee set to matz (Yukihiro Matsumoto)

Revert r33921.

Revert "* proc.c (rb_proc_arity): Fix Proc#arity in case of optional arguments"
Because following two reason:

• Proc#arity's return value with optional arguments is not clear.
  The optional argument for proc/lambda is Ruby 1.9 feature.
  In 1.9, proc(a, b=1){} can receive 1 or more arguments.
  But lambda(a, b=1){} can receive only 1 or two arguments.
  r33921 breaks lambda's arity.
  The spec arround optional arguments of proc/lambda needs more
  discussion.

• No tests.
  Add tests to test-all.

[trans (Thomas Sawyer)]

I never quite understood why ranges were never used. Something like:

lambda{}.arity => 0
lambda{|a| }.arity => 1
lambda{|a,b| }.arity => 2

lambda{|a=1| }.arity => 0..1
lambda{|a,b=2| }.arity => 1..2

lambda{|*a| }.arity => -1
lambda{|a,*b| }.arity => 1..-2

lambda{|a,b=2,*c| }.arity => 1..-3

[naruse (Yui NARUSE)]

• ruby -v set to -

(2011/12/03 4:20), Yehuda Katz wrote:

Yeah… it would be nice to be able to do:

if proc.arity_range.include?(2)
if method.arity_range.include?(2)

Is it really required way?

For example you want to know whether you can obj.meth(a, b) or not,
it should be something like obj.callable?(:meth, a, b), shouldn't it?

--
NARUSE, Yui [e-mail]

[wycats (Yehuda Katz)]

I like it!

On Sat, Dec 3, 2011 at 7:41 PM, NARUSE, Yui [e-mail] wrote:

(2011/12/03 4:20), Yehuda Katz wrote:

Yeah… it would be nice to be able to do:

if proc.arity_range.include?(2)
if method.arity_range.include?(2)

Is it really required way?

For example you want to know whether you can obj.meth(a, b) or not,
it should be something like obj.callable?(:meth, a, b), shouldn't it?

--
NARUSE, Yui [e-mail]

[trans (Thomas Sawyer)]

On Sat, Dec 3, 2011 at 10:41 PM, NARUSE, Yui [e-mail] wrote:

For example you want to know whether you can obj.meth(a, b) or not,
it should be something like obj.callable?(:meth, a, b), shouldn't it?

I concur. That would be sweet!

I think Ruby's arity should still be shored up if it is deficient, though.

[naruse (Yui NARUSE)]

(2011/12/04 12:41), NARUSE, Yui wrote:

(2011/12/03 4:20), Yehuda Katz wrote:

Yeah… it would be nice to be able to do:

if proc.arity_range.include?(2)
if method.arity_range.include?(2)

Is it really required way?

For example you want to know whether you can obj.meth(a, b) or not,
it should be something like obj.callable?(:meth, a, b), shouldn't it?

After some consideration, I'm wondering why following code is not used.
There is already a method or proc object in usual use case?

begin
obj.send(callback, attributes)
rescue ArgumentError
obj.send(callback)
end

--
NARUSE, Yui [e-mail]

[trans (Thomas Sawyer)]

On Sunday, December 4, 2011 3:02:29 PM UTC-5, NARUSE, Yui wrote:

After some consideration, I'm wondering why following code is not used.
There is already a method or proc object in usual use case?

begin
obj.send(callback, attributes)
rescue ArgumentError
obj.send(callback)
end

I avoid rescue clauses when I can because I am under the assumption that
they will (almost) always be less efficient then a conditional check.

[shyouhei (Shyouhei Urabe)]

trans wrote:

On Sunday, December 4, 2011 3:02:29 PM UTC-5, NARUSE, Yui wrote:

After some consideration, I'm wondering why following code is not used.
There is already a method or proc object in usual use case?

begin
obj.send(callback, attributes)
rescue ArgumentError
obj.send(callback)
end

I avoid rescue clauses when I can because I am under the assumption that
they will (almost) always be less efficient then a conditional check.

As of ruby 1.9 a begin ... end block is almost no cost. To raise an
exception is the hard part. So the above code works efficiently as
long as that callback takes attributes arguments.

[marcandre (Marc-Andre Lafortune)]

Hi,

Yui NARUSE wrote:

Revert r33921.

Revert "* proc.c (rb_proc_arity): Fix Proc#arity in case of optional arguments"

I just noticed this, as I think the mailing list didn't get this update (at least I didn't)

You sure seem to enjoy reverting my commits.

Because following two reason:

• Proc#arity's return value with optional arguments is not clear.

What exactly is not clear in the official doc: "If the block has optional arguments, return -n-1, where n is the number of mandatory arguments." ?

The optional argument for proc/lambda is Ruby 1.9 feature.
In 1.9, proc(a, b=1){} can receive 1 or more arguments.
But lambda(a, b=1){} can receive only 1 or two arguments.

I'm glad to see we agree on this.

r33921 breaks lambda's arity.

How so?

The spec arround optional arguments of proc/lambda needs more
discussion.

I don't think so, but if you do, maybe you can suggest something? Maybe you can say in what way you'd want to change the official documentation, and the reasons why you think the arity of lambda's should behave differently from the arity of the equivalent method?

• No tests.
  Add tests to test-all.

As per the commit message, they have been committed to Rubyspec. As per my previous posts, you are most welcome to duplicate these in test-all. This doesn't warrant a revert. As per my previous posts, I would again ask you not to revert my commits in trunk, I'll do that myself if need be.

BTW, I agree it would be nice to easily get the maximum number of parameters for methods and lambdas (as well as maximum number of non-ignored parameters for a proc). I'll throw in the idea that arity_range could return an infinity in the case of unlimited arguments:

->(*args){}.arity_range # => 0...Float::INFINITY ?
->(*args){}.arity_range.include?(42) # => true

I feel it's more useful than (0..-1), which is not a real range. We might as well only introduce arity_max, which could return -1, or even nil for unlimited arguments.

I'm less enthusiastic about callable? because:

1. it is not be able to easily/reliably distinguish between a lamba with many (but limited) number of optional arguments and one with unlimited number of arguments.
2. it doesn't give useful information for Procs. A Proc{||} is callable with 42 arguments, but these will be ignored.
3. it might give the impression that the validity of the parameters will be checked too

The begin rescue is not a good solution because there is no (easy and reliable) way to know if the ArgumentError is due directly to the calling of the lambda/proc/method, or from some code inside of it generating an ArgumentError.

In short, my vote goes to arity_range returning the <min_params .. max_params>, where max_params would always be non-negative, and would be Float::INFINITY for unlimited arguments.

[naruse (Yui NARUSE)]

Marc-Andre Lafortune wrote:

You sure seem to enjoy reverting my commits.

I don't want to see such ugly commits.
I hate reverting but I hate more legacy code in ruby repo.

Because following two reason:

• Proc#arity's return value with optional arguments is not clear.

What exactly is not clear in the official doc: "If the block has optional arguments, return -n-1, where n is the number of mandatory arguments." ?

The rdoc describes optional arguments but doesn't describe rest arguments.
The behavior of rest arguments follows written optional arguments, so it seems rdoc is wrong.

The optional argument for proc/lambda is Ruby 1.9 feature.
In 1.9, proc(a, b=1){} can receive 1 or more arguments.
But lambda(a, b=1){} can receive only 1 or two arguments.

I'm glad to see we agree on this.

r33921 breaks lambda's arity.

How so?

r33921 makes lambda(a, b=1){}.arity returns -1 though lamda doesn't allow more than two arguments.

The spec arround optional arguments of proc/lambda needs more
discussion.

I don't think so, but if you do, maybe you can suggest something? Maybe you can say in what way you'd want to change the official documentation, and the reasons why you think the arity of lambda's should behave differently from the arity of the equivalent method?

Ah, you are correct, I suggest:

• • to take exactly n arguments, returns n. If the block has optional

• • to take exactly n arguments, returns n. If the block has rest

• No tests.
  Add tests to test-all.

As per the commit message, they have been committed to Rubyspec. As per my previous posts, you are most welcome to duplicate these in test-all. This doesn't warrant a revert. As per my previous posts, I would again ask you not to revert my commits in trunk, I'll do that myself if need be.

As I said before, rubyspec is not enough.
I'll revert commits which changes the behavior of ruby and don't have related test-all tests.

BTW, I agree it would be nice to easily get the maximum number of parameters for methods and lambdas (as well as maximum number of non-ignored parameters for a proc). I'll throw in the idea that arity_range could return an infinity in the case of unlimited arguments:

->(*args){}.arity_range # => 0...Float::INFINITY ?
->(*args){}.arity_range.include?(42) # => true

I feel it's more useful than (0..-1), which is not a real range. We might as well only introduce arity_max, which could return -1, or even nil for unlimited arguments.

What is the use case?

I'm less enthusiastic about callable? because:

1. it is not be able to easily/reliably distinguish between a lamba with many (but limited) number of optional arguments and one with unlimited number of arguments.
2. it doesn't give useful information for Procs. A Proc{||} is callable with 42 arguments, but these will be ignored.
3. it might give the impression that the validity of the parameters will be checked too

Hmm, it seems reasonable.

The begin rescue is not a good solution because there is no (easy and reliable) way to know if the ArgumentError is due directly to the calling of the lambda/proc/method, or from some code inside of it generating an ArgumentError.

Does it really need to be distinguished?
And if you really need, you can check backtrace.

In short, my vote goes to arity_range returning the <min_params .. max_params>, where max_params would always be non-negative, and would be Float::INFINITY for unlimited arguments.

It seems reasonable.

[matz (Yukihiro Matsumoto)]

• Status changed from Assigned to Rejected

We are not going to add incompatible changes to trunk, unless we see consensus after discussion. I think this is not the case. Currently and for near future, arity ignores optional arguments. Use #parameters or propose new API like #arity_range (in a separate proposal).

[marcandre (Marc-Andre Lafortune)]

• Category set to core
• Status changed from Rejected to Open

Changing this to "open", as this bug can not simply be "rejected" as the implementation currently doesn't agree with the documentation.

Either the documentation needs to change, or the implementation needs to change.

Here's why I (still) believe the implementation should change:

It is strictly superior because it is consistent with Method#arity and with Proc#arity for method converted to proc. Currently:

def Object.foo(a=42); end
foo = Object.method(:foo).to_proc
bar = ->(a=42){}
foo.class == bar.class # => true
foo.lambda? == bar.lambda? # => true
foo.arity == bar.arity # => false

It is also consistent with the current official documentation.

The current behavior gives the information that a lambda/proc has a "rest" argument, but doesn't give information about any "optional" arguments, so both information would be useful. Indeed I still believe there should be a simple way to know the maximum number of parameters.

I could find no intent for the current behavior in the code, no test for current behavior (test/* nor RubySpec).

I think it is a clear defect as there is a contradiction with the official documentation.

It currently states: "If the block has optional arguments, return -n-1, where n is the number of mandatory arguments."

To be correctly reflect the current behavior, it would have to read something like "If the block has optional arguments, return -n-1, where n is the number of mandatory arguments, except that if the Proc doesn't have unlimited optional arguments (i.e. no *args, only arg=:default) and if the Proc was not created from a method, then it is considered as having no optional argument."

Moreover, I'll be posting an additional issue that is a direct consequence of this same bug.

[matz (Yukihiro Matsumoto)]

Hi,

In message "Re: [ruby-core:41593] [ruby-trunk - Bug #5694][Open] Proc#arity doesn't take optional arguments into account."
on Mon, 12 Dec 2011 02:06:26 +0900, Marc-Andre Lafortune [e-mail] writes:

|It is strictly superior because it is consistent with Method#arity and with Proc#arity for method converted to proc. Currently:
|
| def Object.foo(a=42); end
| foo = Object.method(:foo).to_proc
| bar = ->(a=42){}
| foo.class == bar.class # => true
| foo.lambda? == bar.lambda? # => true
| foo.arity == bar.arity # => false
|
|It is also consistent with the current official documentation.

This is a problem. We have to be consistent here. Let us check.

|I think it is a clear defect as there is a contradiction with the official documentation.

Don't emphasize "official" too much. Historically "official"
documentation has not been written by the original author (me) nor
implementer of the feature, so that it had often been written from
guessing from the code.

[marcandre (Marc-Andre Lafortune)]

Hi,

Yukihiro Matsumoto wrote:

We are not going to add incompatible changes to trunk, ...

Could you please explain to me what difference you see between an "incompatible change" and any "bug fix"?

Every bug fix will change some output under some circumstances. It's always possible to make a program that works before a bug fix is applied and that doesn't after the bug is fixed. This doesn't warrant the term "incompatible change", though.

As I've tried to detail my evaluation criteria, if there is no sign of intent, I do not consider it an "incompatible change". Noone has shown any sign of intent with respect to the arity bug. What justification do you have to call this an "incompatible change"?

Don't emphasize "official" too much. Historically "official"
documentation has not been written by the original author (me) nor
implementer of the feature, so that it had often been written from
guessing from the code.

I agree that if the documentation doesn't state some behavior, no emphasis should be made.

I will continue to emphasize it when it does specify something, though. Thousands of people have read the reference documentation, have built an understanding of the Ruby language from what is says and written code according to it.

That we like it or not, the documentation is a kind of contract with the community and my understanding is that we try avoid changing that contract when possible.

Moreover, if there is any remaining doubt that the fault somehow lies in the documentation, it is clear from r759 (eval.c, see https://github.com/ruby/ruby/commit/1199a7d3d ) that the documentation for Method#arity is faithful to the intentions of the implementor.

We've had #arity return -n-1 whenever there are optional arguments or "rest" arguments for the past 11 years. I'm honestly still surprised I even have to convince anyone and that a discussion is needed. This is the big lesson I'll get from this; I will try not to assume that obvious bug fixes are obvious anymore.

[matz (Yukihiro Matsumoto)]

Hi,

In message "Re: [ruby-core:41600] [ruby-trunk - Bug #5694] Proc#arity doesn't take optional arguments into account."
on Mon, 12 Dec 2011 11:30:26 +0900, Marc-Andre Lafortune [e-mail] writes:

|Yukihiro Matsumoto wrote:
|> We are not going to add incompatible changes to trunk, ...
|
|Could you please explain to me what difference you see between an "incompatible change" and any "bug fix"?

Good point. It's whether the change makes difference from intention
of the original designer. I know it's a vague definition. But too
strict rule did not run community well in the past.

|As I've tried to detail my evaluation criteria, if there is no sign of intent, I do not consider it an "incompatible change". Noone has shown any sign of intent with respect to the arity bug. What justification do you have to call this an "incompatible change"?

I agree we have at least one bug in arity.

 def Object.foo(a=42); end
 foo = Object.method(:foo).to_proc
 bar = ->(a=42){}
 foo.arity == bar.arity # => false

should be true.

Also, I think we need to update the documentation. After defining
consistent "right" definition.

|We've had #arity return -n-1 whenever there are optional arguments or "rest" arguments for the past 11 years. I'm honestly still surprised I even have to convince anyone and that a discussion is needed. This is the big lesson I'll get from this; I will try not to assume that obvious bug fixes are obvious anymore.

Thank you for finding inconsistency. Now we need to fix up the
desirable behavior. First I thought to keep the original arity
behavior, but after investigating the behavior, both Method#arity and
Proc#arity have weird corner cases.

My idea for new behavior is:

• arity ignores all optinal arguments

• arity returns -n-1 if there's rest argument

• where n is number of mandatory arguments

• we might need to add arity_max that honors optinal arguments, or
  existing #parameter may be good enough.

Any opinion?

						matz.

Hi,

On Sun, Dec 11, 2011 at 10:51 PM, Yukihiro Matsumoto [e-mail] wrote:

Hi,

In message "Re: [ruby-core:41600] [ruby-trunk - Bug #5694] Proc#arity doesn't take optional arguments into account."
   on Mon, 12 Dec 2011 11:30:26 +0900, Marc-Andre Lafortune [e-mail] writes:

|Yukihiro Matsumoto wrote:
|> We are not going to add incompatible changes to trunk, ...
|
|Could you please explain to me what difference you see between an "incompatible change" and any "bug fix"?

Good point.  It's whether the change makes difference from intention
of the original designer. ...

So do we agree that my patch is not an incompatible change?

 First I thought to keep the original arity
behavior, but after investigating the behavior, both Method#arity and
Proc#arity have weird corner cases.

Apart from the one I'm pointing out (which is only present in Ruby
1.9), what other corner cases are there? In particular, is there any
corner case in the 1.8 line?

My idea for new behavior is:

 * arity ignores all optional arguments
 * arity returns -n-1 if there's rest argument
 * where n is number of mandatory arguments

Any opinion?

Can you explain what would be gained by this new behavior, i.e. how is
this change more helpful than the current behavior?

Also, can you explain why you consider that breaking the previous
behavior would be a good idea?

A quick check for uses of arity in Rails reveal that the typical use
looks like "does this method use only 1 parameter or can it use more
than that?"

Among other things, this change could break many Rails app.

Moreover, Rails and any other gem using arity would have to jump
through hoops to maintain a compatible version with Ruby 1.8.7 (which
doesn't have the parameters method) and Ruby 1.9.2+.

Finally, assuming you decide to go forward with this feature change
for Ruby 2.0, shouldn't the 1.9 line still be fixed with my patch to
be consistent?

Thanks

[marcandre (Marc-Andre Lafortune)]

Matz, what do you want us to do about ->(a=1){}.arity, in the 1.9.3 release and in the 2.0.0 version?

[ddebernardy (Denis de Bernardy)]

Possibly related: http://bugs.ruby-lang.org/issues/6039

[shyouhei (Shyouhei Urabe)]

• Status changed from Open to Assigned

[akr (Akira Tanaka)]

• Assignee changed from matz (Yukihiro Matsumoto) to nobu (Nobuyoshi Nakada)

We discussed this issue at a developer meeting.
We have consensus that lambda {...}.arity should behave as Method#arity.

Note that we should not change proc {...}.arity.

[marcandre (Marc-Andre Lafortune)]

• Status changed from Assigned to Closed

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

---

Revert r33924.

• proc.c (rb_proc_arity): Fix Proc#arity in case of optional arguments
  [bug #5694] [rubyspec:b8b259] [rubyspec:184c8100f]

[marcandre (Marc-Andre Lafortune)]

• Status changed from Closed to Open
• Assignee changed from nobu (Nobuyoshi Nakada) to matz (Yukihiro Matsumoto)

Hi,

akr (Akira Tanaka) wrote:

We discussed this issue at a developer meeting.
We have consensus that lambda {...}.arity should behave as Method#arity.

Good.

Note that we should not change proc {...}.arity.

I missed this the first time.

Why?

Lambdas and Proc have the same arity for the same signature. Why introduce a difference? Same signature => same arity is simplest, no?

We just confirmed that arity = -n -1 means at least n parameters are asked, but more are accepted (not sure how many). Why would this be different for Proc?

Why would a method defined from a Proc not always have the same arity as that block?

[matz (Yukihiro Matsumoto)]

• Assignee changed from matz (Yukihiro Matsumoto) to nobu (Nobuyoshi Nakada)

Because non-lambda proc and lamdba behave differently.
You can blame me about inconsistency. I should have designed proc more consistently.
But I sometimes made mistakes and it's too late to fix this.
At least for 2.0.

Nobu, could you update r36411 according to our discussion?

Matz.

[marcandre (Marc-Andre Lafortune)]

matz (Yukihiro Matsumoto) wrote:

Because non-lambda proc and lamdba behave differently.
You can blame me about inconsistency. I should have designed proc more consistently.

Oh, I'm not blaming anyone.

Nobu, could you update r36411 according to our discussion?

I'm sorry about r36411. I reverted it when I realized Proc were meant to differ from lambdas.

[naruse (Yui NARUSE)]

• Status changed from Open to Closed

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

---

• proc.c (rb_proc_arity): return normal value (not -n-1) if it is not
  a labmda, or it is a labmda and no arg_opts. [Bug #5694]