Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't dup every element in Enumerator#drop (1.7) #4226

Merged
merged 1 commit into from Oct 18, 2016

Conversation

iconara
Copy link
Contributor

@iconara iconara commented Oct 13, 2016

This fixes #4218 in JRuby 1.7.

In 06f0441 (JRUBY-6892) Enumerable#drop was changed to call #dup on every element that ended up in the result, but that might not work. Not all objects correctly implement #dup, or are even allocatable.

#take was not changed, and looking at the differences between #take and #drop the only thing that stood out was that the signature of the #each call was different. Changing from Arity.NO_ARGUMENTS to Arity.ONE_REQUIRED make the issue that 06f0441 tried to fix go away.

In 06f0441 (JRUBY-6892) Enumerable#drop was changed to call #dup on every element that ended up in the result, but that might not work. Not all objects correctly implement #dup, or are even allocatable.

#take was not changed, and looking at the differences between #take and #drop the only thing that stood out was that the signature of the #each call was different. Changing from Arity.NO_ARGUMENTS to Arity.ONE_REQUIRED make the issue that 06f0441 tried to fix go away.
@iconara iconara changed the title Fix 4218 1.7 Fix #4218 in JRuby 1.7 Oct 13, 2016
@iconara iconara changed the title Fix #4218 in JRuby 1.7 Don't dup every element in Enumerator#drop (1.7) Oct 13, 2016
@iconara iconara changed the base branch from master to 1_7_20 October 13, 2016 14:15
@iconara iconara changed the base branch from 1_7_20 to jruby-1_7 October 13, 2016 14:16
@iconara
Copy link
Contributor Author

iconara commented Oct 13, 2016

I'm unsure which branch that this PR should target, please help me out. The fix should go into the next 1.7.x release.

I'm also unable to run the tests in spec/regression, so I actually don't know if this works (I'm expecting it to since it's exactly the same fix as in #4225 that targets 9K).

@kares
Copy link
Member

kares commented Oct 16, 2016

you got the right branch (jruby-1_7) ... somehow we do not have a travis-ci run for this PR not sure why.

@headius
Copy link
Member

headius commented Oct 18, 2016

I'll verify locally.

@headius
Copy link
Member

headius commented Oct 18, 2016

Seems fine locally. I'll merge.

@headius headius added this to the JRuby 1.7.27 milestone Oct 18, 2016
@headius headius merged commit 6cba9d3 into jruby:jruby-1_7 Oct 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants