Skip to content

Commit

Permalink
Improve Handling of NotNaN Types (#851)
Browse files Browse the repository at this point in the history
In Rust 1.81 it has become essential to 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.
  • Loading branch information
CryZe authored Nov 4, 2024
1 parent d848fb7 commit a1a130b
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 26 deletions.
47 changes: 25 additions & 22 deletions src/run/editor/fuzzy_list.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -50,7 +53,7 @@ impl FuzzyList {
if heap.len() >= max {
heap.pop();
}
heap.push((NotNaN(-score), element));
heap.push((-score, element));
}
}

Expand All @@ -60,7 +63,7 @@ impl FuzzyList {
if heap.len() >= max {
heap.pop();
}
heap.push((NotNaN(-score), element));
heap.push((-score, element));
}
}
} else {
Expand All @@ -69,7 +72,7 @@ impl FuzzyList {
if heap.len() >= max {
heap.pop();
}
heap.push((NotNaN(-score), element));
heap.push((-score, element));
}
}
}
Expand All @@ -78,53 +81,53 @@ impl FuzzyList {
}
}

fn match_against(pattern: &str, text: &str) -> Option<f64> {
let (mut current_score, mut total_score) = (0.0, 0.0);
fn match_against(pattern: &str, text: &str) -> Option<NotNaN> {
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<f64> {
let (mut current_score, mut total_score) = (0.0, 0.0);
fn match_against_ascii(pattern: &str, text: &str) -> Option<NotNaN> {
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
}
Expand Down
93 changes: 89 additions & 4 deletions src/util/not_nan.rs
Original file line number Diff line number Diff line change
@@ -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<PositiveNotNaNNotZero> for PositiveNotNaN {
fn from(value: PositiveNotNaNNotZero) -> Self {
Self(value.0)
}
}

impl From<PositiveNotNaN> 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<PositiveNotNaNNotZero> 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
}
}

Expand All @@ -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() }
}
}

0 comments on commit a1a130b

Please sign in to comment.