From 9d4b1d806b844848faf79157855444e01aebdc08 Mon Sep 17 00:00:00 2001 From: Kevin Reid Date: Fri, 18 Oct 2024 10:22:32 -0700 Subject: [PATCH] math: Add `PositiveSign` type. This will be used to enforce stronger restrictions on color components, which will reduce the number of edge cases color processing code must consider, and remove the the hazard of distinct negative zero (). --- all-is-cubes-base/src/math.rs | 2 + .../src/math/restricted_number.rs | 406 ++++++++++++++++++ all-is-cubes-base/src/math/serde_impls.rs | 23 +- 3 files changed, 430 insertions(+), 1 deletion(-) create mode 100644 all-is-cubes-base/src/math/restricted_number.rs diff --git a/all-is-cubes-base/src/math.rs b/all-is-cubes-base/src/math.rs index 88c0c7b8c..6d7b1cff2 100644 --- a/all-is-cubes-base/src/math.rs +++ b/all-is-cubes-base/src/math.rs @@ -30,6 +30,8 @@ mod grid_iter; pub use grid_iter::*; mod rigid; pub use rigid::*; +mod restricted_number; +pub use restricted_number::*; mod matrix; pub use matrix::*; mod octant; diff --git a/all-is-cubes-base/src/math/restricted_number.rs b/all-is-cubes-base/src/math/restricted_number.rs new file mode 100644 index 000000000..eb25bbe9b --- /dev/null +++ b/all-is-cubes-base/src/math/restricted_number.rs @@ -0,0 +1,406 @@ +#![allow( + clippy::eq_op, + reason = "necessary for const is_nan() tests; remove after Rust 1.83" +)] + +use core::fmt; +use core::hash; +use core::ops; + +use num_traits::ConstZero as _; +use ordered_float::FloatCore; +use ordered_float::NotNan; + +// ------------------------------------------------------------------------------------------------- + +/// A floating-point number which is not NaN and whose sign bit is positive. +/// +/// The allowed values consist of positive zero, positive infinity, +/// and every value in between those. This set of values means that this type: +/// +/// * Implements [`Eq`] straightforwardly; neither NaN nor signed zeroes +/// can cause problems with using it as a map key for caching, interning, etc. +/// * Is closed under multiplication and addition, so unlike [`NotNan`], cannot +/// cause a panic from uncautious arithmetic. +/// +/// The permitted values of a `PositiveSign` are a subset of those of a `NotNan`. +/// (This may not be guaranteed if `T` implements numeric traits in inconsistent ways, +/// but may be assumed for `f32` and `f64`.) +#[derive(Clone, Copy, PartialEq, PartialOrd)] +pub struct PositiveSign(T); + +// /// ``` +// /// use all_is_cubes_base::{positive_sign, math::PositiveSign}; +// /// +// /// const X: PositiveSign = positive_sign!(0.0); +// /// const Y: PositiveSign = positive_sign!(1.0); +// /// const Z: PositiveSign = positive_sign!(99.0); +// /// ``` +// /// +// /// ```compile_fail +// /// use all_is_cubes_base::{positive_sign, math::PositiveSign}; +// /// // Not a literal; will not compile +// /// use f32::NAN; +// /// const X: PositiveSign = positive_sign!(NAN); +// /// ``` +// /// +// /// ```compile_fail +// /// use all_is_cubes_base::{positive_sign, math::PositiveSign}; +// /// // Not positive; will not compile +// /// const X: PositiveSign = positive_sign!(-0.0); +// /// ``` +// #[macro_export] +// #[doc(hidden)] // kludge we don't want to publish, for now +// macro_rules! positive_sign { +// // Parsing shenanigan: Matching `tt` allows only a single token, but `-1.0` is two tokens, +// // so it is prohibited. +// // This is brittle, but we can get rid of it as soon as Rust 1.82 is released. +// ($value:tt) => { +// match $crate::notnan!($value) { +// value => unsafe { $crate::math::PositiveSign::new_nn_unchecked(value) }, +// } +// }; +// } + +// --- Inherent implementations -------------------------------------------------------------------- + +impl PositiveSign { + // public, but unsafe, so it can be called from a macro + #[doc(hidden)] + #[inline] + pub const unsafe fn new_unchecked(value: T) -> Self { + Self(value) + } + + pub(crate) const fn into_nn(self) -> NotNan { + // SAFETY: `PositiveSign`’s restrictions are a superset of `NotNan`’s. + unsafe { NotNan::new_unchecked(self.0) } + } + + #[cfg(test)] + #[track_caller] + pub(crate) fn consistency_check(self) + where + Self: fmt::Debug, + { + assert!(!self.0.is_nan() && self.0.is_sign_positive(), "{self:?}"); + } +} + +// --- Non-generic macro-generated implementations ------------------------------------------------- +// +// As it becomes possible, we should replace these with generic impls. + +macro_rules! non_generic_impls { + ($t:ty) => { + impl PositiveSign<$t> { + /// Wraps the given value in `PositiveSign`. + /// + /// * If `value` is positive (including positive infinity), returns wrapped `value`. + /// * If `value` is zero of either sign, returns wrapped positive zero. + /// This is lossy, but corresponds to the IEEE 754 idea that -0.0 == +0.0. + /// * If `value` is negative non-zero or NaN, panics. + #[track_caller] + #[inline] + pub const fn new_strict(value: $t) -> Self { + if value > 0. { + Self(value) + } else if value == 0. { + Self(0.) + } else { + positive_sign_not_positive_panic() + } + } + + /// Wraps the given value in `PositiveSign`. + /// + /// * If the value is NaN, panics. + /// * If the value has a negative sign, it is replaced with positive zero. + #[track_caller] + #[inline] + pub const fn new_clamped(value: $t) -> Self { + if value > 0. { + Self(value) + } else if value == value { + Self(0.) + } else { + positive_sign_nan_panic() + } + } + + /// Const equivalent of `TryFrom::try_from()`. + #[inline] + pub(crate) const fn try_new(value: $t) -> Result> { + if value > <$t>::ZERO { + Ok(Self(value)) + } else if value == <$t>::ZERO { + // must be zero, not NaN, but we don’t know the sign + Ok(Self::ZERO) + } else { + Err(NotPositiveSign(value)) + } + } + + /// Unwraps the value without modifying it. + // TODO: When #![feature(const_precise_live_drops)] becomes stable, we can make this generic. + #[inline] + pub fn into_inner(self) -> $t { + self.0 + } + + /// Subtract `other` from `self`; if the result would be negative, it is zero instead. + #[inline] + #[must_use] + pub fn saturating_sub(self, other: Self) -> Self { + // should never be able to panic + Self::new_clamped(self.0 - other.0) + } + } + + impl From> for NotNan<$t> { + #[inline] + fn from(value: PositiveSign<$t>) -> Self { + value.into_nn() + } + } + impl From> for $t { + #[inline] + fn from(value: PositiveSign<$t>) -> Self { + value.0 + } + } + + impl TryFrom<$t> for PositiveSign<$t> { + type Error = NotPositiveSign<$t>; + + /// Checks that `value` is non-negative and non-NaN. + /// + /// * If `value` is positive (including positive infinity), returns wrapped `value`. + /// * If `value` is zero of either sign, returns wrapped positive zero. + /// This is lossy, but corresponds to the IEEE 754 idea that -0.0 == +0.0. + /// * If `value` is negative non-zero or NaN, returns an error. + #[inline] + fn try_from(value: $t) -> Result { + Self::try_new(value) + } + } + }; +} + +// When f16 and f128 are stable, implement for those too. +non_generic_impls!(f32); +non_generic_impls!(f64); + +// --- Generic trait implementations --------------------------------------------------------------- + +impl fmt::Debug for PositiveSign { + #[inline(never)] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // Don't print the wrappers, just the value. + let value: &T = &self.0; + value.fmt(f) + } +} +impl fmt::Display for PositiveSign { + #[inline(never)] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // Don't print the wrappers, just the value. + let value: &T = &self.0; + value.fmt(f) + } +} + +// The derived PartialEq implementation is okay, but we need to add Eq. +impl Eq for PositiveSign {} + +#[allow(clippy::derive_ord_xor_partial_ord)] +impl Ord for PositiveSign { + #[inline] + fn cmp(&self, other: &Self) -> core::cmp::Ordering { + // Correctness: All values that would violate `Ord`’s properties are prohibited. + self.partial_cmp(other).unwrap() + } +} + +impl hash::Hash for PositiveSign { + #[inline] + fn hash(&self, state: &mut H) { + self.into_nn().hash(state) + } +} + +impl Default for PositiveSign { + /// The default is zero, regardless of what `T::default()` is. + #[inline] + fn default() -> Self { + Self(T::zero()) + } +} + +impl num_traits::Zero for PositiveSign { + #[inline] + fn zero() -> Self { + Self(T::zero()) + } + + #[inline] + fn is_zero(&self) -> bool { + self.0.is_zero() + } +} +impl num_traits::One for PositiveSign { + #[inline] + fn one() -> Self { + Self(T::one()) + } +} +impl num_traits::ConstZero for PositiveSign { + const ZERO: Self = Self(T::ZERO); +} +impl num_traits::ConstOne for PositiveSign { + const ONE: Self = Self(T::ONE); +} + +impl> ops::Add for PositiveSign { + type Output = Self; + #[inline] + fn add(self, rhs: Self) -> Self::Output { + // Construction safety: + // If the nonnegative subset of T isn't closed under addition, the number type is + // too weird to be useful, and probably doesn’t honestly implement `FloatCore` either. + Self(self.0 + rhs.0) + } +} +impl> ops::Mul for PositiveSign { + type Output = Self; + /// Componentwise multiplication. + #[inline] + fn mul(self, rhs: Self) -> Self::Output { + // Construction safety: + // If the nonnegative subset of T isn't closed under multiplication, the number type is + // too weird to be useful, and probably doesn’t honestly implement `FloatCore` either. + Self(self.0 * rhs.0) + } +} + +impl AsRef for PositiveSign { + #[inline] + fn as_ref(&self) -> &T { + &self.0 + } +} + +impl TryFrom> for PositiveSign +where + Self: TryFrom>, +{ + type Error = NotPositiveSign; + + /// Checks that `value` is non-negative. + /// + /// * If `value` is positive (including positive infinity), returns wrapped `value`. + /// * If `value` is zero of either sign, returns wrapped positive zero. + /// This is lossy, but corresponds to the IEEE 754 idea that -0.0 == +0.0. + /// * If `value` is negative non-zero or NaN, returns an error. + #[inline] + fn try_from(value: NotNan) -> Result { + Self::try_from(value.into_inner()) + } +} + +#[cfg(feature = "arbitrary")] +#[mutants::skip] +#[allow(clippy::missing_inline_in_public_items)] +impl<'a, T> arbitrary::Arbitrary<'a> for PositiveSign +where + NotNan: arbitrary::Arbitrary<'a> + ops::Neg> + Copy, + Self: TryFrom>, +{ + #[inline(never)] + fn arbitrary(u: &mut arbitrary::Unstructured<'a>) -> arbitrary::Result { + let nn = NotNan::::arbitrary(u)?; + Self::try_from(nn) + .or_else(|_| Self::try_from(-nn)) + .map_err(|_| unreachable!("all floats are positive or negative")) + } + + #[inline(never)] + fn size_hint(depth: usize) -> (usize, Option) { + NotNan::::size_hint(depth) + } +} + +// --- Errors -------------------------------------------------------------------------------------- + +/// Error from attempting to construct a [`PositiveSign`]. +#[derive(Clone, Debug)] +pub struct NotPositiveSign(T); + +impl fmt::Display for NotPositiveSign { + #[inline(never)] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let value = self.0; + if value.is_nan() { + write!(f, "value was NaN") + } else { + write!(f, "{value} did not have a positive sign bit") + } + } +} + +impl core::error::Error for NotPositiveSign {} + +#[track_caller] +#[cold] +const fn positive_sign_nan_panic() -> ! { + panic!("PositiveSign value must not be NaN") +} + +#[track_caller] +#[cold] +const fn positive_sign_not_positive_panic() -> ! { + panic!("PositiveSign value must not be NaN or negative") +} + +// ------------------------------------------------------------------------------------------------- + +/// Convenient alias for [`PositiveSign::::new_strict()`], +/// to be used in tests and pseudo-literals. +#[inline] +pub const fn ps32(value: f32) -> PositiveSign { + PositiveSign::::new_clamped(value) +} + +// ------------------------------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + use alloc::string::ToString as _; + use exhaust::Exhaust as _; + + #[test] + fn canonicalizes_negative_zero() { + let was_nz = PositiveSign::::new_strict(-0.0); + assert!(was_nz.into_inner().is_sign_positive()); + assert_eq!(was_nz.to_string(), "0"); + assert_eq!(was_nz, PositiveSign::::try_from(-0.0).unwrap()) + } + + #[test] + fn exhaustive() { + for f in f32::exhaust() { + match PositiveSign::::try_from(f) { + Ok(ps) => { + ps.consistency_check(); + assert_eq!(ps, PositiveSign::::new_clamped(f)); + assert_eq!(ps, PositiveSign::::new_strict(f)); + assert_eq!(ps.into_inner(), f); + assert_eq!(f32::from(ps), f); + assert_eq!(NotNan::from(ps), NotNan::new(f).unwrap()); + } + Err(_) => assert!(f.is_nan() || f < 0.0), + } + } + } +} diff --git a/all-is-cubes-base/src/math/serde_impls.rs b/all-is-cubes-base/src/math/serde_impls.rs index 4ec6f9f51..82b2f833d 100644 --- a/all-is-cubes-base/src/math/serde_impls.rs +++ b/all-is-cubes-base/src/math/serde_impls.rs @@ -2,7 +2,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use crate::math::{Aab, Cube, GridAab, GridCoordinate}; +use crate::math::{self, Aab, Cube, GridAab, GridCoordinate}; #[derive(Debug, Deserialize, Serialize)] struct AabSer { @@ -85,3 +85,24 @@ impl<'de> Deserialize<'de> for GridAab { GridAab::checked_from_lower_upper(lower, upper).map_err(serde::de::Error::custom) } } + +impl Serialize for math::PositiveSign { + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + self.as_ref().serialize(serializer) + } +} + +impl<'de, T: Deserialize<'de>> Deserialize<'de> for math::PositiveSign +where + Self: TryFrom, +{ + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + Self::try_from(T::deserialize(deserializer)?).map_err(serde::de::Error::custom) + } +}