From 57a816c9c4a712de510196841d12bb832fd9780e Mon Sep 17 00:00:00 2001 From: Christopher Serr Date: Mon, 4 Nov 2024 20:07:39 +0100 Subject: [PATCH] Improve Handling of `NotNaN` Types In Rust 1.81 it has become essential that handle `NaN` properly when sorting. An incorrect implementation of `Ord` now leads to panics when sorting. In order to prevent this, we now try to prove that the values are never `NaN` to begin with. --- src/run/editor/fuzzy_list.rs | 47 +++++++++--------- src/util/not_nan.rs | 93 ++++++++++++++++++++++++++++++++++-- 2 files changed, 114 insertions(+), 26 deletions(-) diff --git a/src/run/editor/fuzzy_list.rs b/src/run/editor/fuzzy_list.rs index 5647a376..1c9f2e3a 100644 --- a/src/run/editor/fuzzy_list.rs +++ b/src/run/editor/fuzzy_list.rs @@ -1,4 +1,7 @@ -use crate::{platform::prelude::*, util::not_nan::NotNaN}; +use crate::{ + platform::prelude::*, + util::not_nan::{NotNaN, PositiveNotNaN, PositiveNotNaNNotZero}, +}; use alloc::collections::BinaryHeap; /// With a Fuzzy List, you can implement a fuzzy searching algorithm. The list @@ -50,7 +53,7 @@ impl FuzzyList { if heap.len() >= max { heap.pop(); } - heap.push((NotNaN(-score), element)); + heap.push((-score, element)); } } @@ -60,7 +63,7 @@ impl FuzzyList { if heap.len() >= max { heap.pop(); } - heap.push((NotNaN(-score), element)); + heap.push((-score, element)); } } } else { @@ -69,7 +72,7 @@ impl FuzzyList { if heap.len() >= max { heap.pop(); } - heap.push((NotNaN(-score), element)); + heap.push((-score, element)); } } } @@ -78,53 +81,53 @@ impl FuzzyList { } } -fn match_against(pattern: &str, text: &str) -> Option { - let (mut current_score, mut total_score) = (0.0, 0.0); +fn match_against(pattern: &str, text: &str) -> Option { + let [mut current_score, mut total_score] = [PositiveNotNaN::ZERO; 2]; let mut pattern_chars = pattern.chars(); let mut pattern_char = pattern_chars.next(); for c in text.chars() { if pattern_char == Some(c) { pattern_char = pattern_chars.next(); - current_score = 1.0 + 2.0 * current_score; + current_score = current_score * PositiveNotNaNNotZero::TWO + PositiveNotNaN::ONE; } else { - current_score = 0.0; + current_score = PositiveNotNaN::ZERO; } - total_score += current_score; + total_score = total_score + current_score; } if pattern_char.is_none() { - if pattern == text { - Some(f64::INFINITY) + Some(if pattern == text { + NotNaN::INFINITY } else { - Some(total_score) - } + total_score.into() + }) } else { None } } -fn match_against_ascii(pattern: &str, text: &str) -> Option { - let (mut current_score, mut total_score) = (0.0, 0.0); +fn match_against_ascii(pattern: &str, text: &str) -> Option { + let [mut current_score, mut total_score] = [PositiveNotNaN::ZERO; 2]; let mut pattern_chars = pattern.bytes(); let mut pattern_char = pattern_chars.next(); for c in text.bytes() { if pattern_char == Some(c) { pattern_char = pattern_chars.next(); - current_score = 1.0 + 2.0 * current_score; + current_score = current_score * PositiveNotNaNNotZero::TWO + PositiveNotNaN::ONE; } else { - current_score = 0.0; + current_score = PositiveNotNaN::ZERO; } - total_score += current_score; + total_score = total_score + current_score; } if pattern_char.is_none() { - if pattern == text { - Some(f64::INFINITY) + Some(if pattern == text { + NotNaN::INFINITY } else { - Some(total_score) - } + total_score.into() + }) } else { None } diff --git a/src/util/not_nan.rs b/src/util/not_nan.rs index 445a6c3f..13501e50 100644 --- a/src/util/not_nan.rs +++ b/src/util/not_nan.rs @@ -1,11 +1,95 @@ -use core::cmp::Ordering; +use core::{ + cmp::Ordering, + ops::{Add, Mul, Neg}, +}; +/// Never `NaN`, but can be any other [`f64`]. +#[derive(Copy, Clone)] #[repr(transparent)] -pub struct NotNaN(pub f64); +pub struct NotNaN(f64); + +impl NotNaN { + pub const INFINITY: Self = Self(f64::INFINITY); +} + +/// Never `NaN` and always has a positive sign. May be positive infinity. +#[derive(Copy, Clone)] +#[repr(transparent)] +pub struct PositiveNotNaN(f64); + +/// Never `NaN` and is always larger than (and not equal to) 0. May be positive +/// infinity. +#[derive(Copy, Clone)] +#[repr(transparent)] +pub struct PositiveNotNaNNotZero(f64); + +impl From for PositiveNotNaN { + fn from(value: PositiveNotNaNNotZero) -> Self { + Self(value.0) + } +} + +impl From for NotNaN { + fn from(value: PositiveNotNaN) -> Self { + Self(value.0) + } +} + +impl PositiveNotNaN { + pub const ZERO: Self = Self(0.0); + pub const ONE: Self = Self(1.0); +} + +impl PositiveNotNaNNotZero { + pub const TWO: Self = Self(2.0); +} + +// The following cases result in NaN: +// - `NaN * x`: We ensure neither input is NaN. +// - `0 * Infinity`: We handle this by ensuring at least one side is not 0. +// +// IEEE Std 754-2008 7.2: +// > a) any general-computational or signaling-computational operation on a +// > signaling NaN (see 6.2), except for some conversions (see 5.12) +// > b) multiplication: multiplication(0, ∞) or multiplication(∞, 0) +impl Mul for PositiveNotNaN { + type Output = Self; + + fn mul(self, rhs: PositiveNotNaNNotZero) -> Self { + Self(self.0 * rhs.0) + } +} + +// The following cases result in NaN: +// - `NaN + x`: We ensure neither input is NaN. +// - `Infinity + -Infinity`: We handle this by ensuring the inputs are +// positive. +// +// IEEE Std 754-2008 7.2: +// > a) any general-computational or signaling-computational operation on a +// > signaling NaN (see 6.2), except for some conversions (see 5.12) +// > b) addition or subtraction or fusedMultiplyAdd: magnitude subtraction of infinities, such as: +// > addition(+∞, −∞) +impl Add for PositiveNotNaN { + type Output = Self; + + fn add(self, rhs: Self) -> Self { + Self(self.0 + rhs.0) + } +} + +// Negating a non-NaN value results in a non-NaN value. +impl Neg for NotNaN { + type Output = Self; + + fn neg(self) -> Self { + Self(-self.0) + } +} impl PartialEq for NotNaN { fn eq(&self, other: &Self) -> bool { - self.cmp(other) == Ordering::Equal + self.0 == other.0 } } @@ -19,6 +103,7 @@ impl PartialOrd for NotNaN { impl Ord for NotNaN { fn cmp(&self, other: &Self) -> Ordering { - self.0.partial_cmp(&other.0).unwrap_or(Ordering::Equal) + // SAFETY: The value is guaranteed to not be NaN. See above. + unsafe { self.0.partial_cmp(&other.0).unwrap_unchecked() } } }