From 4b889cbb1fa8c0c2a10b1e50a5fd2b98e6af02d8 Mon Sep 17 00:00:00 2001 From: Trevor Spiteri Date: Thu, 13 Feb 2020 12:03:35 +0100 Subject: [PATCH] reorganize {,checked} {,Euclidean} remainders {,by integers} Now the main logic is in the checked functions, which fail only if rhs is zero, and the other functions all .expect("division by zero"). checked_rem now uses % instead of core checked_rem. checked_rem_euclid now uses core rem_euclid. checked_rem_int uses checked_rem for the most common case. checked_rem_euclid_int uses checked_rem_euclid for the most common case. --- src/arith.rs | 16 +-------------- src/macros_frac.rs | 48 ++++++++++++++++++++++++------------------- src/macros_no_frac.rs | 42 ++++++++++++++++++++----------------- 3 files changed, 51 insertions(+), 55 deletions(-) diff --git a/src/arith.rs b/src/arith.rs index 540c422..5971f23 100644 --- a/src/arith.rs +++ b/src/arith.rs @@ -370,21 +370,7 @@ macro_rules! fixed_arith { type Output = $Fixed; #[inline] fn rem(self, rhs: $Inner) -> $Fixed { - // 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 - }, - } + self.checked_rem_int(rhs).expect("division by zero") } } diff --git a/src/macros_frac.rs b/src/macros_frac.rs index 93378cc..ab16d20 100644 --- a/src/macros_frac.rs +++ b/src/macros_frac.rs @@ -257,19 +257,7 @@ 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 { - // 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, - } + self.checked_rem_euclid_int(rhs).expect("division by zero") } } @@ -385,10 +373,20 @@ assert_eq!(Fix::from_num(3.75).checked_rem_int(0), None); "; #[inline] pub fn checked_rem_int(self, rhs: $Inner) -> Option<$Fixed> { - if rhs == 0 { - None - } else { - Some(self % 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.checked_rem(fixed_rhs), + None => Some(if_signed_unsigned!( + $Signedness, + if self.to_num::<$Inner>().wrapping_abs() == rhs { + Self::from_bits(0) + } else { + self + }, + self, + )), } } } @@ -460,10 +458,18 @@ 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> { - if rhs == 0 { - None - } else { - Some(self.rem_euclid_int(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.checked_rem_euclid(fixed_rhs), + None => Some(self), } } } diff --git a/src/macros_no_frac.rs b/src/macros_no_frac.rs index d46aaff..faa86fa 100644 --- a/src/macros_no_frac.rs +++ b/src/macros_no_frac.rs @@ -442,18 +442,7 @@ assert_eq!(Fix::from_num(7.5).rem_euclid(Fix::from_num(2)), Fix::from_num(1.5)); "; #[inline] pub fn rem_euclid(self, rhs: $Fixed) -> $Fixed { - let r = self % rhs; - if_signed! { - $Signedness; - if r.is_negative() { - return if rhs.to_bits() < 0 { - r - rhs - } else { - r + rhs - }; - } - } - r + self.checked_rem_euclid(rhs).expect("division by zero") } } @@ -609,13 +598,19 @@ assert_eq!(Fix::from_num(1.5).checked_rem(Fix::from_num(0)), None); "; #[inline] pub fn checked_rem(self, rhs: $Fixed) -> Option<$Fixed> { - let rhs = rhs.to_bits(); - if rhs == 0 { + let rhs_bits = rhs.to_bits(); + if rhs_bits == 0 { None - } else if if_signed_unsigned!($Signedness, rhs == -1, false) { - Some(Self::from_bits(0)) } else { - self.to_bits().checked_rem(rhs).map(Self::from_bits) + Some(Self::from_bits(if_signed_unsigned!( + $Signedness, + if rhs_bits == -1 { + 0 + } else { + self.to_bits() % rhs_bits + }, + self.to_bits() % rhs_bits, + ))) } } } @@ -698,10 +693,19 @@ assert_eq!(num.checked_rem_euclid(Fix::from_num(0)), None); "; #[inline] pub fn checked_rem_euclid(self, rhs: $Fixed) -> Option<$Fixed> { - if rhs.to_bits() == 0 { + let rhs_bits = rhs.to_bits(); + if rhs_bits == 0 { None } else { - Some(self.rem_euclid(rhs)) + Some(Self::from_bits(if_signed_unsigned!( + $Signedness, + if rhs_bits == -1 { + 0 + } else { + self.to_bits().rem_euclid(rhs_bits) + }, + self.to_bits() % rhs_bits, + ))) } } }