Add respond_to-aware hashCode and equals to RubyBasicObject.

We use Object#hashCode and #equals in our Hash impl among other places. When the object key is RubyObject or a descendant, we would dispatch dynamically to the appropriate methods in Ruby. However, RubyBasicObject did not override either method, so they both just used base JVM impls and ignored user-defined #hash and This patch adds "checked" versions of these methods to RubyBasicObject and fixes #3227.
jruby · Aug 6, 2015 · df6cf57 · df6cf57
1 parent ee7fa54
commit df6cf57
Showing 4 changed files with 66 additions and 18 deletions.
diff --git a/core/src/main/java/org/jruby/RubyBasicObject.java b/core/src/main/java/org/jruby/RubyBasicObject.java
@@ -56,6 +56,7 @@
 import org.jruby.runtime.Visibility;
 
 import static org.jruby.anno.FrameField.*;
+import static org.jruby.runtime.Helpers.invokeChecked;
 import static org.jruby.runtime.Visibility.*;
 import org.jruby.exceptions.RaiseException;
 import org.jruby.runtime.Arity;
@@ -73,6 +74,7 @@
 import org.jruby.util.unsafe.UnsafeHolder;
 
 import static org.jruby.runtime.Helpers.invokedynamic;
+import static org.jruby.runtime.invokedynamic.MethodNames.HASH;
 import static org.jruby.runtime.invokedynamic.MethodNames.OP_EQUAL;
 import static org.jruby.runtime.invokedynamic.MethodNames.OP_CMP;
 import static org.jruby.runtime.invokedynamic.MethodNames.EQL;
@@ -2822,6 +2824,46 @@ public RubyArray instance_variables19(ThreadContext context) {
         return array;
     }
 
+    /**
+     * This method is just a wrapper around the Ruby "==" method,
+     * provided so that RubyObjects can be used as keys in the Java
+     * HashMap object underlying RubyHash.
+     */
+    @Override
+    public boolean equals(Object other) {
+        if (other == this) return true;
+
+        if (other instanceof IRubyObject) {
+            IRubyObject equals = invokeChecked(getRuntime().getCurrentContext(), this, "==", (IRubyObject)other);
+            if (equals == null) return false;
+            return equals.isTrue();
+        }
+
+        return false;
+    }
+
+    /**
+     * Override the Object#hashCode method to make sure that the Ruby
+     * hash is actually used as the hashcode for Ruby objects. If the
+     * Ruby "hash" method doesn't return a number, the Object#hashCode
+     * implementation will be used instead.
+     */
+    @Override
+    public int hashCode() {
+        IRubyObject hashValue = invokeChecked(getRuntime().getCurrentContext(), this, "hash");
+        if (hashValue == null) return super.hashCode();
+        if (hashValue instanceof RubyFixnum) return (int) RubyNumeric.fix2long(hashValue);
+        return nonFixnumHashCode(hashValue);
+    }
+
+    protected static int nonFixnumHashCode(IRubyObject hashValue) {
+        RubyInteger integer = hashValue.convertToInteger();
+        if (integer instanceof RubyBignum) {
+            return integer.getBigIntegerValue().intValue();
+        }
+        return (int) integer.getLongValue();
+    }
+
     /**
      * Checks if the name parameter represents a legal instance variable name, and otherwise throws a Ruby NameError
      */

diff --git a/core/src/main/java/org/jruby/RubyClass.java b/core/src/main/java/org/jruby/RubyClass.java
@@ -622,16 +622,25 @@ public IRubyObject finvoke(ThreadContext context, IRubyObject self, String name)
      * MRI: rb_check_funcall
      */
     public IRubyObject finvokeChecked(ThreadContext context, IRubyObject self, String name) {
+        return finvokeChecked(context, self, name, IRubyObject.NULL_ARRAY);
+    }
+
+    /**
+     * Safely attempt to invoke the given method name on self, using respond_to? and method_missing as appropriate.
+     *
+     * MRI: rb_check_funcall
+     */
+    public IRubyObject finvokeChecked(ThreadContext context, IRubyObject self, String name, IRubyObject... args) {
         RubyClass klass = self.getMetaClass();
         DynamicMethod me;
         if (!checkFuncallRespondTo(context, self.getMetaClass(), self, name))
             return null;
 
         me = searchMethod(name);
         if (!checkFuncallCallable(context, me, CallType.FUNCTIONAL, self)) {
-            return checkFuncallMissing(context, klass, self, name);
+            return checkFuncallMissing(context, klass, self, name, args);
         }
-        return me.call(context, self, klass, name);
+        return me.call(context, self, klass, name, args);
     }
 
     // MRI: check_funcall_exec
@@ -689,6 +698,7 @@ public static boolean rbMethodCallStatus(ThreadContext context, DynamicMethod me
         return method != null && !method.isUndefined() && method.isCallableFrom(self, callType);
     }
 
+    // MRI: check_funcall_missing
     private static IRubyObject checkFuncallMissing(ThreadContext context, RubyClass klass, IRubyObject self, String method, IRubyObject... args) {
         Ruby runtime = context.runtime;
         if (klass.isMethodBuiltin("method_missing")) {

diff --git a/core/src/main/java/org/jruby/RubyObject.java b/core/src/main/java/org/jruby/RubyObject.java
@@ -319,9 +319,9 @@ public static void puts(Object obj) {
     }
 
     /**
-     * This method is just a wrapper around the Ruby "==" method,
-     * provided so that RubyObjects can be used as keys in the Java
-     * HashMap object underlying RubyHash.
+     * This override does not do a "checked" dispatch.
+     *
+     * @see RubyBasicObject#equals(Object)
      */
     @Override
     public boolean equals(Object other) {
@@ -478,7 +478,7 @@ protected static boolean equalInternal(final ThreadContext context, final IRubyO
         } else if (a instanceof RubySymbol) {
             return false;
         } else if (a instanceof RubyFixnum && b instanceof RubyFixnum) {
-            return ((RubyFixnum)a).fastEqual((RubyFixnum)b);
+            return ((RubyFixnum)a).fastEqual((RubyFixnum) b);
         } else if (a instanceof RubyFloat && b instanceof RubyFloat) {
             return ((RubyFloat)a).fastEqual((RubyFloat)b);
         } else {
@@ -504,10 +504,9 @@ protected static boolean eqlInternal(final ThreadContext context, final IRubyObj
     }
 
     /**
-     * Override the Object#hashCode method to make sure that the Ruby
-     * hash is actually used as the hashcode for Ruby objects. If the
-     * Ruby "hash" method doesn't return a number, the Object#hashCode
-     * implementation will be used instead.
+     * This override does not do "checked" dispatch since Object usually has #hash defined.
+     *
+     * @see RubyBasicObject#hashCode()
      */
     @Override
     public int hashCode() {
@@ -516,14 +515,6 @@ public int hashCode() {
         return nonFixnumHashCode(hashValue);
     }
 
-    private int nonFixnumHashCode(IRubyObject hashValue) {
-        RubyInteger integer = hashValue.convertToInteger();
-        if (integer instanceof RubyBignum) {
-            return integer.getBigIntegerValue().intValue();
-        }
-        return (int) integer.getLongValue();
-    }
-
     /** rb_inspect
      *
      * The internal helper that ensures a RubyString instance is returned

diff --git a/core/src/main/java/org/jruby/runtime/Helpers.java b/core/src/main/java/org/jruby/runtime/Helpers.java
@@ -445,6 +445,11 @@ public static IRubyObject invokeChecked(ThreadContext context, IRubyObject self,
         return self.getMetaClass().finvokeChecked(context, self, name);
     }
 
+    // MRI: rb_check_funcall
+    public static IRubyObject invokeChecked(ThreadContext context, IRubyObject self, String name, IRubyObject... args) {
+        return self.getMetaClass().finvokeChecked(context, self, name, args);
+    }
+
     /**
      * The protocol for super method invocation is a bit complicated
      * in Ruby. In real terms it involves first finding the real