[GR-16506] Better testing of BigDecimal for future changes.

PullRequest: truffleruby/903

Author: chrisseaton

CHANGELOG.md

@@ -20,6 +20,7 @@ Compatibility:
 * Implemented `rb_enc_sprintf` (#1702).
 * Implemented `ENV#{filter,filter!}` aliases for `select` and `select!`.
 * Non-blocking `StringIO` and `Socket` APIs now support `exception: false` like MRI (#1702).
+* Increased compatibility of `BigDecimal`.
 
 Changes:
 

lib/mri/bigdecimal/util.rb

@@ -68,11 +68,7 @@ class String
   #
   # TruffleRuby: MRI defines this method in C. We define it in Ruby for simplicity & clarity.
   def to_d
-    begin
-      BigDecimal(self)
-    rescue ArgumentError
-      BigDecimal(0)
-    end
+    Truffle.invoke_primitive :bigdecimal_new, self, Truffle::UNDEFINED, false
   end
 end
 

lib/truffle/bigdecimal.rb

@@ -207,7 +207,13 @@ def frac
   end
 
   def to_s(format = 'E')
-    if finite?
+    if zero?
+      if sign == SIGN_NEGATIVE_ZERO
+        '-0.0'
+      else
+        '0.0'
+      end
+    elsif finite?
       float_format    = format[-1] == 'F' || format[-1] == 'f'
       space_frequency = format.to_i
       prefix          = if self > 0 && [' ', '+'].include?(format[0])
@@ -321,7 +327,7 @@ def self.new(*args)
 
 module Kernel
   def BigDecimal(value, precision = Truffle::UNDEFINED)
-    Truffle.invoke_primitive :bigdecimal_new, value, precision
+    Truffle.invoke_primitive :bigdecimal_new, value, precision, true
   end
 end
 

spec/ruby/library/bigdecimal/BigDecimal_spec.rb

@@ -150,8 +150,13 @@
     BigDecimal("-12345.6E-1").should == -reference
   end
 
-  it 'raises ArgumentError when Float is used without precision' do
+  it "raises ArgumentError when Float is used without precision" do
     lambda { BigDecimal(1.0) }.should raise_error(ArgumentError)
   end
 
+  it "returns appropriate BigDecimal zero for signed zero" do
+    BigDecimal(-0.0, Float::DIG).sign.should == -1
+    BigDecimal(0.0, Float::DIG).sign.should == 1
+  end
+
 end

spec/ruby/library/bigdecimal/inspect_spec.rb

@@ -15,6 +15,12 @@
     @bigdec.inspect.should == "0.12345678e4"
   end
 
+  it "does not add an exponent for zero values" do
+    BigDecimal("0").inspect.should == "0.0"
+    BigDecimal("+0").inspect.should == "0.0"
+    BigDecimal("-0").inspect.should == "-0.0"
+  end
+
   it "properly cases non-finite values" do
     BigDecimal("NaN").inspect.should == "NaN"
     BigDecimal("Infinity").inspect.should == "Infinity"

spec/ruby/library/bigdecimal/to_d_spec.rb

@@ -2,7 +2,6 @@
 require 'bigdecimal'
 require 'bigdecimal/util'
 
-
 describe "Float#to_d" do
   it "returns appropriate BigDecimal zero for signed zero" do
     -0.0.to_d.sign.should == -1

spec/ruby/library/bigdecimal/to_s_spec.rb

@@ -19,6 +19,12 @@
     @bigdec.to_s.should =~ /^0\.[0-9]*e[0-9]*$/
   end
 
+  it "does not add an exponent for zero values" do
+    BigDecimal("0").to_s.should == "0.0"
+    BigDecimal("+0").to_s.should == "0.0"
+    BigDecimal("-0").to_s.should == "-0.0"
+  end
+
   it "takes an optional argument" do
     lambda {@bigdec.to_s("F")}.should_not raise_error()
   end

spec/tags/library/bigdecimal/BigDecimal_tags.txt

@@ -1,4 +1 @@
 slow:BigDecimal is not defined unless it is required
-fails:Kernel#BigDecimal raises ArgumentError for invalid strings
-fails:Kernel#BigDecimal does not ignores trailing garbage
-fails:Kernel#BigDecimal process underscores as Float()

spec/tags/library/bigdecimal/gt_tags.txt

@@ -1,2 +1 @@
 aot:BigDecimal.new treats invalid strings as 0.0
-fails:BigDecimal.new raises ArgumentError for invalid strings

spec/tags/library/bigdecimal/gte_tags.txt

@@ -198,6 +198,11 @@ public DynamicObject argumentError(Rope message, Node currentNode, Throwable jav
         return ExceptionOperations.createRubyException(context, exceptionClass, StringOperations.createString(context, message), currentNode, javaThrowable);
     }
 
+    @TruffleBoundary
+    public DynamicObject argumentErrorInvalidBigDecimal(String string, Node currentNode) {
+        return argumentError(StringUtils.format("invalid value for BigDecimal(): \"%s\"", string), currentNode);
+    }
+
     // FrozenError
 
     @TruffleBoundary

spec/tags/library/bigdecimal/limit_tags.txt

@@ -16,6 +16,8 @@
 
 public abstract class RubyGuards {
 
+    private static final long NEGATIVE_ZERO_DOUBLE_BITS = Double.doubleToRawLongBits(-0.0);
+
     // Basic Java types
 
     public static boolean isBoolean(Object value) {
@@ -74,6 +76,7 @@ public static boolean isPrimitiveClass(Class<?> clazz) {
         return clazz == Boolean.class || clazz == Byte.class || clazz == Short.class || clazz == Integer.class
                 || clazz == Long.class || clazz == Float.class || clazz == Double.class;
     }
+
     // Ruby types
 
     public static boolean isRubyBasicObject(Object object) {
@@ -313,10 +316,18 @@ public static boolean isInfinity(double value) {
         return Double.isInfinite(value);
     }
 
+    public static boolean isFinite(double value) {
+        return Double.isFinite(value);
+    }
+
     public static boolean isPositive(double value) {
         return value >= 0;
     }
 
+    public static boolean isNegativeZero(double value) {
+        return Double.doubleToRawLongBits(value) == NEGATIVE_ZERO_DOUBLE_BITS;
+    }
+
     // Composite
 
     public static boolean isSingletonClass(DynamicObject value) {

spec/tags/library/bigdecimal/lt_tags.txt

@@ -25,6 +25,9 @@ public abstract class AbstractAddNode extends BigDecimalOpNode {
     private final ConditionProfile aNormalProfile = ConditionProfile.createBinaryProfile();
 
     protected Object add(DynamicObject a, DynamicObject b, int precision) {
+        if (precision == 0) {
+            precision = getLimit();
+        }
         return createBigDecimal(addBigDecimal(a, b, new MathContext(precision, getRoundMode())));
     }
 

spec/tags/library/bigdecimal/lte_tags.txt

@@ -49,6 +49,9 @@ private BigDecimal divBigDecimal(BigDecimal a, BigDecimal b, MathContext mathCon
     }
 
     protected Object div(DynamicObject a, DynamicObject b, int precision) {
+        if (precision == 0) {
+            precision = getLimit();
+        }
         return createBigDecimal(divBigDecimalConsideringSignum(a, b, new MathContext(precision, getRoundMode())));
     }
 

spec/tags/library/bigdecimal/new_tags.txt

@@ -38,6 +38,9 @@ private Object multBigDecimal(BigDecimal a, BigDecimal b, MathContext mathContex
     }
 
     protected Object mult(DynamicObject a, DynamicObject b, int precision) {
+        if (precision == 0) {
+            precision = getLimit();
+        }
         return createBigDecimal(multBigDecimalConsideringSignum(a, b, new MathContext(precision, getRoundMode())));
     }
 

spec/tags/library/bigdecimal/to_d_tags.txt

@@ -30,6 +30,9 @@ private BigDecimal subBigDecimal(DynamicObject a, DynamicObject b, MathContext m
     }
 
     protected Object subNormal(DynamicObject a, DynamicObject b, int precision) {
+        if (precision == 0) {
+            precision = getLimit();
+        }
         return createBigDecimal(subBigDecimal(a, b, new MathContext(precision, getRoundMode())));
     }
 

spec/tags/library/bigdecimal/to_f_tags.txt

@@ -68,6 +68,7 @@ public BigDecimal doInt(long value, RoundingMode roundingMode) {
     @TruffleBoundary
     @Specialization
     public BigDecimal doDouble(double value, RoundingMode roundingMode) {
+        assert !RubyGuards.isNegativeZero(value);
         return BigDecimal.valueOf(value);
     }
 

src/main/java/org/truffleruby/core/exception/CoreExceptions.java

@@ -51,11 +51,15 @@ public static boolean isNan(DynamicObject value) {
     }
 
     protected DynamicObject createBigDecimal(Object value) {
-        return getCreateBigDecimal().executeCreate(value, NotProvided.INSTANCE);
+        return createBigDecimal(value, true);
     }
 
-    protected DynamicObject createBigDecimal(Object value, int digits) {
-        return getCreateBigDecimal().executeCreate(value, digits);
+    protected DynamicObject createBigDecimal(Object value, boolean strict) {
+        return getCreateBigDecimal().executeCreate(value, NotProvided.INSTANCE, strict);
+    }
+
+    protected DynamicObject createBigDecimal(Object value, int digits, boolean strict) {
+        return getCreateBigDecimal().executeCreate(value, digits, strict);
     }
 
     protected RoundingMode getRoundMode() {
@@ -110,7 +114,7 @@ protected int getLimit() {
     private CreateBigDecimalNode getCreateBigDecimal() {
         if (createBigDecimal == null) {
             CompilerDirectives.transferToInterpreterAndInvalidate();
-            createBigDecimal = insert(CreateBigDecimalNodeFactory.create(null, null));
+            createBigDecimal = insert(CreateBigDecimalNodeFactory.create(null, null, null));
         }
 
         return createBigDecimal;

src/main/java/org/truffleruby/language/RubyGuards.java

@@ -52,13 +52,13 @@ public abstract class BigDecimalNodes {
     public abstract static class NewNode extends BigDecimalCoreMethodArrayArgumentsNode {
 
         @Specialization
-        public Object newBigDecimal(Object value, NotProvided digits) {
-            return createBigDecimal(value);
+        public Object newBigDecimal(Object value, NotProvided notProvided, boolean strict) {
+            return createBigDecimal(value, strict);
         }
 
         @Specialization
-        public Object newBigDecimal(Object value, int digits) {
-            return createBigDecimal(value, digits);
+        public Object newBigDecimal(Object value, int digits, boolean strict) {
+            return createBigDecimal(value, digits, strict);
         }
 
     }
@@ -971,11 +971,27 @@ public int compare(DynamicObject a, long b) {
             return compareBigDecimal(a, BigDecimal.valueOf(b));
         }
 
-        @Specialization(guards = "isNormal(a)")
-        public int compare(DynamicObject a, double b) {
+        @Specialization(guards = {
+                "isNormal(a)",
+                "isFinite(b)"
+        })
+        public int compareFinite(DynamicObject a, double b) {
             return compareBigDecimal(a, valueOf(b));
         }
 
+        @Specialization(guards = {
+                "isNormal(a)",
+                "!isFinite(b)"
+        })
+        public Object compareNotFinite(DynamicObject a, double b) {
+            if (Double.isNaN(b)) {
+                return nil();
+            } else {
+                assert Double.isInfinite(b);
+                return b < 0 ? +1 : -1;
+            }
+        }
+
         @Specialization(guards = {
                 "isNormal(a)",
                 "isRubyBignum(b)"
@@ -997,11 +1013,28 @@ public Object compareSpecial(DynamicObject a, long b) {
             return compareSpecial(a, createBigDecimal(BigDecimal.valueOf(b)));
         }
 
-        @Specialization(guards = "!isNormal(a)")
-        public Object compareSpecial(DynamicObject a, double b) {
+        @Specialization(guards = {
+                "!isNormal(a)",
+                "isFinite(b)"
+        })
+        public Object compareSpecialFinite(DynamicObject a, double b) {
             return compareSpecial(a, createBigDecimal(valueOf(b)));
         }
 
+        @Specialization(guards = {
+                "!isNormal(a)",
+                "!isFinite(b)"
+        })
+        public Object compareSpecialInfinite(DynamicObject a, double b) {
+            final BigDecimalType type = Layouts.BIG_DECIMAL.getType(a);
+
+            if (type == BigDecimalType.NAN || Double.isNaN(b)) {
+                return nil();
+            } else {
+                return b < 0 ? +1 : -1;
+            }
+        }
+
         @Specialization(guards = {
                 "!isNormal(a)",
                 "isRubyBignum(b)"
@@ -1416,7 +1449,7 @@ public double toFSpecial(DynamicObject value,
                     return Double.POSITIVE_INFINITY;
                 case NEGATIVE_ZERO:
                     negZeroProfile.enter();
-                    return 0.0;
+                    return -0.0;
                 case NAN:
                     nanProfile.enter();
                     return Double.NaN;