Fix each_with_index argument handling #807
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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`
BanzaiMan
added a commit
that referenced
this pull request
Jun 17, 2013
Fix each_with_index argument handling
|
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #744 and cleans up a couple of related things discovered while down that rabbit hole.
The block
RubyEnumerablecreates to handleeach_with_indexwas incorrectly assigned a fixed arity of two. When theeachwe passed this block to is implemented with acallrather than ayield, 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_indexblock to haveArity.OPTIONAL, and fixes theEachWithIndex.calllogic around packaging up the arguments to handle all arities properly.Also noticed that
RubyEnumerator$EachWithIndexneeded precisely the same logic around argument packaging, and refactored it to use the fixedRubyEnumerable$EachWithIndex. In particular, this fixes an issue whereRubyEnumerator$EachWithIndexhandled multiple arguments incorrectly (it always truncated down to the first arg).