Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 48cc9ba915b3
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: 4e775c76e31d
Choose a head ref
  • 2 commits
  • 2 files changed
  • 2 contributors

Commits on Oct 13, 2016

  1. Don't dup every element in Enumerator#drop

    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 Signature.NO_ARGUMENTS to Signature.ONE_REQUIRED make the issue that 06f0441 tried to fix go away.
    iconara committed Oct 13, 2016
    Copy the full SHA
    6996e2e View commit details

Commits on Nov 8, 2016

  1. Merge pull request #4225 from iconara/fix-4218-9k

    Don't dup every element in Enumerator#drop (9K)
    headius authored Nov 8, 2016
    Copy the full SHA
    4e775c7 View commit details
Showing with 23 additions and 6 deletions.
  1. +3 −6 core/src/main/java/org/jruby/RubyEnumerable.java
  2. +20 −0 spec/regression/GH-4218_enumerable_drop_should_not_change_identities.rb
9 changes: 3 additions & 6 deletions core/src/main/java/org/jruby/RubyEnumerable.java
Original file line number Diff line number Diff line change
@@ -355,16 +355,13 @@ public static IRubyObject drop(ThreadContext context, IRubyObject self, IRubyObj
final RubyArray result = runtime.newArray();

try {
each(context, self, new JavaInternalBlockBody(runtime, Signature.NO_ARGUMENTS) {
each(context, self, new JavaInternalBlockBody(runtime, Signature.ONE_REQUIRED) {
long i = len; // Atomic ?
public IRubyObject yield(ThreadContext context, IRubyObject[] args) {
IRubyObject packedArg = packEnumValues(context.runtime, args);
synchronized (result) {
if (i == 0) {
// While iterating over an RubyEnumerator, "arg"
// gets overwritten by the new value, leading to JRUBY-6892.
// So call .dup() whenever appropriate.
result.append(packedArg.isImmediate() ? packedArg : packedArg.dup());
IRubyObject packedArg = packEnumValues(context.runtime, args);
result.append(packedArg);
} else {
--i;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
class GH4218Enumerable
include Enumerable

def initialize(elements)
@elements = elements
end

def each(&block)
@elements.each(&block)
end
end

describe '#4218 Enumerable#drop' do
it 'does not change the identity of the elements' do
original = [Object.new, Object.new, Object.new]
enumerable = GH4218Enumerable.new(original)
expect(original[1]).to be_equal(enumerable.drop(1).first)
expect(enumerable.to_a[1]).to be_equal(enumerable.drop(1).first)
end
end