TypeError with Prime.each_with_index and mass assignment #744

Closed
sferik opened this Issue May 18, 2013 · 4 comments

Comments

Projects
None yet
3 participants
@sferik
Contributor

sferik commented May 18, 2013

I have written a simple program to find clean chains of prime numbers:

require 'prime'

Prime.each_with_index.inject(0) do |sum, (number, index)|
  puts index + 1 if sum % number == 0
  sum += number
end

This program runs on MRI 1.9+ but fails on JRuby 1.7 with the following error:

TypeError: Array can't be coerced into Fixnum
                % at org/jruby/RubyFixnum.java:634
           (root) at clean_chains.rb:4
             call at org/jruby/RubyProc.java:255
             each at /Users/sferik/.rbenv/versions/jruby-1.7.4/lib/ruby/1.9/prime.rb:287
             loop at org/jruby/RubyKernel.java:1489
             each at /Users/sferik/.rbenv/versions/jruby-1.7.4/lib/ruby/1.9/prime.rb:286
             each at /Users/sferik/.rbenv/versions/jruby-1.7.4/lib/ruby/1.9/prime.rb:149
         __send__ at org/jruby/RubyBasicObject.java:1703
             each at /Users/sferik/.rbenv/versions/jruby-1.7.4/lib/ruby/1.9/forwardable.rb:201
  each_with_index at org/jruby/RubyEnumerable.java:920
             each at org/jruby/RubyEnumerator.java:270
           inject at org/jruby/RubyEnumerable.java:815
           (root) at clean_chains.rb:3

The program does not crash (and runs as expected) if I replace Prime with the array:

[2, 3, 5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47, 53, 59, 61, 67, 71]

I get a similar error on MRI if I remove the parentheses, which leads me to believe the issue is related to destructuring assignment in conjunction with the way Prime.each_with_index is implemented in JRuby.

@BanzaiMan

This comment has been minimized.

Show comment
Hide comment
@BanzaiMan

BanzaiMan Jun 16, 2013

Member

When an Enumerator is used, the value assigned to number is different.

Member

BanzaiMan commented Jun 16, 2013

When an Enumerator is used, the value assigned to number is different.

@dmarcotte

This comment has been minimized.

Show comment
Hide comment
@dmarcotte

dmarcotte Jun 16, 2013

Contributor

Hey @BanzaiMan, I'm just putting the finishing touches on a fix for this. The root cause here is a bad arity assumption in RubyEnumerable#each_with_indexCommon which only comes into play when each is implemented with a call rather than a yield (as it is in Prime). Here's a minimal repro scenario:

one_args_each = Class.new do
  include Enumerable
  def each(&block)
    yield "yield arg has the correct type"
    block.call("call arg is wrapped in an array with an extra nil")
  end
end

one_args_each.new.each_with_index do |each_args, index|
  p each_args
end

Will send the pull once I've finished validating a few things... just wanted to give you the heads up since you commented so that we don't dupe work.

Contributor

dmarcotte commented Jun 16, 2013

Hey @BanzaiMan, I'm just putting the finishing touches on a fix for this. The root cause here is a bad arity assumption in RubyEnumerable#each_with_indexCommon which only comes into play when each is implemented with a call rather than a yield (as it is in Prime). Here's a minimal repro scenario:

one_args_each = Class.new do
  include Enumerable
  def each(&block)
    yield "yield arg has the correct type"
    block.call("call arg is wrapped in an array with an extra nil")
  end
end

one_args_each.new.each_with_index do |each_args, index|
  p each_args
end

Will send the pull once I've finished validating a few things... just wanted to give you the heads up since you commented so that we don't dupe work.

@BanzaiMan

This comment has been minimized.

Show comment
Hide comment
@BanzaiMan

BanzaiMan Jun 16, 2013

Member

@dmarcotte I would be busy the rest of the way today, so your contribution is welcome!

Member

BanzaiMan commented Jun 16, 2013

@dmarcotte I would be busy the rest of the way today, so your contribution is welcome!

@dmarcotte

This comment has been minimized.

Show comment
Hide comment
@dmarcotte

dmarcotte Jun 17, 2013

Contributor

Glad to help @BanzaiMan! Just sent pull #807. Check it out for even more context on what's going on here.

Contributor

dmarcotte commented Jun 17, 2013

Glad to help @BanzaiMan! Just sent pull #807. Check it out for even more context on what's going on here.

@BanzaiMan BanzaiMan closed this in #807 Jun 17, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment