work-around TreeSet internals (used to track SortedSet's order)

TreeSet assumes compare == 0 -> equals == true ... but this is not how Ruby's SortedSet works (at least in MRI 2.4) resolves GH-5035
jruby · Feb 12, 2018 · af82b10 · af82b10
1 parent acd9a53
commit af82b10
Showing 2 changed files with 76 additions and 1 deletion.
diff --git a/core/src/main/java/org/jruby/ext/set/RubySortedSet.java b/core/src/main/java/org/jruby/ext/set/RubySortedSet.java
@@ -30,7 +30,8 @@
 
 import org.jruby.*;
 import org.jruby.anno.JRubyMethod;
-import org.jruby.runtime.*;
+import org.jruby.runtime.ObjectAllocator;
+import org.jruby.runtime.ThreadContext;
 import org.jruby.runtime.builtin.IRubyObject;
 
 import java.util.*;
@@ -72,6 +73,18 @@ private static class OrderComparator extends DefaultComparator {
         protected ThreadContext context() {
             return runtime.getCurrentContext();
         }
+
+        // NOTE: need a custom impl so that we use special care when compare returns 0
+        // this is required since/if we're using a TreeSet which assumes 0 -> eql == true
+
+        public int compare(IRubyObject obj1, IRubyObject obj2) {
+            final int cmp = super.compare(obj1, obj2);
+            if (cmp == 0) {
+                return equalInternal(context(), obj1, obj2) ? 0 : 1;
+            }
+            return cmp;
+        }
+
     }
 
     private final TreeSet<IRubyObject> order;

diff --git a/test/jruby/test_set.rb b/test/jruby/test_set.rb
@@ -129,4 +129,66 @@ def test_to_java
     assert_equal java.util.TreeSet.new([1, 2]), set
   end if defined? JRUBY_VERSION
 
+  def test_cmp_0_but_not_eql
+    set = Set[1, 2]
+    assert_equal set.to_a, set.dup.to_a
+    assert_equal set.to_a, set.clone.dup.to_a
+
+    set = SortedSet.new
+    set << cmp1 = CmpObj1.new(Time.at(0))
+    set << cmp2 = CmpObj1.new(Time.at(1))
+    set << cmp3 = CmpObj1.new(Time.at(0))
+    assert_equal 3, set.size
+    assert_equal 3, set.to_a.size
+    assert_equal SortedSet[cmp1, cmp3, cmp2], set
+    assert_equal [cmp1, cmp3, cmp2], set.to_a
+  end
+
+  class CmpObj1
+    attr_reader :time
+
+    def initialize(time); @time = time end
+
+    def <=>(other)
+      time <=> other.time
+    end
+
+    def ==(other)
+      object_id == other.object_id
+    end
+  end
+
+
+  def test_cmp_0_but_not_eql2
+    set = Set[1, 2]
+    assert_equal set.to_a, set.dup.to_a
+    assert_equal set.to_a, set.clone.dup.to_a
+
+    set = SortedSet.new
+    set << cmp1 = CmpObj2.new(Time.at(0))
+    set << cmp2 = CmpObj2.new(Time.at(2))
+    set << cmp3 = CmpObj2.new(Time.at(0))
+    set << cmp4 = CmpObj2.new(Time.at(0))
+    set << cmp5 = CmpObj2.new(Time.at(1))
+    set << cmp6 = CmpObj2.new(Time.at(0))
+    assert_equal 6, set.size
+    assert_equal 6, set.to_a.size
+    assert_equal [cmp1, cmp3, cmp4, cmp6, cmp5, cmp2], set.to_a
+    assert_equal SortedSet[cmp6, cmp5, cmp4, cmp3, cmp2, cmp1], set
+  end
+
+  class CmpObj2
+    attr_reader :time
+
+    def initialize(time); @time = time end
+
+    def <=>(other)
+      time <=> other.time
+    end
+
+    def eql?(other)
+      object_id == other.object_id
+    end
+  end
+
 end