Improve BigDecimal compatibility

Author: chrisseaton

CHANGELOG.md

@@ -18,6 +18,7 @@ Compatibility:
 * `blocking: true` is now supported for `FFI::Library#attach_function`.
 * Implemented `Proc#>>` and `#<<` (#1688).
 * `Thread.report_on_exception` is now `true` by default like MRI 2.5+.
+* `BigDecimal` compatibility has been generally improved in several ways.
 
 Changes:
 

lib/truffle/bigdecimal.rb

@@ -331,4 +331,20 @@ def BigDecimal(value, precision = Truffle::UNDEFINED)
   end
 end
 
+module Truffle::BigDecimalOperations
+
+  def self.coerce_integer_to_bigdecimal(value)
+    Truffle.invoke_primitive(:bigdecimal_new, value, Truffle::UNDEFINED, true)
+  end
+
+  def self.coerce_float_to_bigdecimal(value)
+    Truffle.invoke_primitive(:bigdecimal_new, value.to_s, Truffle::UNDEFINED, true)
+  end
+
+  def self.coerce_rational_to_bigdecimal(value)
+    Truffle.invoke_primitive(:bigdecimal_new, value.to_f.to_s, Truffle::UNDEFINED, true)
+  end
+
+end
+
 require 'bigdecimal/math'

spec/ruby/core/integer/coerce_spec.rb

@@ -1,5 +1,7 @@
 require_relative '../../spec_helper'
 
+require 'bigdecimal'
+
 describe "Integer#coerce" do
   context "fixnum" do
     describe "when given a Fixnum" do
@@ -88,4 +90,15 @@
       ary.should == [1.2, a.to_f]
     end
   end
+
+  context "bigdecimal" do
+    it "produces Floats" do
+      x, y = 3.coerce(BigDecimal("3.4"))
+      x.class.should == Float
+      x.should == 3.4
+      y.class.should == Float
+      y.should == 3.0
+    end
+  end
+
 end

spec/ruby/library/bigdecimal/add_spec.rb

@@ -81,6 +81,12 @@
     end
   end
 
+  describe "with Rational" do
+    it "produces a BigDecimal" do
+      (@three + Rational(500, 2)).should == BigDecimal("0.253e3")
+    end
+  end
+
   it "favors the precision specified in the second argument over the global limit" do
     BigDecimalSpecs.with_limit(1) do
       BigDecimal('0.888').add(@zero, 3).should == BigDecimal('0.888')

spec/ruby/library/bigdecimal/divide_spec.rb

@@ -4,4 +4,14 @@
 
 describe "BigDecimal#/" do
   it_behaves_like :bigdecimal_quo, :/, []
+
+  before :each do
+    @three = BigDecimal("3")
+  end
+
+  describe "with Rational" do
+    it "produces a BigDecimal" do
+      (@three / Rational(500, 2)).should == BigDecimal("0.12e-1")
+    end
+  end
 end

spec/ruby/library/bigdecimal/multiply_spec.rb

@@ -8,6 +8,7 @@
 
 describe "BigDecimal#*" do
   before :each do
+    @three = BigDecimal("3")
     @e3_minus = BigDecimal("3E-20001")
     @e3_plus = BigDecimal("3E20001")
     @e = BigDecimal("1.00000000000000000000123456789")
@@ -31,4 +32,10 @@
       (@e3_minus * object).should == BigDecimal("9")
     end
   end
+
+  describe "with Rational" do
+    it "produces a BigDecimal" do
+      (@three * Rational(500, 2)).should == BigDecimal("0.75e3")
+    end
+  end
 end

spec/ruby/library/bigdecimal/sub_spec.rb

@@ -7,6 +7,7 @@
     @one = BigDecimal("1")
     @zero = BigDecimal("0")
     @two = BigDecimal("2")
+    @three = BigDecimal("3")
     @nan = BigDecimal("NaN")
     @infinity = BigDecimal("Infinity")
     @infinity_minus = BigDecimal("-Infinity")
@@ -42,6 +43,12 @@
     end
   end
 
+  describe "with Rational" do
+    it "produces a BigDecimal" do
+      (@three - Rational(500, 2)).should == BigDecimal('-0.247e3')
+    end
+  end
+
   it "returns NaN if NaN is involved" do
     @one.sub(@nan, 1).nan?.should == true
     @nan.sub(@one, 1).nan?.should == true

spec/ruby/shared/rational/coerce.rb

@@ -1,5 +1,7 @@
 require_relative '../../spec_helper'
 
+require 'bigdecimal'
+
 describe :rational_coerce, shared: true do
   it "returns the passed argument, self as Float, when given a Float" do
     result = Rational(3, 4).coerce(1.0)
@@ -18,4 +20,10 @@
   it "returns [argument, self] when given a Rational" do
     Rational(3, 7).coerce(Rational(9, 2)).should == [Rational(9, 2), Rational(3, 7)]
   end
+
+  it "raises an error when passed a BigDecimal" do
+    -> {
+      Rational(500, 3).coerce(BigDecimal('166.666666666'))
+    }.should raise_error(TypeError, /BigDecimal can't be coerced into Rational/)
+  end
 end

spec/tags/truffle/bigdecimal_tags.txt

@@ -0,0 +1 @@
+fails:BigDecimal bug GR-16506 produces the expected result

spec/truffle/bigdecimal_spec.rb

@@ -0,0 +1,103 @@
+# Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. This
+# code is released under a tri EPL/GPL/LGPL license. You can use it,
+# redistribute it and/or modify it under the terms of the:
+#
+# Eclipse Public License version 1.0, or
+# GNU General Public License version 2, or
+# GNU Lesser General Public License version 2.1.
+
+require_relative '../ruby/spec_helper'
+
+require 'bigdecimal'
+
+describe "BigDecimal" do
+
+  describe "bug GR-16506" do
+
+    before :each do
+      @a = BigDecimal('166.666666666')
+      @b = Rational(500, 3)
+      @c = @a - @b
+    end
+
+    # Check the input is as we understand it
+
+    it "has the LHS print as expected" do
+      @a.to_s.should == "0.166666666666e3"
+      @a.to_f.to_s.should == "166.666666666"
+      Float(@a).to_s.should == "166.666666666"
+    end
+
+    it "has the RHS print as expected" do
+      @b.to_s.should == "500/3"
+      @b.to_f.to_s.should == "166.66666666666666"
+      Float(@b).to_s.should == "166.66666666666666"
+    end
+
+    it "has the expected precision on the LHS" do
+      @a.precs[0].should == 18
+    end
+
+    it "has the expected maximum precision on the LHS" do
+      @a.precs[1].should == 27
+    end
+
+    it "produces the expected result when done via Float" do
+      (Float(@a) - Float(@b)).to_s.should == "-6.666596163995564e-10"
+    end
+
+    it "produces the expected result when done via to_f" do
+      (@a.to_f - @b.to_f).to_s.should == "-6.666596163995564e-10"
+    end
+
+    # Check underlying methods work as we understand
+
+    it "BigDecimal precision is the number of digits rounded up to a multiple of nine" do
+      1.upto(100) do |n|
+        b = BigDecimal('4' * n)
+        precs, _ = b.precs
+        (precs >= 9).should be_true
+        (precs >= n).should be_true
+        (precs % 9).should == 0
+      end
+      BigDecimal('NaN').precs[0].should == 9
+    end
+
+    it "BigDecimal maximum precision is nine more than precision except for abnormals" do
+      1.upto(100) do |n|
+        b = BigDecimal('4' * n)
+        precs, max = b.precs
+        max.should == precs + 9
+      end
+      BigDecimal('NaN').precs[1].should == 9
+    end
+
+    it "BigDecimal(Rational, 18) produces the result we expect" do
+      BigDecimal(@b, 18).to_s.should == "0.166666666666666667e3"
+    end
+
+    it "BigDecimal(Rational, BigDecimal.precs[0]) produces the result we expect" do
+      BigDecimal(@b, @a.precs[0]).to_s.should == "0.166666666666666667e3"
+    end
+
+    # Check the top-level expression works as we expect
+
+    it "produces a BigDecimal" do
+      @c.class.should == BigDecimal
+    end
+
+    it "produces the expected result" do
+      @c.should == BigDecimal("-0.666667e-9")
+      @c.to_s.should == "-0.666667e-9"
+    end
+
+    it "produces the correct class for other artihmetic operators" do
+      (@a + @b).class.should == BigDecimal
+      (@a * @b).class.should == BigDecimal
+      (@a / @b).class.should == BigDecimal
+      (@a % @b).class.should == BigDecimal
+    end
+
+  end
+
+end

src/main/java/org/truffleruby/core/CoreLibrary.java

@@ -176,6 +176,7 @@ public class CoreLibrary {
     private final DynamicObject truffleRegexpOperationsModule;
     private final DynamicObject truffleThreadOperationsModule;
     private final DynamicObject bigDecimalClass;
+    private final DynamicObject bigDecimalOperationsModule;
     private final DynamicObject encodingCompatibilityErrorClass;
     private final DynamicObject encodingUndefinedConversionErrorClass;
     private final DynamicObject methodClass;
@@ -546,6 +547,7 @@ public CoreLibrary(RubyContext context) {
 
         bigDecimalClass = defineClass(numericClass, "BigDecimal");
         Layouts.CLASS.setInstanceFactoryUnsafe(bigDecimalClass, Layouts.BIG_DECIMAL.createBigDecimalShape(bigDecimalClass, bigDecimalClass));
+        bigDecimalOperationsModule = defineModule(truffleModule, "BigDecimalOperations");
 
         truffleFFIModule = defineModule(truffleModule, "FFI");
         DynamicObject truffleFFIAbstractMemoryClass = defineClass(truffleFFIModule, objectClass, "AbstractMemory");
@@ -955,6 +957,10 @@ public DynamicObject getBigDecimalClass() {
         return bigDecimalClass;
     }
 
+    public DynamicObject getBigDecimalOperationsModule() {
+        return bigDecimalOperationsModule;
+    }
+
     public DynamicObjectFactory getBindingFactory() {
         return bindingFactory;
     }

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

@@ -198,7 +198,6 @@ 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);
     }

src/main/java/org/truffleruby/stdlib/bigdecimal/AbstractAddNode.java

@@ -22,7 +22,7 @@ public abstract class AbstractAddNode extends BigDecimalOpNode {
     private final ConditionProfile nanProfile = ConditionProfile.createBinaryProfile();
     private final ConditionProfile posInfinityProfile = ConditionProfile.createBinaryProfile();
     private final ConditionProfile negInfinityProfile = ConditionProfile.createBinaryProfile();
-    private final ConditionProfile aNormalProfile = ConditionProfile.createBinaryProfile();
+    private final ConditionProfile normalProfile = ConditionProfile.createBinaryProfile();
 
     protected Object add(DynamicObject a, DynamicObject b, int precision) {
         if (precision == 0) {
@@ -53,7 +53,7 @@ protected Object addSpecial(DynamicObject a, DynamicObject b, int precision) {
 
         // One is NEGATIVE_ZERO and second is NORMAL
 
-        if (aNormalProfile.profile(isNormal(a))) {
+        if (normalProfile.profile(isNormal(a))) {
             return a;
         } else {
             return b;

src/main/java/org/truffleruby/stdlib/bigdecimal/AbstractSubNode.java

@@ -24,12 +24,7 @@ public abstract class AbstractSubNode extends BigDecimalOpNode {
     private final ConditionProfile negInfinityProfile = ConditionProfile.createBinaryProfile();
     private final ConditionProfile normalProfile = ConditionProfile.createBinaryProfile();
 
-    @TruffleBoundary
-    private BigDecimal subBigDecimal(DynamicObject a, DynamicObject b, MathContext mathContext) {
-        return Layouts.BIG_DECIMAL.getValue(a).subtract(Layouts.BIG_DECIMAL.getValue(b), mathContext);
-    }
-
-    protected Object subNormal(DynamicObject a, DynamicObject b, int precision) {
+    protected Object sub(DynamicObject a, DynamicObject b, int precision) {
         if (precision == 0) {
             precision = getLimit();
         }
@@ -64,4 +59,10 @@ protected Object subSpecial(DynamicObject a, DynamicObject b, int precision) {
             return createBigDecimal(Layouts.BIG_DECIMAL.getValue(b).negate());
         }
     }
+
+    @TruffleBoundary
+    private BigDecimal subBigDecimal(DynamicObject a, DynamicObject b, MathContext mathContext) {
+        return Layouts.BIG_DECIMAL.getValue(a).subtract(Layouts.BIG_DECIMAL.getValue(b), mathContext);
+    }
+
 }