Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions core/src/main/java/org/jruby/RubyBignum.java
Original file line number Diff line number Diff line change
Expand Up @@ -1017,6 +1017,20 @@ private IRubyObject float_cmp(ThreadContext context, RubyFloat y) {
return RubyFixnum.minus_one(runtime);
}

// mri : rb_integer_float_eq
private boolean float_eq(ThreadContext context, RubyFloat y) {
double yd = y.value;

if (Double.isNaN(yd) || Double.isInfinite(yd)) return false;

// if yd has a fractional part, it can't equal an integer
if (Math.floor(yd) != yd) return false;

// compare as integers to avoid precision loss
IRubyObject rel = op_cmp(context, newBignorm(context.runtime, yd));
return rel instanceof RubyFixnum fix && fix.getLongValue() == 0;
}

@Override
public final int compareTo(IRubyObject other) {
if (other instanceof RubyBignum bignum) return value.compareTo(bignum.value);
Expand Down Expand Up @@ -1045,9 +1059,7 @@ public IRubyObject op_cmp(ThreadContext context, IRubyObject other) {
} else if (other instanceof RubyBignum big) {
otherValue = big.value;
} else if (other instanceof RubyFloat flt) {
return flt.isInfinite() ?
asFixnum(context, flt.value > 0.0 ? -1 : 1) :
dbl_cmp(context.runtime, big2dbl(this), flt.value);
return float_cmp(context, flt);
} else {
return coerceCmp(context, sites(context).op_cmp, other);
}
Expand All @@ -1067,9 +1079,7 @@ public IRubyObject op_equal(ThreadContext context, IRubyObject other) {
} else if (other instanceof RubyBignum bignum) {
otherValue = bignum.value;
} else if (other instanceof RubyFloat flote) {
double a = flote.value;
if (Double.isNaN(a) || Double.isInfinite(a)) return context.fals;
return asBoolean(context, a == big2dbl(this));
return asBoolean(context, float_eq(context, flote));
} else {
return other.op_eqq(context, this);
}
Expand Down
54 changes: 54 additions & 0 deletions spec/regression/bignum_float_comparison_precision_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
require 'rspec'

# Regression test for Bignum <=> and == with Float precision.
#
# When comparing a large Bignum against a Float, the comparison must
# not lose precision by converting the Bignum to double. For example,
# 2**64 + 1 cannot be exactly represented as a double (it rounds to
# 2**64), so (2**64 + 1) <=> (2**64).to_f must return 1, not 0.
#
# The bug was that op_cmp used dbl_cmp(big2dbl(this), flt.value)
# which lost precision, and op_equal used big2dbl comparison which
# had the same problem.

describe "Bignum comparison with Float precision" do
context "<=>" do
it "returns 1 when Bignum is greater than Float despite double rounding" do
expect((2**64 + 1) <=> (2**64).to_f).to eq 1
expect((2**100 + 1) <=> (2**100).to_f).to eq 1
end

it "returns 0 when Bignum exactly equals Float" do
expect((2**64) <=> (2**64).to_f).to eq 0
end

it "returns -1 when Bignum is less than Float" do
expect((2**64 - 1) <=> (2**64).to_f).to eq(-1)
end

it "returns nil for NaN" do
expect((2**100) <=> Float::NAN).to be_nil
end

it "handles Infinity correctly" do
expect((2**100) <=> Float::INFINITY).to eq(-1)
expect((2**100) <=> -Float::INFINITY).to eq 1
end
end

context "==" do
it "returns false when Bignum differs from Float due to precision" do
expect((2**64 + 1) == (2**64).to_f).to be false
expect((2**100 + 1) == (2**100).to_f).to be false
end

it "returns true when Bignum exactly equals Float" do
expect((2**64) == (2**64).to_f).to be true
end

it "returns false for NaN and Infinity" do
expect((2**100) == Float::NAN).to be false
expect((2**100) == Float::INFINITY).to be false
end
end
end
Loading