Move Fixnum#remainder up to Numeric and clean up isNeg/Pos.

jruby · Feb 23, 2018 · e8ef793 · e8ef793
1 parent 0f3746b
commit e8ef793
Show file tree

Hide file tree

Showing 6 changed files with 45 additions and 43 deletions.
diff --git a/core/src/main/java/org/jruby/RubyFixnum.java b/core/src/main/java/org/jruby/RubyFixnum.java
@@ -1383,22 +1383,6 @@ public RubyRational convertToRational() {
         return RubyRational.newRationalRaw(runtime, this, one(runtime));
     }
 
-    @Override
-    public IRubyObject remainder(ThreadContext context, IRubyObject y) {
-        RubyFixnum x = this;
-        JavaSites.FixnumSites sites = sites(context);
-        IRubyObject z = sites.op_mod.call(context, this, this, y);
-
-        if ((!Helpers.rbEqual(context, z, RubyFixnum.zero(context.runtime), sites.op_equal).isTrue()) &&
-                ((x.isNegative() &&
-                        ((RubyInteger) y).isPositive()) ||
-                        (x.isPositive() &&
-                                ((RubyInteger) y).isNegative()))) {
-            return sites.op_minus.call(context, z, z, y);
-        }
-        return z;
-    }
-
     private static JavaSites.FixnumSites sites(ThreadContext context) {
         return context.sites.Fixnum;
     }

diff --git a/core/src/main/java/org/jruby/RubyFloat.java b/core/src/main/java/org/jruby/RubyFloat.java
@@ -200,13 +200,23 @@ public int signum() {
     @Override
     @JRubyMethod(name = "negative?")
     public IRubyObject isNegative(ThreadContext context) {
-        return context.runtime.newBoolean(signum() < 0);
+        return context.runtime.newBoolean(isNegative());
     }
 
     @Override
     @JRubyMethod(name = "positive?")
     public IRubyObject isPositive(ThreadContext context) {
-        return context.runtime.newBoolean(signum() > 0);
+        return context.runtime.newBoolean(isPositive());
+    }
+
+    @Override
+    public boolean isNegative() {
+        return signum() < 0;
+    }
+
+    @Override
+    public boolean isPositive() {
+        return signum() > 0;
     }
 
     public static RubyFloat newFloat(Ruby runtime, double value) {

diff --git a/core/src/main/java/org/jruby/RubyInteger.java b/core/src/main/java/org/jruby/RubyInteger.java
@@ -123,13 +123,13 @@ public IRubyObject isPositive(ThreadContext context) {
         return context.runtime.newBoolean(signum() > 0);
     }
 
-    // @Override // NOTE: could be public + defined on Numeric
-    final boolean isNegative() {
+    @Override
+    public boolean isNegative() {
         return signum() < 0;
     }
 
-    // @Override // NOTE: could be public + defined on Numeric
-    final boolean isPositive() {
+    @Override
+    public boolean isPositive() {
         return signum() > 0;
     }
 

diff --git a/core/src/main/java/org/jruby/RubyNumeric.java b/core/src/main/java/org/jruby/RubyNumeric.java
@@ -809,16 +809,20 @@ public IRubyObject modulo(ThreadContext context, IRubyObject other) {
      *
      */
     @JRubyMethod(name = "remainder")
-    public IRubyObject remainder(ThreadContext context, IRubyObject dividend) {
-        IRubyObject z = numFuncall(context, this, sites(context).op_mod, dividend);
-        RubyFixnum zero = RubyFixnum.zero(context.runtime);
+    public IRubyObject remainder(ThreadContext context, IRubyObject y) {
+        RubyNumeric x = this;
+        JavaSites.NumericSites sites = sites(context);
+        IRubyObject z = sites.op_mod.call(context, this, this, y);
+
+        // non-numeric would error out in % call
+        RubyNumeric yNum = (RubyNumeric) y;
 
-        if (!equalInternal(context, z, zero) &&
-                ((isNegative(context).isTrue() &&
-                        positiveIntP(context, dividend).isTrue()) ||
-                (isPositive(context).isTrue() &&
-                        negativeIntP(context, dividend).isTrue()))) {
-            return sites(context).op_minus.call(context, z, z, dividend);
+        if ((!Helpers.rbEqual(context, z, RubyFixnum.zero(context.runtime), sites.op_equal).isTrue()) &&
+                ((x.isNegative() &&
+                        yNum.isPositive()) ||
+                        (x.isPositive() &&
+                                yNum.isNegative()))) {
+            return sites.op_minus.call(context, z, z, y);
         }
         return z;
     }
@@ -1374,8 +1378,8 @@ public Throwable fillInStackTrace() {
     @JRubyMethod(name = "negative?")
     public IRubyObject isNegative(ThreadContext context) {
         return compareWithZero(context, this, sites(context).op_lt_checked);
-
     }
+
     /** num_positive_p
      *
      */
@@ -1384,18 +1388,12 @@ public IRubyObject isPositive(ThreadContext context) {
         return compareWithZero(context, this, sites(context).op_gt_checked);
     }
 
-    protected static IRubyObject negativeIntP(ThreadContext context, IRubyObject obj) {
-        if (obj instanceof RubyNumeric) {
-            return ((RubyNumeric) obj).isNegative(context);
-        }
-        return compareWithZero(context, obj, sites(context).op_lt_checked);
+    public boolean isNegative() {
+        return isNegative(getRuntime().getCurrentContext()).isTrue();
     }
 
-    protected static IRubyObject positiveIntP(ThreadContext context, IRubyObject obj) {
-        if (obj instanceof RubyNumeric) {
-            return ((RubyNumeric) obj).isPositive(context);
-        }
-        return compareWithZero(context, obj, sites(context).op_gt_checked);
+    public boolean isPositive() {
+        return isPositive(getRuntime().getCurrentContext()).isTrue();
     }
 
     protected static IRubyObject compareWithZero(ThreadContext context, IRubyObject num, JavaSites.CheckedSites site) {

diff --git a/core/src/main/java/org/jruby/RubyRational.java b/core/src/main/java/org/jruby/RubyRational.java
@@ -471,6 +471,16 @@ public IRubyObject isPositive(ThreadContext context) {
         return context.runtime.newBoolean(signum() > 0);
     }
 
+    @Override
+    public boolean isNegative() {
+        return signum() < 0;
+    }
+
+    @Override
+    public boolean isPositive() {
+        return signum() > 0;
+    }
+
     public final int signum() { return num.signum(); }
 
     /** f_imul

diff --git a/core/src/main/java/org/jruby/runtime/JavaSites.java b/core/src/main/java/org/jruby/runtime/JavaSites.java
@@ -198,6 +198,7 @@ public static class NumericSites {
         public final CallSite op_rshift = new FunctionalCachingCallSite(">>");
         public final CallSite size = new FunctionalCachingCallSite("size");
         public final CallSite ceil = new FunctionalCachingCallSite("ceil");
+        public final CallSite op_equal = new FunctionalCachingCallSite("==");
     }
 
     public static class IntegerSites {
@@ -236,7 +237,6 @@ public static class FixnumSites {
         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_equal = new FunctionalCachingCallSite("==");
     }
 
     public static class BignumSites {