each_slice enumerator methods fail

Thanks for the report, Nicholas.

Bisecting shows that this is a regression introduced with this commit:

7732e5eb43a037615caf4ed5814551f66d497930 is the first bad commit
commit 7732e5eb43a037615caf4ed5814551f66d497930
Author: Charles Oliver Nutter <headius@headius.com>
Date:   Mon Dec 6 00:36:15 2010 -0600

    Tweaks to support some lighter-weight versions of newer Enumerable methods.
    
    * Make all "CallBlock" blocks in Enumerable pass along a proper arity, so we know whether they might want a real array or specific-arity args
    * Improve Array#each_splice by avoiding intermediate blocks (always) and extra array creations (for specific-arity > 1) to reduce overhead. 30-50% faster.

:040000 040000 055a544312d844dbe5596705e1ac2be440180b78 ea9a65add4d82e1ce348e24b67d5fe20bc79597e M	src

We'll also have to take a look at RubySpec, since we missed this.

Fixed this in d649a49. I'll address the spec issue separately.

jruby --1.9 throws java.lang.ArrayIndexOutOfBoundsException for this test:

require 'test/minirunit'
test_equal [[0, 1], [2, 3]], [0, 1, 2, 3].each_slice(2).map

jruby --1.8 throws ArrayIndexOutOfBoundsException and jruby --1.9 throws ConcurrencyError for this:

[0, 1, 2, 3, 4, 5, 6, 7].each_slice(2) do |x0, x1|
end

Pushed another attempt at fixing this in 4a3c6fe. I verified that the last two cases are corrected. (I'm going to wait a little bit this time before resolving this ticket.)

jruby --1.9 still throws ArrayIndexOutOfBoundsException for this test:

test_equal [[0, 1], [2, 3]], [0, 1, 2, 3].each_slice(2).map

jruby --1.9 yields array as first block argument for each_slice(2).map, compare:

[0, 1, 2, 3].each_slice(2).map do |x0, x1|
p [x0, x1]
end

[0, 1, 2, 3].each_slice(2).each do |x0, x1|
p [x0, x1]
end

Nicholas,

The last point indicates to me that there is a problem with Enumerable#map, rather than Array#each_slice.

This patch passes those tests, but it seems a bit questionable:

--- a/src/org/jruby/RubyEnumerable.java
+++ b/src/org/jruby/RubyEnumerable.java
@@ -729,7 +729,8 @@ public class RubyEnumerable {
 
             callEach19(runtime, context, self, block.arity(), new BlockCallback() {
                 public IRubyObject call(ThreadContext ctx, IRubyObject[] largs, Block blk) {
-                    IRubyObject value = block.yieldArray(ctx, runtime.newArrayNoCopyLight(largs), null, null);
+                    IRubyObject larg = checkArgs(runtime, largs);
+                    IRubyObject value = block.yield(ctx, larg);
                     synchronized (result) {
                         result.append(value);
                     }

That patch effectively reverses this commit by Charlie for RubyEnumerable: https://github.com/jruby/jruby/commit/0c5e9d7421f578c0fd2970d445f69663252cf03d

I'm not 100% clear on what the rationale was for Charlie's commit, so I'll assign this ticket to him for review.

I could not find the case for which I made that Enumerator change, so I reverted it. All cases from this bug now pass, and I'll stick them into a regression test. If there's still a need for my old Enumerator change, I'm sure we'll find it again soon

I think I just found it

test_equal [1],

.map

Yup, that was it. And I fixed it last night

commit 3d95a53902e7600ef1ff474ee535034c6b4a9b1d
Author: Charles Oliver Nutter <headius@headius.com>
Date: Tue Jan 4 05:10:37 2011 -0600

Fix for JRUBY-5302: [1.9] Rails Object#to_param appears to lose values for a Hash

• Ruby 1.9 defines Hash#each to basically do each_pair logic.