From 79b11985a3703f1186da774583e271be0e186cb1 Mon Sep 17 00:00:00 2001 From: Matt Oestreich Date: Thu, 5 Mar 2026 07:53:47 -0600 Subject: [PATCH 1/3] Handle Sub Overflow Edge Case --- src/value/arithmetic.rs | 92 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 91 insertions(+), 1 deletion(-) diff --git a/src/value/arithmetic.rs b/src/value/arithmetic.rs index 1b9fa70..f1a28fe 100644 --- a/src/value/arithmetic.rs +++ b/src/value/arithmetic.rs @@ -14,6 +14,17 @@ impl CheckedAdd for f64 { } } +// shim for the missing method on f64 +trait CheckedSub: Sized + ops::Sub { + fn checked_sub(self, rhs: Self) -> Option; +} + +impl CheckedSub for f64 { + fn checked_sub(self, rhs: Self) -> Option { + Some(self - rhs) + } +} + impl ops::AddAssign for Value where Rhs: Into, @@ -48,7 +59,11 @@ where if rhs > *self { self.promote_to_signed(); } - dispatch_operation!(self, rhs, n, |rhs| *n -= rhs); + *self = dispatch_operation!(self, rhs, n, |rhs| (*n).checked_sub(rhs).map(Value::from)) + .unwrap_or_else(|| { + self.promote(); + dispatch_operation!(self, rhs, n, |rhs| Value::from(*n - rhs)) + }) } } @@ -382,6 +397,81 @@ mod tests { assert_eq!(result, (-1_i64).into()); } + #[test] + fn sub_overflow_promotes_to_signed_or_float() { + // ------------------------------------------------------------------------ + // -- sub overflow u64 bounds, result should promote to SignedBigInt + // ------------------------------------------------------------------------ + let left: Value = u64::MIN.into(); + let right: Value = u64::MAX.into(); + let result = left - right; + let expected_order = Order::SignedBigInt; + assert_eq!( + result.order(), + expected_order, + "sub overflow should promote this to {expected_order:?} : got = {result:?}" + ); + let expected_value = (-18446744073709551615_i128).into(); + assert_eq!( + result, expected_value, + "sub overflow value should be {expected_value:?} : got = {result:?}" + ); + + // ------------------------------------------------------------------------ + // -- sub overflow i64 bounds, result should promote to SignedBigInt + // ------------------------------------------------------------------------ + let left: Value = i64::MIN.into(); + let right: Value = i64::MAX.into(); + let result = left - right; + let expected_order = Order::SignedBigInt; + assert_eq!( + result.order(), + expected_order, + "sub overflow should promote this to {expected_order:?} : got = {result:?}" + ); + let expected_value = (-18446744073709551615_i128).into(); + assert_eq!( + result, expected_value, + "sub overflow value should be {expected_value:?} : got = {result:?}" + ); + + // ------------------------------------------------------------------------ + // -- sub overflow u128 bounds, result should promote to Float + // ------------------------------------------------------------------------ + let left: Value = u128::MIN.into(); + let right: Value = u128::MAX.into(); + let result = left - right; + let expected_order = Order::Float; + assert_eq!( + result.order(), + expected_order, + "sub overflow should promote this to {expected_order:?} : got = {result:?}" + ); + let expected_value = (-3.402823669209385e38).into(); + assert_eq!( + result, expected_value, + "sub overflow value should be {expected_value:?} : got = {result:?}" + ); + + // ------------------------------------------------------------------------ + // -- sub overflow i128 bounds, result should promote to Float + // ------------------------------------------------------------------------ + let left: Value = i128::MIN.into(); + let right: Value = i128::MAX.into(); + let result = left - right; + let expected_order = Order::Float; + assert_eq!( + result.order(), + expected_order, + "sub overflow should promote this to {expected_order:?} : got = {result:?}" + ); + let expected_value = (-3.402823669209385e38).into(); + assert_eq!( + result, expected_value, + "sub overflow value should be {expected_value:?} : got = {result:?}" + ); + } + // ---------- INFINITY PROPAGATION ---------- #[test] fn inf_plus_finite_is_inf() { From 7059d07c09ec92faa5921567adaeaea61249bd78 Mon Sep 17 00:00:00 2001 From: Matt Oestreich Date: Fri, 6 Mar 2026 11:48:08 -0600 Subject: [PATCH 2/3] update tests to more accurately reflect issue at hand --- src/value/arithmetic.rs | 48 ++++++----------------------------------- 1 file changed, 6 insertions(+), 42 deletions(-) diff --git a/src/value/arithmetic.rs b/src/value/arithmetic.rs index f1a28fe..17c1860 100644 --- a/src/value/arithmetic.rs +++ b/src/value/arithmetic.rs @@ -399,29 +399,11 @@ mod tests { #[test] fn sub_overflow_promotes_to_signed_or_float() { - // ------------------------------------------------------------------------ - // -- sub overflow u64 bounds, result should promote to SignedBigInt - // ------------------------------------------------------------------------ - let left: Value = u64::MIN.into(); - let right: Value = u64::MAX.into(); - let result = left - right; - let expected_order = Order::SignedBigInt; - assert_eq!( - result.order(), - expected_order, - "sub overflow should promote this to {expected_order:?} : got = {result:?}" - ); - let expected_value = (-18446744073709551615_i128).into(); - assert_eq!( - result, expected_value, - "sub overflow value should be {expected_value:?} : got = {result:?}" - ); - // ------------------------------------------------------------------------ // -- sub overflow i64 bounds, result should promote to SignedBigInt // ------------------------------------------------------------------------ - let left: Value = i64::MIN.into(); - let right: Value = i64::MAX.into(); + let left: Value = 0_i64.into(); + let right: Value = i64::MIN.into(); let result = left - right; let expected_order = Order::SignedBigInt; assert_eq!( @@ -429,25 +411,7 @@ mod tests { expected_order, "sub overflow should promote this to {expected_order:?} : got = {result:?}" ); - let expected_value = (-18446744073709551615_i128).into(); - assert_eq!( - result, expected_value, - "sub overflow value should be {expected_value:?} : got = {result:?}" - ); - - // ------------------------------------------------------------------------ - // -- sub overflow u128 bounds, result should promote to Float - // ------------------------------------------------------------------------ - let left: Value = u128::MIN.into(); - let right: Value = u128::MAX.into(); - let result = left - right; - let expected_order = Order::Float; - assert_eq!( - result.order(), - expected_order, - "sub overflow should promote this to {expected_order:?} : got = {result:?}" - ); - let expected_value = (-3.402823669209385e38).into(); + let expected_value = (9223372036854775808_i128).into(); assert_eq!( result, expected_value, "sub overflow value should be {expected_value:?} : got = {result:?}" @@ -456,8 +420,8 @@ mod tests { // ------------------------------------------------------------------------ // -- sub overflow i128 bounds, result should promote to Float // ------------------------------------------------------------------------ - let left: Value = i128::MIN.into(); - let right: Value = i128::MAX.into(); + let left: Value = 0_i128.into(); + let right: Value = i128::MIN.into(); let result = left - right; let expected_order = Order::Float; assert_eq!( @@ -465,7 +429,7 @@ mod tests { expected_order, "sub overflow should promote this to {expected_order:?} : got = {result:?}" ); - let expected_value = (-3.402823669209385e38).into(); + let expected_value = (1.7014118346046923e38).into(); assert_eq!( result, expected_value, "sub overflow value should be {expected_value:?} : got = {result:?}" From c5339008cd581a8d8861a3775fd67e6b6863e518 Mon Sep 17 00:00:00 2001 From: Matt Oestreich Date: Fri, 6 Mar 2026 14:40:33 -0600 Subject: [PATCH 3/3] separate test cases into own tests --- src/value/arithmetic.rs | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/src/value/arithmetic.rs b/src/value/arithmetic.rs index 17c1860..22d4ba8 100644 --- a/src/value/arithmetic.rs +++ b/src/value/arithmetic.rs @@ -397,42 +397,28 @@ mod tests { assert_eq!(result, (-1_i64).into()); } - #[test] - fn sub_overflow_promotes_to_signed_or_float() { - // ------------------------------------------------------------------------ - // -- sub overflow i64 bounds, result should promote to SignedBigInt - // ------------------------------------------------------------------------ - let left: Value = 0_i64.into(); - let right: Value = i64::MIN.into(); + #[rstest] + #[case::i64_overflows_to_i128(0_i64, i64::MIN, Order::SignedBigInt, 9223372036854775808_i128)] + #[case::i128_overflows_to_float(0_i128, i128::MIN, Order::Float, 1.7014118346046923e38)] + fn sub_overflow_promotes_to_signed_or_float( + #[case] left: impl Into, + #[case] right: impl Into, + #[case] expected_order: Order, + #[case] expected_value: impl Into, + ) { + let left = left.into(); + let right = right.into(); let result = left - right; - let expected_order = Order::SignedBigInt; - assert_eq!( - result.order(), - expected_order, - "sub overflow should promote this to {expected_order:?} : got = {result:?}" - ); - let expected_value = (9223372036854775808_i128).into(); - assert_eq!( - result, expected_value, - "sub overflow value should be {expected_value:?} : got = {result:?}" - ); - // ------------------------------------------------------------------------ - // -- sub overflow i128 bounds, result should promote to Float - // ------------------------------------------------------------------------ - let left: Value = 0_i128.into(); - let right: Value = i128::MIN.into(); - let result = left - right; - let expected_order = Order::Float; assert_eq!( result.order(), expected_order, "sub overflow should promote this to {expected_order:?} : got = {result:?}" ); - let expected_value = (1.7014118346046923e38).into(); + let expected_value = expected_value.into(); assert_eq!( result, expected_value, - "sub overflow value should be {expected_value:?} : got = {result:?}" + "sub overflow value should be {expected_value:?} : got = {result:?}", ); }