Skip to content

Commit bd5053a

Browse files
authored
Merge pull request jruby#9337 from kares/bignum-cmp-10
[fix] Bignum comparison with Float in <=> and ==
2 parents 4fb0bbe + f87e77f commit bd5053a

2 files changed

Lines changed: 70 additions & 6 deletions

File tree

core/src/main/java/org/jruby/RubyBignum.java

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,6 +1017,20 @@ private IRubyObject float_cmp(ThreadContext context, RubyFloat y) {
10171017
return RubyFixnum.minus_one(runtime);
10181018
}
10191019

1020+
// mri : rb_integer_float_eq
1021+
private boolean float_eq(ThreadContext context, RubyFloat y) {
1022+
double yd = y.value;
1023+
1024+
if (Double.isNaN(yd) || Double.isInfinite(yd)) return false;
1025+
1026+
// if yd has a fractional part, it can't equal an integer
1027+
if (Math.floor(yd) != yd) return false;
1028+
1029+
// compare as integers to avoid precision loss
1030+
IRubyObject rel = op_cmp(context, newBignorm(context.runtime, yd));
1031+
return rel instanceof RubyFixnum fix && fix.getLongValue() == 0;
1032+
}
1033+
10201034
@Override
10211035
public final int compareTo(IRubyObject other) {
10221036
if (other instanceof RubyBignum bignum) return value.compareTo(bignum.value);
@@ -1045,9 +1059,7 @@ public IRubyObject op_cmp(ThreadContext context, IRubyObject other) {
10451059
} else if (other instanceof RubyBignum big) {
10461060
otherValue = big.value;
10471061
} else if (other instanceof RubyFloat flt) {
1048-
return flt.isInfinite() ?
1049-
asFixnum(context, flt.value > 0.0 ? -1 : 1) :
1050-
dbl_cmp(context.runtime, big2dbl(this), flt.value);
1062+
return float_cmp(context, flt);
10511063
} else {
10521064
return coerceCmp(context, sites(context).op_cmp, other);
10531065
}
@@ -1067,9 +1079,7 @@ public IRubyObject op_equal(ThreadContext context, IRubyObject other) {
10671079
} else if (other instanceof RubyBignum bignum) {
10681080
otherValue = bignum.value;
10691081
} else if (other instanceof RubyFloat flote) {
1070-
double a = flote.value;
1071-
if (Double.isNaN(a) || Double.isInfinite(a)) return context.fals;
1072-
return asBoolean(context, a == big2dbl(this));
1082+
return asBoolean(context, float_eq(context, flote));
10731083
} else {
10741084
return other.op_eqq(context, this);
10751085
}
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
require 'rspec'
2+
3+
# Regression test for Bignum <=> and == with Float precision.
4+
#
5+
# When comparing a large Bignum against a Float, the comparison must
6+
# not lose precision by converting the Bignum to double. For example,
7+
# 2**64 + 1 cannot be exactly represented as a double (it rounds to
8+
# 2**64), so (2**64 + 1) <=> (2**64).to_f must return 1, not 0.
9+
#
10+
# The bug was that op_cmp used dbl_cmp(big2dbl(this), flt.value)
11+
# which lost precision, and op_equal used big2dbl comparison which
12+
# had the same problem.
13+
14+
describe "Bignum comparison with Float precision" do
15+
context "<=>" do
16+
it "returns 1 when Bignum is greater than Float despite double rounding" do
17+
expect((2**64 + 1) <=> (2**64).to_f).to eq 1
18+
expect((2**100 + 1) <=> (2**100).to_f).to eq 1
19+
end
20+
21+
it "returns 0 when Bignum exactly equals Float" do
22+
expect((2**64) <=> (2**64).to_f).to eq 0
23+
end
24+
25+
it "returns -1 when Bignum is less than Float" do
26+
expect((2**64 - 1) <=> (2**64).to_f).to eq(-1)
27+
end
28+
29+
it "returns nil for NaN" do
30+
expect((2**100) <=> Float::NAN).to be_nil
31+
end
32+
33+
it "handles Infinity correctly" do
34+
expect((2**100) <=> Float::INFINITY).to eq(-1)
35+
expect((2**100) <=> -Float::INFINITY).to eq 1
36+
end
37+
end
38+
39+
context "==" do
40+
it "returns false when Bignum differs from Float due to precision" do
41+
expect((2**64 + 1) == (2**64).to_f).to be false
42+
expect((2**100 + 1) == (2**100).to_f).to be false
43+
end
44+
45+
it "returns true when Bignum exactly equals Float" do
46+
expect((2**64) == (2**64).to_f).to be true
47+
end
48+
49+
it "returns false for NaN and Infinity" do
50+
expect((2**100) == Float::NAN).to be false
51+
expect((2**100) == Float::INFINITY).to be false
52+
end
53+
end
54+
end

0 commit comments

Comments
 (0)