From 11de139a6d35259a6898e06dc32a0bd56eb68c98 Mon Sep 17 00:00:00 2001 From: Matt Oestreich Date: Fri, 6 Mar 2026 20:10:03 -0600 Subject: [PATCH 1/3] resolve arithmetic overflow promotion --- src/value/arithmetic.rs | 133 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 121 insertions(+), 12 deletions(-) diff --git a/src/value/arithmetic.rs b/src/value/arithmetic.rs index 22d4ba8..55dd561 100644 --- a/src/value/arithmetic.rs +++ b/src/value/arithmetic.rs @@ -4,25 +4,40 @@ use super::dispatch_operation; use crate::Value; // shim for the missing method on f64 -trait CheckedAdd: Sized + ops::Add { +trait CheckedMaths: + Sized + + ops::Add + + ops::Sub + + ops::Div + + ops::Rem +{ fn checked_add(self, rhs: Self) -> Option; + fn checked_sub(self, rhs: Self) -> Option; + fn checked_div(self, rhs: Self) -> Option; + fn checked_mul(self, rhs: Self) -> Option; + fn checked_rem(self, rhs: Self) -> Option; } -impl CheckedAdd for f64 { +impl CheckedMaths for f64 { fn checked_add(self, rhs: Self) -> Option { Some(self + rhs) } -} -// 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) } + + fn checked_div(self, rhs: Self) -> Option { + Some(self / rhs) + } + + fn checked_mul(self, rhs: Self) -> Option { + Some(self * rhs) + } + + fn checked_rem(self, rhs: Self) -> Option { + Some(self % rhs) + } } impl ops::AddAssign for Value @@ -85,7 +100,11 @@ where { fn mul_assign(&mut self, rhs: Rhs) { let mut rhs = rhs.into(); - dispatch_operation!(self, rhs, n, |rhs| *n *= rhs); + *self = dispatch_operation!(self, rhs, n, |rhs| (*n).checked_mul(rhs).map(Value::from)) + .unwrap_or_else(|| { + self.promote(); + dispatch_operation!(self, rhs, n, |rhs| Value::from(*n * rhs)) + }); } } @@ -109,7 +128,11 @@ where self.promote_to_float(); let mut rhs = rhs.into(); rhs.promote_to_float(); - dispatch_operation!(self, rhs, n, |rhs| *n /= rhs); + *self = dispatch_operation!(self, rhs, n, |rhs| (*n).checked_div(rhs).map(Value::from)) + .unwrap_or_else(|| { + self.promote(); + dispatch_operation!(self, rhs, n, |rhs| Value::from(*n / rhs)) + }); } } @@ -131,7 +154,11 @@ where { fn rem_assign(&mut self, rhs: Rhs) { let mut rhs = rhs.into(); - dispatch_operation!(self, rhs, n, |rhs| *n %= rhs); + *self = dispatch_operation!(self, rhs, n, |rhs| (*n).checked_rem(rhs).map(Value::from)) + .unwrap_or_else(|| { + self.promote(); + dispatch_operation!(self, rhs, n, |rhs| Value::from(*n % rhs)) + }); } } @@ -422,6 +449,88 @@ mod tests { ); } + #[rstest] + #[case::i64_overflows_to_i128( + i64::MAX, + i64::MAX, + Order::SignedBigInt, + 85070591730234615847396907784232501249_i128 + )] + #[case::i128_overflows_to_float(i128::MAX, i128::MAX, Order::Float, 2.894802230932905e76)] + fn mul_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; + + assert_eq!( + result.order(), + expected_order, + "mul overflow should promote this to {expected_order:?} : got = {result:?}" + ); + let expected_value = expected_value.into(); + assert_eq!( + result, expected_value, + "mul overflow value should be {expected_value:?} : got = {result:?}", + ); + } + + #[rstest] + #[case::i64_overflows_to_float(i64::MIN, -1_i64, 9.223372036854776e18)] + #[case::i128_overflows_to_float(i128::MIN, -1_i128, 1.7014118346046923e38)] + fn div_overflow_promotes_to_float( + #[case] left: impl Into, + #[case] right: impl Into, + #[case] expected_value: impl Into, + ) { + let left = left.into(); + let right = right.into(); + let result = left / right; + + // Div always produces a float + let expected_order = Order::Float; + + assert_eq!( + result.order(), + expected_order, + "div overflow should promote this to {expected_order:?} : got = {result:?}" + ); + let expected_value = expected_value.into(); + assert_eq!( + result, expected_value, + "sub overflow value should be {expected_value:?} : got = {result:?}", + ); + } + + #[rstest] + #[case::i64_overflows_to_i128(i64::MIN, -1_i64, Order::SignedBigInt, 0_u128)] + #[case::i128_overflows_to_float(i128::MIN, -1_i128, Order::Float, -0.0)] + fn rem_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; + + assert_eq!( + result.order(), + expected_order, + "rem overflow should promote this to {expected_order:?} : got = {result:?}" + ); + let expected_value = expected_value.into(); + assert_eq!( + result, expected_value, + "rem overflow value should be {expected_value:?} : got = {result:?}", + ); + } + // ---------- INFINITY PROPAGATION ---------- #[test] fn inf_plus_finite_is_inf() { From a938f6686448960ba2b38714f906da1c3477871b Mon Sep 17 00:00:00 2001 From: Matt Oestreich Date: Fri, 6 Mar 2026 20:22:30 -0600 Subject: [PATCH 2/3] add Mul to CheckedMaths --- src/value/arithmetic.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/value/arithmetic.rs b/src/value/arithmetic.rs index 55dd561..7ddb0bc 100644 --- a/src/value/arithmetic.rs +++ b/src/value/arithmetic.rs @@ -9,6 +9,7 @@ trait CheckedMaths: + ops::Add + ops::Sub + ops::Div + + ops::Mul + ops::Rem { fn checked_add(self, rhs: Self) -> Option; From 1bf0ef45b3d25a91a94e209d2d7f17dd8ba9f874 Mon Sep 17 00:00:00 2001 From: Matt Oestreich Date: Sat, 7 Mar 2026 10:04:35 -0600 Subject: [PATCH 3/3] fix assert comment --- src/value/arithmetic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/value/arithmetic.rs b/src/value/arithmetic.rs index 7ddb0bc..220243f 100644 --- a/src/value/arithmetic.rs +++ b/src/value/arithmetic.rs @@ -503,7 +503,7 @@ mod tests { let expected_value = expected_value.into(); assert_eq!( result, expected_value, - "sub overflow value should be {expected_value:?} : got = {result:?}", + "div overflow value should be {expected_value:?} : got = {result:?}", ); }