Coercion fixes for bitwise operators on Integer and custom types.

jruby · Jul 9, 2017 · 8275435 · 8275435
1 parent 0697344
commit 8275435
Showing 4 changed files with 40 additions and 48 deletions.
diff --git a/core/src/main/java/org/jruby/RubyBignum.java b/core/src/main/java/org/jruby/RubyBignum.java
@@ -41,7 +41,6 @@
 import org.jruby.common.IRubyWarnings.ID;
 import org.jruby.runtime.ClassIndex;
 import org.jruby.runtime.JavaSites;
-import org.jruby.runtime.ObjectAllocator;
 import org.jruby.runtime.ThreadContext;
 import org.jruby.runtime.builtin.IRubyObject;
 import org.jruby.runtime.marshal.MarshalStream;
@@ -735,7 +734,7 @@ public IRubyObject op_and(ThreadContext context, IRubyObject other) {
         } else if (other instanceof RubyFixnum) {
             return bignorm(context.runtime, value.and(fix2big((RubyFixnum)other)));
         }
-        return coerceBit(context, sites(context).op_and, other);
+        return coerceBit(context, sites(context).checked_op_and, other);
     }
 
     @Deprecated
@@ -754,7 +753,7 @@ public IRubyObject op_or(ThreadContext context, IRubyObject other) {
         if (other instanceof RubyFixnum) { // no bignorm here needed
             return bignorm(context.runtime, value.or(fix2big((RubyFixnum)other)));
         }
-        return coerceBit(context, sites(context).op_or, other);
+        return coerceBit(context, sites(context).checked_op_or, other);
     }
 
     @Override
@@ -778,7 +777,7 @@ public IRubyObject op_xor(ThreadContext context, IRubyObject other) {
         if (other instanceof RubyFixnum) {
             return bignorm(context.runtime, value.xor(BigInteger.valueOf(((RubyFixnum) other).getLongValue())));
         }
-        return coerceBit(context, sites(context).op_xor, other);
+        return coerceBit(context, sites(context).checked_op_xor, other);
     }
 
     @Deprecated

diff --git a/core/src/main/java/org/jruby/RubyFixnum.java b/core/src/main/java/org/jruby/RubyFixnum.java
@@ -93,17 +93,6 @@ private static IRubyObject fixCoerce(IRubyObject x) {
         return x;
     }
 
-    private IRubyObject bitCoerce(ThreadContext context, IRubyObject y) {
-        if(!(y instanceof RubyFixnum || y instanceof RubyBignum)) {
-            RubyArray ary = doCoerce(context, y, true);
-            y = ary.last();
-            if(!(y instanceof RubyFixnum || y instanceof RubyBignum)) {
-                coerceFailed(context, y);
-            }
-        }
-        return y;
-    }
-
     public RubyFixnum(Ruby runtime) {
         this(runtime, 0);
     }
@@ -1120,18 +1109,13 @@ public IRubyObject op_neg(ThreadContext context) {
      */
     @Override
     public IRubyObject op_and(ThreadContext context, IRubyObject other) {
-        if (!((other = bitCoerce(context, other)) instanceof RubyFixnum)) {
-            return ((RubyBignum) other).op_and(context, this);
+        if (other instanceof RubyFixnum) {
+            return context.runtime.newFixnum(value & ((RubyFixnum) other).value);
         }
-
-        return op_andOther(context, other);
-    }
-
-    private IRubyObject op_andOther(ThreadContext context, IRubyObject other) {
-        if (other instanceof RubyFixnum || (other = fixCoerce(other)) instanceof RubyFixnum) {
-            return newFixnum(context.runtime, value & ((RubyFixnum) other).value);
+        if (other instanceof RubyBignum) {
+            return ((RubyBignum) other).op_and(context, this);
         }
-        return ((RubyBignum) other).op_and(context, this);
+        return coerceBit(context, sites(context).checked_op_and, other);
     }
 
     public IRubyObject op_and(ThreadContext context, long other) {
@@ -1143,11 +1127,13 @@ public IRubyObject op_and(ThreadContext context, long other) {
      */
     @Override
     public IRubyObject op_or(ThreadContext context, IRubyObject other) {
-        if ((other = bitCoerce(context, other)) instanceof RubyFixnum) {
-            return newFixnum(context.runtime, value | ((RubyFixnum) other).value);
+        if (other instanceof RubyFixnum) {
+            return context.runtime.newFixnum(value | ((RubyFixnum) other).value);
         }
-
-        return ((RubyBignum) other).op_or(context, this);
+        if (other instanceof RubyBignum) {
+            return ((RubyBignum) other).op_or(context, this);
+        }
+        return coerceBit(context, sites(context).checked_op_or, other);
     }
 
     public IRubyObject op_or(ThreadContext context, long other) {
@@ -1159,13 +1145,13 @@ public IRubyObject op_or(ThreadContext context, long other) {
      */
     @Override
     public IRubyObject op_xor(ThreadContext context, IRubyObject other) {
-        if (!((other = bitCoerce(context, other)) instanceof RubyFixnum)) {
-            return ((RubyBignum) other).op_xor(context, this);
+        if (other instanceof RubyFixnum) {
+            return context.runtime.newFixnum(value ^ ((RubyFixnum) other).value);
         }
-        if (other instanceof RubyFixnum || (other = fixCoerce(other)) instanceof RubyFixnum) {
-            return newFixnum(context.runtime, value ^ ((RubyFixnum) other).value);
+        if (other instanceof RubyBignum) {
+            return ((RubyBignum) other).op_and(context, this);
         }
-        return ((RubyBignum) other).op_xor(context, this);
+        return coerceBit(context, sites(context).checked_op_xor, other);
     }
 
     public IRubyObject op_xor(ThreadContext context, long other) {

diff --git a/core/src/main/java/org/jruby/RubyNumeric.java b/core/src/main/java/org/jruby/RubyNumeric.java
@@ -559,19 +559,23 @@ protected final IRubyObject coerceBit(ThreadContext context, String method, IRub
         return callMethod(context, method, other);
     }
 
-    protected final IRubyObject coerceBit(ThreadContext context, CallSite site, IRubyObject other) {
-        if (!(other instanceof RubyFixnum) && !(other instanceof RubyBignum)) {
-            RubyArray ary = doCoerce(context, other, true);
-            IRubyObject x = ary.eltInternal(0);
-            IRubyObject y = ary.eltInternal(1);
-
-            if (!(x instanceof RubyFixnum) && !(x instanceof RubyBignum)
-                    && !(y instanceof RubyFixnum) && !(y instanceof RubyBignum)) {
-                coerceFailed(context, other);
+    protected final IRubyObject coerceBit(ThreadContext context, JavaSites.CheckedSites site, IRubyObject other) {
+        RubyArray ary = doCoerce(context, other, true);
+        final IRubyObject x = ary.eltOk(0);
+        IRubyObject y = ary.eltOk(1);
+        IRubyObject ret = context.safeRecurse(new ThreadContext.RecursiveFunctionEx<JavaSites.CheckedSites>() {
+            @Override
+            public IRubyObject call(ThreadContext context, JavaSites.CheckedSites site, IRubyObject obj, boolean recur) {
+                if (recur) {
+                    throw context.runtime.newNameError("recursive call to " + site.methodName, site.methodName);
+                }
+                return x.getMetaClass().finvokeChecked(context, x, site, obj);
             }
-            return numFuncall(context, x, site, y);
+        }, site, y, site.methodName, true);
+        if (ret == null) {
+            coerceFailed(context, other);
         }
-        return numFuncall(context, this, site, other);
+        return ret;
     }
 
     /** rb_num_coerce_cmp

diff --git a/core/src/main/java/org/jruby/runtime/JavaSites.java b/core/src/main/java/org/jruby/runtime/JavaSites.java
@@ -224,6 +224,9 @@ public static class FixnumSites {
         public final CallSite op_lt_bignum = new FunctionalCachingCallSite("<");
         public final CallSite op_exp_rational = new FunctionalCachingCallSite("**");
         public final CallSite fdiv = new FunctionalCachingCallSite("fdiv");
+        public final CheckedSites checked_op_and = new CheckedSites("&");
+        public final CheckedSites checked_op_or = new CheckedSites("|");
+        public final CheckedSites checked_op_xor = new CheckedSites("^");
     }
 
     public static class BignumSites {
@@ -237,9 +240,9 @@ public static class BignumSites {
         public final CallSite op_times = new FunctionalCachingCallSite("*");
         public final CallSite quo = new FunctionalCachingCallSite("quo");
         public final CallSite remainder = new FunctionalCachingCallSite("remainder");
-        public final CallSite op_and = new FunctionalCachingCallSite("&");
-        public final CallSite op_or = new FunctionalCachingCallSite("|");
-        public final CallSite op_xor = new FunctionalCachingCallSite("^");
+        public final CheckedSites checked_op_and = new CheckedSites("&");
+        public final CheckedSites checked_op_or = new CheckedSites("|");
+        public final CheckedSites checked_op_xor = new CheckedSites("^");
         public final CallSite op_cmp = new FunctionalCachingCallSite("<=>");
         public final CallSite fdiv = new FunctionalCachingCallSite("fdiv");
     }