Fix each_with_index argument handling #807

Merged
merged 1 commit into from Jun 17, 2013

Projects

None yet

2 participants

@dmarcotte
Contributor

This fixes #744 and cleans up a couple of related things discovered while down that rabbit hole.

The block RubyEnumerable creates to handle each_with_index was incorrectly assigned a fixed arity of two. When the each we passed this block to is implemented with a call rather than a yield, this arity was enforced. This resulted in wrapping zero or one arguments in an array (which is what leads to #744) or, in the case of more than two arguments, lopping off the extra arguments.

This pull updates the each_with_index block to have Arity.OPTIONAL, and fixes the EachWithIndex.call logic around packaging up the arguments to handle all arities properly.

Also noticed that RubyEnumerator$EachWithIndex needed precisely the same logic around argument packaging, and refactored it to use the fixed RubyEnumerable$EachWithIndex. In particular, this fixes an issue where RubyEnumerator$EachWithIndex handled multiple arguments incorrectly (it always truncated down to the first arg).

@dmarcotte dmarcotte Fix each_with_index argument handling
The block `RubyEnumerable` creates to handle each_with_index was
incorrectly assigned a fixed arity of two.  When the `each` we passed
this block to is implemented with a `call` rather than a `yield`, this
arity was enforced. This resulted in wrapping zero or one arguments in
an array or, in the case of more than two arguments, lopping off the
extra arguments.

Update the each_with_index block to have Arity.OPTIONAL, and fix the
EachWithIndex.call logic around packaging up the arguments to handle
all arities properly.

`RubyEnumerator$EachWithIndex` also needs precisely the same logic
around argument packaging, so refactor it to use the fixed
`RubyEnumerable$EachWithIndex`
43da339
@BanzaiMan BanzaiMan merged commit 3ecf9ae into jruby:master Jun 17, 2013

1 check was pending

default The Travis CI build is in progress
Details
@BanzaiMan
Member

I verified the specs, though they weren't run by CI (See https://travis-ci.org/jruby/jruby/jobs/8144748). I fixed this with 215ebe2.

Thanks, @dmarcotte!

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