some fixes and tests

* rem_euclid no longer uses rhs.abs() which could overflow.

  * Rem<Inner> and rem_euclid_int now allow for the signed case where
    rhs is equal to the absolute value of the minimum fixed value.

  * checked_rem_int, checked_rem_euclid and checked_rem_euclid_int now
    check for zero then defer to Rem<Inner>, rem_euclid and
    rem_euclid_int.

src/arith.rs

@@ -370,13 +370,21 @@ macro_rules! fixed_arith {
             type Output = $Fixed<Frac>;
             #[inline]
             fn rem(self, rhs: $Inner) -> $Fixed<Frac> {
-                // Any overflow in coverting rhs to $Fixed<Frac> 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<Frac> 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);
+    }
 }

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<Frac> {
-                    // Any overflow in coverting rhs to $Fixed<Frac> 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<Frac> 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<Frac>> {
-                    // Any overflow in coverting rhs to $Fixed<Frac> 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<Frac>> {
-                    // Any overflow in coverting rhs to $Fixed<Frac> 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))
+                    }
                 }
             }
 

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<Frac>) -> Option<$Fixed<Frac>> {
-                    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)
                 }
             }