diff --git a/common/types/compare.go b/common/types/compare.go index e1968261..2e7c999b 100644 --- a/common/types/compare.go +++ b/common/types/compare.go @@ -20,28 +20,54 @@ import ( "github.com/google/cel-go/common/types/ref" ) +// compareDoubleInt orders a double against an int by their mathematical values. +// +// The int is split from the double rather than converted to one: ints above 2^53 have no exact +// double representation, so converting rounds them to a neighbouring value. Callers screen NaN +// before dispatching here. func compareDoubleInt(d Double, i Int) Int { - if d < math.MinInt64 { + f := float64(d) + if f < math.MinInt64 { + return IntNegOne + } + // MaxInt64 rounds up to 2^63 as a double, which is the first value above the int64 range. + if f >= math.MaxInt64 { + return IntOne + } + // The integral part is now within the int64 range, so it converts exactly. + whole := math.Trunc(f) + if Int(whole) < i { return IntNegOne } - if d > math.MaxInt64 { + if Int(whole) > i { return IntOne } - return compareDouble(d, Double(i)) + // Equal whole parts, so the remaining fraction decides the order. + return compareDouble(Double(f-whole), 0) } func compareIntDouble(i Int, d Double) Int { return -compareDoubleInt(d, i) } +// compareDoubleUint orders a double against a uint by their mathematical values, splitting the +// double instead of converting the uint for the reason given on compareDoubleInt. func compareDoubleUint(d Double, u Uint) Int { - if d < 0 { + f := float64(d) + if f < 0 { + return IntNegOne + } + if f >= doubleTwoTo64 { + return IntOne + } + whole := math.Trunc(f) + if Uint(whole) < u { return IntNegOne } - if d > math.MaxUint64 { + if Uint(whole) > u { return IntOne } - return compareDouble(d, Double(u)) + return compareDouble(Double(f-whole), 0) } func compareUintDouble(u Uint, d Double) Int { diff --git a/common/types/double_test.go b/common/types/double_test.go index 47e875f2..984b24b4 100644 --- a/common/types/double_test.go +++ b/common/types/double_test.go @@ -116,6 +116,23 @@ func TestDoubleCompare(t *testing.T) { b: Int(204), out: IntOne, }, + { + // 2^53+1 has no double representation, so it must not compare equal to 2^53. + a: Double(1 << 53), + b: Int(1<<53) + 1, + out: IntNegOne, + }, + { + a: Double(1 << 53), + b: Uint(1<<53) + 1, + out: IntNegOne, + }, + { + // MaxInt64 rounds up to 2^63 as a double, but remains below it as an int. + a: Double(1 << 63), + b: Int(math.MaxInt64), + out: IntOne, + }, { a: Double(1), b: String("1"), @@ -407,6 +424,26 @@ func TestDoubleEqual(t *testing.T) { b: Int(10), out: False, }, + { + a: Double(1 << 53), + b: Int(1<<53) + 1, + out: False, + }, + { + a: Double(1 << 53), + b: Uint(1<<53) + 1, + out: False, + }, + { + a: Double(1 << 63), + b: Int(math.MaxInt64), + out: False, + }, + { + a: Double(math.MaxUint64), + b: Uint(math.MaxUint64), + out: False, + }, } for _, tc := range tests { got := tc.a.Equal(tc.b) diff --git a/common/types/int_test.go b/common/types/int_test.go index 560c3dfb..a8bc9b56 100644 --- a/common/types/int_test.go +++ b/common/types/int_test.go @@ -129,6 +129,17 @@ func TestIntCompare(t *testing.T) { b: Double(math.MinInt64) - 1025.0, out: IntOne, }, + { + // 2^53+1 has no double representation, so it must not compare equal to 2^53. + a: Int(1<<53) + 1, + b: Double(1 << 53), + out: IntOne, + }, + { + a: Int(math.MaxInt64), + b: Double(1 << 63), + out: IntNegOne, + }, { a: Int(1), b: String("1"), @@ -429,6 +440,16 @@ func TestIntEqual(t *testing.T) { b: Double(math.NaN()), out: False, }, + { + a: Int(1<<53) + 1, + b: Double(1 << 53), + out: False, + }, + { + a: Int(math.MaxInt64), + b: Double(1 << 63), + out: False, + }, { a: Int(1), b: String("1"), diff --git a/common/types/uint_test.go b/common/types/uint_test.go index 2484fbf3..9a174350 100644 --- a/common/types/uint_test.go +++ b/common/types/uint_test.go @@ -117,6 +117,17 @@ func TestUintCompare(t *testing.T) { b: Double(-1.0), out: IntOne, }, + { + // 2^53+1 has no double representation, so it must not compare equal to 2^53. + a: Uint(1<<53) + 1, + b: Double(1 << 53), + out: IntOne, + }, + { + a: Uint(math.MaxUint64), + b: Double(math.MaxUint64), + out: IntNegOne, + }, { a: Uint(1), b: String("1"), @@ -379,6 +390,16 @@ func TestUintEqual(t *testing.T) { b: Double(math.NaN()), out: False, }, + { + a: Uint(1<<53) + 1, + b: Double(1 << 53), + out: False, + }, + { + a: Uint(math.MaxUint64), + b: Double(math.MaxUint64), + out: False, + }, } for _, tc := range tests { got := tc.a.Equal(tc.b)