diff --git a/src/arith.rs b/src/arith.rs index 62bd82a..540c422 100644 --- a/src/arith.rs +++ b/src/arith.rs @@ -370,13 +370,21 @@ macro_rules! fixed_arith { type Output = $Fixed; #[inline] fn rem(self, rhs: $Inner) -> $Fixed { - // Any overflow in coverting rhs to $Fixed means that |rhs| > |self|, - // and consequently the remainder is self. - let fixed_rhs = match Self::checked_from_num(rhs) { - Some(s) => s, - None => return self, - }; - self.rem(fixed_rhs) + // Overflow converting rhs to $Fixed means that either + // * |rhs| > |self|, and so remainder is self, or + // * self is signed min, and the value of rhs is -self, so remainder is 0. + match Self::checked_from_num(rhs) { + Some(fixed_rhs) => self.rem(fixed_rhs), + None => if_signed_unsigned! { + $Signedness, + if self.to_num::<$Inner>().wrapping_abs() == rhs { + Self::from_bits(0) + } else { + self + }, + self + }, + } } } @@ -789,4 +797,45 @@ mod tests { assert_eq!((af % b).to_bits(), (a << frac) % (b << frac)); } } + + fn check_rem_int(a: i32, b: i32) { + use crate::types::I16F16; + assert_eq!(I16F16::from_num(a) % b, a % b); + assert_eq!(I16F16::from_num(a).rem_euclid_int(b), a.rem_euclid(b)); + match (I16F16::from_num(a).checked_rem_int(b), a.checked_rem(b)) { + (Some(a), Some(b)) => assert_eq!(a, b), + (None, None) => {} + (a, b) => panic!("mismatch {:?}, {:?}", a, b), + } + match ( + I16F16::from_num(a).checked_rem_euclid_int(b), + a.checked_rem_euclid(b), + ) { + (Some(a), Some(b)) => assert_eq!(a, b), + (None, None) => {} + (a, b) => panic!("mismatch {:?}, {:?}", a, b), + } + } + + #[test] + fn rem_int() { + use crate::types::I16F16; + check_rem_int(-0x8000, -0x8000); + check_rem_int(-0x8000, -0x7fff); + check_rem_int(-0x8000, 0x7fff); + check_rem_int(-0x8000, 0x8000); + check_rem_int(-0x7fff, -0x8000); + check_rem_int(-0x7fff, -0x7fff); + check_rem_int(-0x7fff, 0x7fff); + check_rem_int(-0x7fff, 0x8000); + check_rem_int(0x7fff, -0x8000); + check_rem_int(0x7fff, -0x7fff); + check_rem_int(0x7fff, 0x7fff); + check_rem_int(0x7fff, 0x8000); + + assert_eq!(I16F16::min_value() % -1, 0); + assert_eq!(I16F16::min_value().checked_rem_int(-1).unwrap(), 0); + assert_eq!(I16F16::min_value().rem_euclid_int(-1), 0); + assert_eq!(I16F16::min_value().checked_rem_euclid_int(-1).unwrap(), 0); + } } diff --git a/src/macros_frac.rs b/src/macros_frac.rs index 08e33bc..93378cc 100644 --- a/src/macros_frac.rs +++ b/src/macros_frac.rs @@ -257,13 +257,19 @@ assert_eq!(Fix::from_num(7.5).rem_euclid_int(2), Fix::from_num(1.5)); "; #[inline] pub fn rem_euclid_int(self, rhs: $Inner) -> $Fixed { - // Any overflow in coverting rhs to $Fixed means that |rhs| > |self|, - // and consequently the remainder is self. - let fixed_rhs = match Self::checked_from_num(rhs) { - Some(s) => s, - None => return self, - }; - self.rem_euclid(fixed_rhs) + // For signed rem_euclid_int, rhs can be made + // negative without changing result. + // Then, overflow converting rhs to $Fixed means + // that |rhs| > |self|, and so remainder is self. + let prep_rhs = if_signed_unsigned!( + $Signedness, + rhs.wrapping_abs().wrapping_neg(), + rhs, + ); + match Self::checked_from_num(prep_rhs) { + Some(fixed_rhs) => self.rem_euclid(fixed_rhs), + None => self, + } } } @@ -379,13 +385,11 @@ assert_eq!(Fix::from_num(3.75).checked_rem_int(0), None); "; #[inline] pub fn checked_rem_int(self, rhs: $Inner) -> Option<$Fixed> { - // Any overflow in coverting rhs to $Fixed means that |rhs| > |self|, - // and consequently the remainder is self. - let fixed_rhs = match Self::checked_from_num(rhs) { - Some(s) => s, - None => return Some(self), - }; - self.checked_rem(fixed_rhs) + if rhs == 0 { + None + } else { + Some(self % rhs) + } } } @@ -456,13 +460,11 @@ assert_eq!(Fix::from_num(7.5).checked_rem_euclid_int(0), None); "; #[inline] pub fn checked_rem_euclid_int(self, rhs: $Inner) -> Option<$Fixed> { - // Any overflow in coverting rhs to $Fixed means that |rhs| > |self|, - // and consequently the remainder is self. - let fixed_rhs = match Self::checked_from_num(rhs) { - Some(s) => s, - None => return Some(self), - }; - self.checked_rem_euclid(fixed_rhs) + if rhs == 0 { + None + } else { + Some(self.rem_euclid_int(rhs)) + } } } diff --git a/src/macros_no_frac.rs b/src/macros_no_frac.rs index d60f727..d46aaff 100644 --- a/src/macros_no_frac.rs +++ b/src/macros_no_frac.rs @@ -446,7 +446,11 @@ assert_eq!(Fix::from_num(7.5).rem_euclid(Fix::from_num(2)), Fix::from_num(1.5)); if_signed! { $Signedness; if r.is_negative() { - return r + rhs.abs(); + return if rhs.to_bits() < 0 { + r - rhs + } else { + r + rhs + }; } } r @@ -694,14 +698,11 @@ assert_eq!(num.checked_rem_euclid(Fix::from_num(0)), None); "; #[inline] pub fn checked_rem_euclid(self, rhs: $Fixed) -> Option<$Fixed> { - let r = self.checked_rem(rhs)?; - if_signed! { - $Signedness; - if r.is_negative() { - return Some(r + rhs.abs()); - } + if rhs.to_bits() == 0 { + None + } else { + Some(self.rem_euclid(rhs)) } - Some(r) } }