fix checked_rem_int and fix+deprecate {wrapping,overflowing}_rem_int

https://gitlab.com/tspiteri/fixed/issues/13
This commit is contained in:
Trevor Spiteri 2020-02-12 17:32:34 +01:00
parent 677ee97f6f
commit 97128d2b9f
6 changed files with 101 additions and 160 deletions

View File

@ -70,6 +70,8 @@ The conversions supported cover the following cases.
### Version 0.5.3 news (unreleased)
* Bug fix: all remainder operations with a fixed-point LHS and an
integer RHS were giving an incorrect answer ([issue 13]).
* [`Rem`] and [`RemAssign`] were implemented for fixed-point
numbers.
* The following methods were added to all fixed-point types and to
@ -81,6 +83,7 @@ The conversions supported cover the following cases.
* [`wrapping_div_euclid`]
* [`overflowing_div_euclid`]
[issue 13]: https://gitlab.com/tspiteri/fixed/issues/13
[`RemAssign`]: https://doc.rust-lang.org/nightly/core/ops/trait.RemAssign.html
[`Rem`]: https://doc.rust-lang.org/nightly/core/ops/trait.Rem.html
[`checked_div_euclid`]: https://docs.rs/fixed/0.5.3/fixed/struct.FixedI32.html#method.checked_div_euclid

View File

@ -8,6 +8,9 @@ as-is, without any warranty. -->
Version 0.5.3 (unreleased)
==========================
* Bug fix: all remainder operations with a fixed-point LHS and an
integer RHS were giving an incorrect answer
(https://gitlab.com/tspiteri/fixed/issues/13).
* `Rem` and `RemAssign` were implemented for fixed-point numbers.
* The following methods were added to all fixed-point types and to
the `Fixed` trait:

View File

@ -265,6 +265,39 @@ assert_eq!(Fix::max_value().checked_div_euclid(Fix::from_num(0.25)), None);
}
}
comment! {
"Checked fixed-point remainder for division by an integer.
Returns the remainder, or [`None`] if the divisor is zero.
# Examples
```rust
use fixed::{types::extra::U4, ", $s_fixed, "};
type Fix = ", $s_fixed, "<U4>;
assert_eq!(Fix::from_num(3.75).checked_rem_int(2), Some(Fix::from_num(1.75)));
assert_eq!(Fix::from_num(3.75).checked_rem_int(0), None);
",
if_signed_else_empty_str! {
$Signedness,
"assert_eq!(Fix::from_num(-3.75).checked_rem_int(2), Some(Fix::from_num(-1.75)));
",
},
"```
[`None`]: https://doc.rust-lang.org/nightly/core/option/enum.Option.html#variant.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)
}
}
comment! {
"Saturating multiplication. Returns the product, saturating on overflow.
@ -537,6 +570,28 @@ assert_eq!(Fix::max_value().overflowing_div_euclid(Fix::from_num(0.25)), (wrappe
(q, overflow)
}
}
/// Remainder for division by an integer.
///
/// # Panics
///
/// Panics if the divisor is zero.
#[deprecated(since = "0.5.3", note = "cannot overflow, use `%` or `Rem::rem` instead")]
#[inline]
pub fn wrapping_rem_int(self, rhs: $Inner) -> $Fixed<Frac> {
self % rhs
}
/// Remainder for division by an integer.
///
/// # Panics
///
/// Panics if the divisor is zero.
#[deprecated(since = "0.5.3", note = "cannot overflow, use `%` or `Rem::rem` instead")]
#[inline]
pub fn overflowing_rem_int(self, rhs: $Inner) -> ($Fixed<Frac>, bool) {
(self % rhs, false)
}
}
};
}

View File

@ -734,40 +734,6 @@ assert_eq!(Fix::from_num(1).checked_div_int(0), None);
}
}
comment! {
"Checked fixed-point remainder for division by an integer.
Returns the remainder, or [`None`] if the divisor is zero",
if_signed_unsigned! {
$Signedness,
" or if the division results in overflow.",
".",
},
"
# Examples
```rust
use fixed::{types::extra::U4, ", $s_fixed, "};
type Fix = ", $s_fixed, "<U4>;
// binary 1.0101 / 8 = binary 0.0010 remainder 0.0101
assert_eq!(Fix::from_bits(0b10101).checked_rem_int(8), Some(Fix::from_bits(0b101)));
assert_eq!(Fix::from_num(1).checked_rem_int(0), None);
",
if_signed_else_empty_str! {
$Signedness,
"assert_eq!(Fix::min_value().checked_rem_int(-1), None);
",
},
"```
[`None`]: https://doc.rust-lang.org/nightly/core/option/enum.Option.html#variant.None
";
#[inline]
pub fn checked_rem_int(self, rhs: $Inner) -> Option<$Fixed<Frac>> {
self.to_bits().checked_rem(rhs).map(Self::from_bits)
}
}
comment! {
"Checked remainder for Euclidean division. Returns the
remainder, or [`None`] if the divisor is zero.
@ -1261,44 +1227,6 @@ assert_eq!(Fix::from_num(3).wrapping_div_int(2), one_point_5);
}
}
comment! {
"Wrapping fixed-point remainder for division by an integer. Returns the remainder",
if_signed_unsigned! {
$Signedness,
", wrapping on overflow.
Overflow can only occur when dividing the minimum value by 1.",
".
Can never overflow for unsigned values.",
},
"
# Panics
Panics if the divisor is zero.
# Examples
```rust
use fixed::{types::extra::U4, ", $s_fixed, "};
type Fix = ", $s_fixed, "<U4>;
// binary 1.0101 / 8 = binary 0.0010 remainder 0.0101
assert_eq!(Fix::from_bits(0b10101).wrapping_rem_int(8), Fix::from_bits(0b101));
",
if_signed_else_empty_str! {
$Signedness,
"assert_eq!(Fix::min_value().wrapping_rem_int(-1), 0);
",
},
"```
";
#[inline]
pub fn wrapping_rem_int(self, rhs: $Inner) -> $Fixed<Frac> {
Self::from_bits(self.to_bits().wrapping_rem(rhs))
}
}
comment! {
"Wrapping Euclidean division by an integer. Returns the quotient",
if_signed_unsigned! {
@ -1611,48 +1539,6 @@ assert_eq!(Fix::from_num(3).overflowing_div_int(2), (one_point_5, false));
}
}
comment! {
"Overflowing fixed-point remainder for division by an integer.
Returns a [tuple] of the remainder and ",
if_signed_unsigned! {
$Signedness,
"a [`bool`] indicating whether an overflow has
occurred. On overflow, the wrapped value is returned. Overflow can
only occur when dividing the minimum value by 1.",
"[`false`][`bool`], as the division can never overflow for unsigned values.",
},
"
# Panics
Panics if the divisor is zero.
# Examples
```rust
use fixed::{types::extra::U4, ", $s_fixed, "};
type Fix = ", $s_fixed, "<U4>;
// binary 1.0101 / 8 = binary 0.0010 remainder 0.0101
assert_eq!(Fix::from_bits(0b10101).overflowing_rem_int(8), (Fix::from_bits(0b101), false));
",
if_signed_else_empty_str! {
$Signedness,
"assert_eq!(Fix::min_value().overflowing_rem_int(-1), (Fix::from_num(0), true));
",
},
"```
[`bool`]: https://doc.rust-lang.org/nightly/std/primitive.bool.html
[tuple]: https://doc.rust-lang.org/nightly/std/primitive.tuple.html
";
#[inline]
pub fn overflowing_rem_int(self, rhs: $Inner) -> ($Fixed<Frac>, bool) {
let (ans, o) = self.to_bits().overflowing_rem(rhs);
(Self::from_bits(ans), o)
}
}
comment! {
"Overflowing Euclidean division by an integer.

View File

@ -776,16 +776,6 @@ where
/// Panics if the divisor is zero.
fn wrapping_div_int(self, rhs: Self::Bits) -> Self;
/// Wrapping fixed-point remainder for division by an integer.
/// Returns the remainder, wrapping on overflow.
///
/// Overflow can only occur when dividing the minimum value by 1.
///
/// # Panics
///
/// Panics if the divisor is zero.
fn wrapping_rem_int(self, rhs: Self::Bits) -> Self;
/// Wrapping Euclidean division by an integer. Returns the
/// quotient, wrapping on overflow.
///
@ -906,21 +896,6 @@ where
/// [tuple]: https://doc.rust-lang.org/nightly/std/primitive.tuple.html
fn overflowing_div_int(self, rhs: Self::Bits) -> (Self, bool);
/// Overflowing fixed-point remainder for division by an integer.
///
/// Returns a [tuple] of the remainder and a [`bool`], indicating
/// whether an overflow has occurred. On overflow, the wrapped
/// value is returned. Overflow can only occur when dividing the
/// minimum value by 1.
///
/// # Panics
///
/// Panics if the divisor is zero.
///
/// [`bool`]: https://doc.rust-lang.org/nightly/std/primitive.bool.html
/// [tuple]: https://doc.rust-lang.org/nightly/std/primitive.tuple.html
fn overflowing_rem_int(self, rhs: Self::Bits) -> (Self, bool);
/// Overflowing Euclidean division by an integer.
///
/// Returns a [tuple] of the quotient and a [`bool`], indicating
@ -969,6 +944,26 @@ where
/// [`bool`]: https://doc.rust-lang.org/nightly/std/primitive.bool.html
/// [tuple]: https://doc.rust-lang.org/nightly/std/primitive.tuple.html
fn overflowing_shr(self, rhs: u32) -> (Self, bool);
/// Remainder for division by an integer.
///
/// # Panics
///
/// Panics if the divisor is zero.
#[deprecated(since = "0.5.3", note = "cannot overflow, use `%` or `Rem::rem` instead")]
fn wrapping_rem_int(self, rhs: Self::Bits) -> Self {
self % rhs
}
/// Remainder for division by an integer.
///
/// # Panics
///
/// Panics if the divisor is zero.
#[deprecated(since = "0.5.3", note = "cannot overflow, use `%` or `Rem::rem` instead")]
fn overflowing_rem_int(self, rhs: Self::Bits) -> (Self, bool) {
(self % rhs, false)
}
}
/// This trait provides methods common to all signed fixed-point numbers.
@ -1614,7 +1609,6 @@ macro_rules! impl_fixed {
trait_delegate! { fn wrapping_div_euclid(self, rhs: Self) -> Self }
trait_delegate! { fn wrapping_mul_int(self, rhs: Self::Bits) -> Self }
trait_delegate! { fn wrapping_div_int(self, rhs: Self::Bits) -> Self }
trait_delegate! { fn wrapping_rem_int(self, rhs: Self::Bits) -> Self }
trait_delegate! { fn wrapping_div_euclid_int(self, rhs: Self::Bits) -> Self }
trait_delegate! { fn wrapping_rem_euclid_int(self, rhs: Self::Bits) -> Self }
trait_delegate! { fn wrapping_shl(self, rhs: u32) -> Self }
@ -1627,7 +1621,6 @@ macro_rules! impl_fixed {
trait_delegate! { fn overflowing_div_euclid(self, rhs: Self) -> (Self, bool) }
trait_delegate! { fn overflowing_mul_int(self, rhs: Self::Bits) -> (Self, bool) }
trait_delegate! { fn overflowing_div_int(self, rhs: Self::Bits) -> (Self, bool) }
trait_delegate! { fn overflowing_rem_int(self, rhs: Self::Bits) -> (Self, bool) }
trait_delegate! { fn overflowing_div_euclid_int(self, rhs: Self::Bits) -> (Self, bool) }
trait_delegate! { fn overflowing_rem_euclid_int(self, rhs: Self::Bits) -> (Self, bool) }
trait_delegate! { fn overflowing_shl(self, rhs: u32) -> (Self, bool) }

View File

@ -20,6 +20,7 @@ use crate::{
traits::{Fixed, FixedSigned, FixedUnsigned, FromFixed, ToFixed},
FixedI128, FixedI16, FixedI32, FixedI64, FixedI8, FixedU128, FixedU16, FixedU32, FixedU64,
FixedU8,
types::extra::{LeEqU128, LeEqU16, LeEqU32, LeEqU64, LeEqU8},
};
use core::{
fmt::{Display, Formatter, Result as FmtResult},
@ -1016,45 +1017,45 @@ impl<'a, F: 'a + Fixed> Product<&'a Wrapping<F>> for Wrapping<F> {
macro_rules! op_bits {
(
$Fixed:ident($Bits:ident)::$wrapping:ident,
$Fixed:ident($Bits:ident $(, $LeEqU:ident)*)::$wrapping:ident,
$Op:ident $op:ident,
$OpAssign:ident $op_assign:ident
) => {
impl<Frac> $Op<$Bits> for Wrapping<$Fixed<Frac>> {
impl<Frac $(: $LeEqU)*> $Op<$Bits> for Wrapping<$Fixed<Frac>> {
type Output = Wrapping<$Fixed<Frac>>;
#[inline]
fn $op(self, other: $Bits) -> Wrapping<$Fixed<Frac>> {
Wrapping((self.0).$wrapping(other))
}
}
impl<'a, Frac> $Op<$Bits> for &'a Wrapping<$Fixed<Frac>> {
impl<'a, Frac $(: $LeEqU)*> $Op<$Bits> for &'a Wrapping<$Fixed<Frac>> {
type Output = Wrapping<$Fixed<Frac>>;
#[inline]
fn $op(self, other: $Bits) -> Wrapping<$Fixed<Frac>> {
Wrapping((self.0).$wrapping(other))
}
}
impl<'a, Frac> $Op<&'a $Bits> for Wrapping<$Fixed<Frac>> {
impl<'a, Frac $(: $LeEqU)*> $Op<&'a $Bits> for Wrapping<$Fixed<Frac>> {
type Output = Wrapping<$Fixed<Frac>>;
#[inline]
fn $op(self, other: &$Bits) -> Wrapping<$Fixed<Frac>> {
Wrapping((self.0).$wrapping(*other))
}
}
impl<'a, 'b, Frac> $Op<&'a $Bits> for &'b Wrapping<$Fixed<Frac>> {
impl<'a, 'b, Frac $(: $LeEqU)*> $Op<&'a $Bits> for &'b Wrapping<$Fixed<Frac>> {
type Output = Wrapping<$Fixed<Frac>>;
#[inline]
fn $op(self, other: &$Bits) -> Wrapping<$Fixed<Frac>> {
Wrapping((self.0).$wrapping(*other))
}
}
impl<Frac> $OpAssign<$Bits> for Wrapping<$Fixed<Frac>> {
impl<Frac $(: $LeEqU)*> $OpAssign<$Bits> for Wrapping<$Fixed<Frac>> {
#[inline]
fn $op_assign(&mut self, other: $Bits) {
self.0 = (self.0).$wrapping(other);
}
}
impl<'a, Frac> $OpAssign<&'a $Bits> for Wrapping<$Fixed<Frac>> {
impl<'a, Frac $(: $LeEqU)*> $OpAssign<&'a $Bits> for Wrapping<$Fixed<Frac>> {
#[inline]
fn $op_assign(&mut self, other: &$Bits) {
self.0 = (self.0).$wrapping(*other);
@ -1064,19 +1065,19 @@ macro_rules! op_bits {
}
macro_rules! ops {
($Fixed:ident($Bits:ident)) => {
($Fixed:ident($Bits:ident, $LeEqU:ident)) => {
op_bits! { $Fixed($Bits)::wrapping_mul_int, Mul mul, MulAssign mul_assign }
op_bits! { $Fixed($Bits)::wrapping_div_int, Div div, DivAssign div_assign }
op_bits! { $Fixed($Bits)::wrapping_rem_int, Rem rem, RemAssign rem_assign }
op_bits! { $Fixed($Bits, $LeEqU)::rem, Rem rem, RemAssign rem_assign }
};
}
ops! { FixedI8(i8) }
ops! { FixedI16(i16) }
ops! { FixedI32(i32) }
ops! { FixedI64(i64) }
ops! { FixedI128(i128) }
ops! { FixedU8(u8) }
ops! { FixedU16(u16) }
ops! { FixedU32(u32) }
ops! { FixedU64(u64) }
ops! { FixedU128(u128) }
ops! { FixedI8(i8, LeEqU8) }
ops! { FixedI16(i16, LeEqU16) }
ops! { FixedI32(i32, LeEqU32) }
ops! { FixedI64(i64, LeEqU64) }
ops! { FixedI128(i128, LeEqU128) }
ops! { FixedU8(u8, LeEqU8) }
ops! { FixedU16(u16, LeEqU16) }
ops! { FixedU32(u32, LeEqU32) }
ops! { FixedU64(u64, LeEqU64) }
ops! { FixedU128(u128, LeEqU128) }