Bug #9605

Chaining "each_with_index.detect &lambda" raises ArgumentError

Added by Alex Rothenberg over 1 year ago. Updated 10 months ago.

[ruby-core:61340]
Status:Closed
Priority:Normal
Assignee:Yukihiro Matsumoto
ruby -v:2.1.1 Backport:1.9.3: UNKNOWN, 2.0.0: UNKNOWN, 2.1: UNKNOWN

Description

I found an odd edge case where "detect" and "select" behave differently from other methods of Enumerable.

Normally these methods yield a single argument to a block but when you chain them after "each_with_index" they yield two arguments "item" and "index". The problem is when you try passing a lambda instead of a block then they raise an ArgumentError

$ irb
2.1.1 :001 > lambda = ->(word, index) { word.length == 3 }
=> #<Proc:0x007ff8848630d8@(irb):1 (lambda)>
2.1.1 :002 > %w(Hi there how are you).each_with_index.detect &lambda
ArgumentError: wrong number of arguments (1 for 2)
from (irb):1:in `block in irb_binding'
from (irb):2:in `each'
from (irb):2:in `each_with_index'
from (irb):2:in `each'
from (irb):2:in `detect'
from (irb):2
from /Users/alex/.rvm/rubies/ruby-2.1.1/bin/irb:11:in `<main>'

2.1.1 :003 > %w(Hi there how are you).each_with_index.select &lambda
ArgumentError: wrong number of arguments (1 for 2)
from (irb):1:in `block in irb_binding'
from (irb):3:in `each'
from (irb):3:in `each_with_index'
from (irb):3:in `each'
from (irb):3:in `select'
from (irb):3
from /Users/alex/.rvm/rubies/ruby-2.1.1/bin/irb:11:in `<main>'

Interestingly it works just find when calling other methods like "map"

2.1.1 :004 > %w(Hi there how are you).each_with_index.map &lambda
=> [false, false, true, true, true]

It also works when you use a proc

2.1.1 :001 > proc = Proc.new {|word, index| word.length == 3 }
=> #<Proc:0x007fc375a3a558@(irb):1>
2.1.1 :002 > %w(Hi there how are you).each_with_index.detect &proc
=> ["how", 2]
2.1.1 :003 > %w(Hi there how are you).each_with_index.map &proc
=> [false, false, true, true, true]

or a block

2.1.1 :001 > %w(Hi there how are you).each_with_index.detect {|word, index| word.length == 3 }
=> ["how", 2]
2.1.1 :002 > %w(Hi there how are you).each_with_index.map {|word, index| word.length == 3 }
=> [false, false, true, true, true]

When testing against JRuby or Rubinius none of these scenarios raise an ArgumentError. I'm guessing this is a bug and not intended behavior. If it is intended then both those implementations have a bug in not raising ArgumentError.

Associated revisions

Revision 45284
Added by Nobuyoshi Nakada over 1 year ago

enum.c: yield multiple values

  • enum.c (find_i): yield multiple values instead of a packed array, so that lambda block can work. with tests by Alex Rothenberg. [Bug #9605]

Revision 45284
Added by Nobuyoshi Nakada over 1 year ago

enum.c: yield multiple values

  • enum.c (find_i): yield multiple values instead of a packed array, so that lambda block can work. with tests by Alex Rothenberg. [Bug #9605]

Revision 45286
Added by Nobuyoshi Nakada over 1 year ago

enum.c: yield multiple values

  • enum.c (find_i): yield multiple values instead of a packed array, so that lambda block can work. with tests by Alex Rothenberg. [Bug #9605]

Revision 45286
Added by Nobuyoshi Nakada over 1 year ago

enum.c: yield multiple values

  • enum.c (find_i): yield multiple values instead of a packed array, so that lambda block can work. with tests by Alex Rothenberg. [Bug #9605]

Revision 45292
Added by Nobuyoshi Nakada over 1 year ago

enum.c: suppress warnings

  • enum.c (each_val_i): svalue is no longer used. [Bug #9605]

Revision 45292
Added by Nobuyoshi Nakada over 1 year ago

enum.c: suppress warnings

  • enum.c (each_val_i): svalue is no longer used. [Bug #9605]

Revision 45322
Added by Nobuyoshi Nakada over 1 year ago

revert [Bug #9605]

Revision 45322
Added by Nobuyoshi Nakada over 1 year ago

revert [Bug #9605]

Revision 45327
Added by Nobuyoshi Nakada over 1 year ago

vm_insnhelper.c: relax arity check

  • vm.c (invoke_block_from_c): add splattable argument.
  • vm.c (vm_invoke_proc): disallow to splat when directly invoked.
  • vm_insnhelper.c (vm_callee_setup_arg_complex, vm_callee_setup_arg): relax arity check of yielded lambda. [Bug #9605]
  • test/ruby/test_yield.rb (TestRubyYieldGen#emu_bind_params): no longer raise ArgumentError when splatting to lambda.

Revision 45327
Added by Nobuyoshi Nakada over 1 year ago

vm_insnhelper.c: relax arity check

  • vm.c (invoke_block_from_c): add splattable argument.
  • vm.c (vm_invoke_proc): disallow to splat when directly invoked.
  • vm_insnhelper.c (vm_callee_setup_arg_complex, vm_callee_setup_arg): relax arity check of yielded lambda. [Bug #9605]
  • test/ruby/test_yield.rb (TestRubyYieldGen#emu_bind_params): no longer raise ArgumentError when splatting to lambda.

Revision 48193
Added by Nobuyoshi Nakada 10 months ago

vm_insnhelper.c: allow to_ary

  • vm_insnhelper.c (vm_callee_setup_arg{_complex,}): try conversion by to_ary for a lambda, as well as a proc. [Bug #9605]

Revision 48193
Added by Nobuyoshi Nakada 10 months ago

vm_insnhelper.c: allow to_ary

  • vm_insnhelper.c (vm_callee_setup_arg{_complex,}): try conversion by to_ary for a lambda, as well as a proc. [Bug #9605]

History

#1 Updated by Nobuyoshi Nakada over 1 year ago

There are other methods which have same behavior
I've thought it would have compatibility issues, but make check didn't fail at least.

WIP: https://github.com/nobu/ruby/compare/enum-yield_values?expand=1

#2 Updated by Alex Rothenberg over 1 year ago

@nobu Wow, you fixed this issue so quickly. I wrote a few additional test cases that fail on trunk but pass on your branch in case you want to merge them in https://github.com/nobu/ruby/pull/1

Thank you!

#3 Updated by Nobuyoshi Nakada over 1 year ago

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

Applied in changeset r45284.


enum.c: yield multiple values

  • enum.c (find_i): yield multiple values instead of a packed array, so that lambda block can work. with tests by Alex Rothenberg. [Bug #9605]

#4 Updated by Nobuyoshi Nakada over 1 year ago

  • Status changed from Closed to Rejected

It conflicts with existing behaviors too much.

#5 Updated by Alex Rothenberg over 1 year ago

Nobuyoshi Nakada wrote:

It conflicts with existing behaviors too much.

I'm not surprised this caused other problems but I was hoping it wouldn't.

Is the current behavior now the expected behavior of Ruby and should I let other implementations know they do not work this way? It seems that implementation specific differences are not a good thing even though this one is so odd it probably does not affect many (or any) real projects.

#6 Updated by Nobuyoshi Nakada over 1 year ago

Ruby raises ArgumentError at arity mismatch when yielding to a lambda, as test/ruby/test_lambda.rb.

$ ruby -v -e '[1].each(&lambda{p :ng})'
ruby 2.2.0dev (2014-03-09 trunk 45302) [universal.x86_64-darwin13.0]
-e:1:in `block in <main>': wrong number of arguments (1 for 0) (ArgumentError)
    from -e:1:in `each'
    from -e:1:in `<main>'
bash: exit 1

$ jruby -v -e '[1].each(&lambda{p :ng})'
jruby 1.7.4 (1.9.3p392) 2014-03-13 fffffff on Java HotSpot(TM) 64-Bit Server VM 1.6.0_65-b14-462-11M4609 [darwin-x86_64]
:ng

#7 Updated by Nobuyoshi Nakada over 1 year ago

Hmmm, in JRuby, ->(){} and lambda{} are different?

$ jruby -v -e '[1].each(&lambda{p :ng})'
jruby 1.7.4 (1.9.3p392) 2014-03-13 fffffff on Java HotSpot(TM) 64-Bit Server VM 1.6.0_65-b14-462-11M4609 [darwin-x86_64]
:ng

$ jruby -v -e '[1].each(&->(){p :ng})'
jruby 1.7.4 (1.9.3p392) 2014-03-13 fffffff on Java HotSpot(TM) 64-Bit Server VM 1.6.0_65-b14-462-11M4609 [darwin-x86_64]
ArgumentError: wrong number of arguments (1 for 0)
    each at org/jruby/RubyArray.java:1617
  (root) at -e:1

#8 Updated by Alex Rothenberg over 1 year ago

Wow this gets weirder and weirder. It seems to be happening when jruby turns a lambda created with "->" syntax into a block. MRI does consistently raise ArgumentError in all 4 cases.

$ jruby -v -e 'def test(l); l.call(1); end; test(lambda{p :ng})'
jruby 1.7.10 (1.9.3p392) 2014-01-09 c4ecd6b on Java HotSpot(TM) 64-Bit Server VM 1.7.0_51-b13 [darwin-x86_64]
ArgumentError: wrong number of arguments (1 for 0)
call at org/jruby/RubyProc.java:267
test at -e:1
(root) at -e:1

$ jruby -v -e 'def test(l); l.call(1); end; test(->(){p :ng})'
jruby 1.7.10 (1.9.3p392) 2014-01-09 c4ecd6b on Java HotSpot(TM) 64-Bit Server VM 1.7.0_51-b13 [darwin-x86_64]
ArgumentError: wrong number of arguments (1 for 0)
call at org/jruby/RubyProc.java:267
test at -e:1
(root) at -e:1

$ jruby -v -e 'def test; yield 1; end; test(&->(){p :ng})'
jruby 1.7.10 (1.9.3p392) 2014-01-09 c4ecd6b on Java HotSpot(TM) 64-Bit Server VM 1.7.0_51-b13 [darwin-x86_64]
ArgumentError: wrong number of arguments (1 for 0)
test at -e:1
(root) at -e:1

$ jruby -v -e 'def test; yield 1; end; test(&lambda{p :ng})'
jruby 1.7.10 (1.9.3p392) 2014-01-09 c4ecd6b on Java HotSpot(TM) 64-Bit Server VM 1.7.0_51-b13 [darwin-x86_64]
:ng

This feels like its probably a jruby bug and is probably a discussion I should move onto their issue list.

#9 Updated by Alex Rothenberg over 1 year ago

A little more digging and I found Rubinius is consistent with the lambda and -> syntax but neither raises the ArgumentError. It seems like they remove the arity check when the lambda is converted to a block

$ ruby -v -e '[1].each(&lambda{p :ng})'
rubinius 2.2.3 (2.1.0 4792e746 2013-12-29 JI) [x86_64-darwin13.0.2]
:ng

$ ruby -v -e '[1].each(&->(){p :ng})'
rubinius 2.2.3 (2.1.0 4792e746 2013-12-29 JI) [x86_64-darwin13.0.2]
:ng

$ ruby -v -e 'def test(l); l.call(1); end;  test(lambda{p :ng})'
rubinius 2.2.3 (2.1.0 4792e746 2013-12-29 JI) [x86_64-darwin13.0.2]
An exception occurred evaluating command line code:

    method '__block__': given 1, expected 0 (ArgumentError)

Backtrace:

                                    Proc#call at kernel/bootstrap/proc.rb:20
                                  Object#test at -e:1
                     { } in Object#__script__ at -e:1
  Rubinius::BlockEnvironment#call_on_instance at kernel/common/block_environment.rb:53
                Kernel(Rubinius::Loader)#eval at kernel/common/eval.rb:176
                       Rubinius::Loader#evals at kernel/loader.rb:616
                        Rubinius::Loader#main at kernel/loader.rb:830
$ ruby -v -e 'def test(l); l.call(1); end;  test(->(){p :ng})' 
rubinius 2.2.3 (2.1.0 4792e746 2013-12-29 JI) [x86_64-darwin13.0.2]
An exception occurred evaluating command line code:

    method '__block__': given 1, expected 0 (ArgumentError)

Backtrace:

                                    Proc#call at kernel/bootstrap/proc.rb:20
                                  Object#test at -e:1
                     { } in Object#__script__ at -e:1
  Rubinius::BlockEnvironment#call_on_instance at kernel/common/block_environment.rb:53
                Kernel(Rubinius::Loader)#eval at kernel/common/eval.rb:176
                       Rubinius::Loader#evals at kernel/loader.rb:616
                        Rubinius::Loader#main at kernel/loader.rb:830

#10 Updated by Alex Rothenberg over 1 year ago

Finally found another implementation that behaves identically to MRI. RubyMotion acts the same with "each_with_index.map" accepting 2 args while "each_with_index.detect" raises an ArgumentError.

$ rake
     Build ./build/iPhoneSimulator-7.1-Development
   Compile ./app/app_delegate.rb
    Create ./build/iPhoneSimulator-7.1-Development/test.app
      Link ./build/iPhoneSimulator-7.1-Development/test.app/test
    Create ./build/iPhoneSimulator-7.1-Development/test.app/PkgInfo
    Create ./build/iPhoneSimulator-7.1-Development/test.app/Info.plist
      Copy ./resources/Default-568h@2x.png
    Create ./build/iPhoneSimulator-7.1-Development/test.dSYM
  Simulate ./build/iPhoneSimulator-7.1-Development/test.app
(main)> [1].each(&lambda{p :ng})
2014-03-13 08:05:41.185 test[13295:70b] wrong number of arguments (1 for 0) (ArgumentError)
=> #<ArgumentError: wrong number of arguments (1 for 0)>
(main)> [1].each(&->(){p :ng})
2014-03-13 08:05:48.836 test[13295:70b] wrong number of arguments (1 for 0) (ArgumentError)
=> #<ArgumentError: wrong number of arguments (1 for 0)>
(main)> [1].each_with_index.map(&->(x,i){p [x,i].inspect})
"[1, 0]"
=> ["[1, 0]"]
(main)> [1].each_with_index.detect(&->(x,i){p [x,i].inspect})
2014-03-13 08:06:26.346 test[13295:70b] wrong number of arguments (1 for 2) (ArgumentError)
=> #<ArgumentError: wrong number of arguments (1 for 2)>

Also the 4 cases with lambdas and lambdas convereted to blocks all raise ArgumentError.

(main)> def test
(main)>   yield 1
(main)> end
=> :test
(main)> test(&lambda{p :ng})
2014-03-13 08:28:38.975 test[13295:70b] wrong number of arguments (1 for 0) (ArgumentError)
=> #<ArgumentError: wrong number of arguments (1 for 0)>
(main)> test(&->(){p :ng})
2014-03-13 08:28:50.617 test[13295:70b] wrong number of arguments (1 for 0) (ArgumentError)
=> #<ArgumentError: wrong number of arguments (1 for 0)>

(main)> def test(l)
(main)>   l.call(1)
(main)> end
=> :test
(main)> test(&lambda{p :ng})                                                                                                                                        2014-03-13 08:29:08.934 test[13295:70b] wrong number of arguments (0 for 1) (ArgumentError)
=> #<ArgumentError: wrong number of arguments (0 for 1)>
(main)> test(&->(){p :ng})                                                                                                                                        2014-03-13 08:29:17.132 test[13295:70b] wrong number of arguments (0 for 1) (ArgumentError)
=> #<ArgumentError: wrong number of arguments (0 for 1)>

#11 Updated by Alex Rothenberg over 1 year ago

I created a jruby issue https://github.com/jruby/jruby/issues/1559 to track the odd "lambda" vs "->" difference.

#12 Updated by Nobuyoshi Nakada over 1 year ago

Today, matz and I chatted about this issue, and he decided to relax the arity check of lambda blocks.
Iff:

  • just one argument is yielded
  • it is an array, and
  • its length is same as the number of the formal argument

the argument will be splatted to the block.

plus = ->(x,y) {next x+y}
[1,2].tap(&plus) # to be allowed
[1].tap(&plus) # ArgumentError, as the current

#13 Updated by Alex Rothenberg over 1 year ago

If I understand correctly what you're saying is that it would behave as below.

plus = ->(x,y) {puts x+y}
  1. This would continue to work as it does today
def test
  yield 1,2
end
test(&plus)
# prints 3
  1. The new behavior
def test
  yield [1,2]
end
test(&plus)
# prints 3
  1. Existing ArgumentError
def test
  yield 1,2,3
end
test(&plus)
# Would raise ArgumentError 3 for 2
  1. Existing ArgumentError
def test
  yield 1
end
test(&plus)
# Would raise ArgumentError 1 for 2

What would happen in the case where you don't convert the lambda to a block and call with an array of the same length as the lambda's arity?

def test l
  l.call [1,2]
end
test(plus)

Currently it raises "ArgumentError: wrong number of arguments (1 for 2)". It feels a bit strange to me if that behavior continues but is different the similar example with an array and lambda converted to a block.

#14 Updated by Alex Rothenberg over 1 year ago

@nobu I meant to tell you how honored I am that you and matz are taking so much time talking about this issue and even considering changing this small corner of ruby itself. The idea of changing the language because of something I noticed does feel a little intimidating to me though :)

Thank you for all your hard work building this language I love and let me know if there's anything more I can do to help with this issue.

#15 Updated by Koichi Sasada 10 months ago

  • Category set to core
  • Assignee set to Yukihiro Matsumoto
  • Target version set to current: 2.2.0

This change introduce inconsistency between block and lambda.

An Array object is splatted, but ArrayLike object whcih has to_ary is not splatted.


class ArrayLike
  def initialize *args
    @ary = args
  end

  def to_ary
    @ary
  end
end

def m0
  yield [1, 2, 3]
end

def m1
  yield ArrayLike.new(1, 2, 3)
end

m0(&proc{|a, b, c| p [a, b, c]})
m0(&lambda{|a, b, c| p [a, b, c]})

m1(&proc{|a, b, c| p [a, b, c]})
m1(&lambda{|a, b, c| p [a, b, c]})

__END__
[1, 2, 3]
[1, 2, 3]
[1, 2, 3]
test.rb:24:in `block in <main>': wrong number of arguments (1 for 3) (ArgumentError)
    from test.rb:17:in `m1'
    from test.rb:24:in `<main>'

Is it intentional behavior?

#16 Updated by Koichi Sasada 10 months ago

  • Status changed from Rejected to Open

#17 Updated by Nobuyoshi Nakada 10 months ago

  • Status changed from Open to Closed

Applied in changeset r48193.


vm_insnhelper.c: allow to_ary

  • vm_insnhelper.c (vm_callee_setup_arg{_complex,}): try conversion by to_ary for a lambda, as well as a proc. [Bug #9605]

Also available in: Atom PDF