From 98bffc0a0c7e0b10ec88c81b14bcd6778b2aad5f Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 6 Sep 2024 11:01:49 +0200 Subject: [PATCH 1/2] Implement total ord for Key --- insta/src/content/serialization.rs | 54 +++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/insta/src/content/serialization.rs b/insta/src/content/serialization.rs index ae4e4536..b0eeea6d 100644 --- a/insta/src/content/serialization.rs +++ b/insta/src/content/serialization.rs @@ -5,7 +5,7 @@ use crate::content::Content; use serde::{ser, Serialize, Serializer}; -#[derive(PartialEq, PartialOrd, Debug)] +#[derive(PartialEq, Debug)] pub enum Key<'a> { Bool(bool), U64(u64), @@ -18,17 +18,61 @@ pub enum Key<'a> { Other, } +impl<'a> Key<'a> { + /// Needed because std::mem::discriminant is not Ord + fn discriminant(&self) -> usize { + match self { + Key::Bool(_) => 1, + Key::U64(_) => 2, + Key::I64(_) => 3, + Key::F64(_) => 4, + Key::U128(_) => 5, + Key::I128(_) => 6, + Key::Str(_) => 7, + Key::Bytes(_) => 8, + Key::Other => 9, + } + } +} + impl<'a> Eq for Key<'a> {} -// We're making a deliberate choice to just "round down" here, so ignoring the -// clippy lint -#[allow(clippy::derive_ord_xor_partial_ord)] impl<'a> Ord for Key<'a> { fn cmp(&self, other: &Self) -> Ordering { - self.partial_cmp(other).unwrap_or(Ordering::Less) + let self_discriminant = self.discriminant(); + let other_discriminant = other.discriminant(); + match Ord::cmp(&self_discriminant, &other_discriminant) { + Ordering::Equal => match (self, other) { + (Key::Bool(a), Key::Bool(b)) => Ord::cmp(a, b), + (Key::U64(a), Key::U64(b)) => Ord::cmp(a, b), + (Key::I64(a), Key::I64(b)) => Ord::cmp(a, b), + (Key::F64(a), Key::F64(b)) => f64_total_cmp(*a, *b), + (Key::U128(a), Key::U128(b)) => Ord::cmp(a, b), + (Key::I128(a), Key::I128(b)) => Ord::cmp(a, b), + (Key::Str(a), Key::Str(b)) => Ord::cmp(a, b), + (Key::Bytes(a), Key::Bytes(b)) => Ord::cmp(a, b), + _ => Ordering::Equal, + }, + cmp => cmp, + } } } +impl<'a> PartialOrd for Key<'a> { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } +} + +fn f64_total_cmp(left: f64, right: f64) -> Ordering { + // this is taken from f64::total_cmp on newer rust versions + let mut left = left.to_bits() as i64; + let mut right = right.to_bits() as i64; + left ^= (((left >> 63) as u64) >> 1) as i64; + right ^= (((right >> 63) as u64) >> 1) as i64; + left.cmp(&right) +} + impl Content { pub(crate) fn as_key(&self) -> Key<'_> { match *self.resolve_inner() { From e1f4a970b62dcdbdd20cc46a2a7a64440c65da7a Mon Sep 17 00:00:00 2001 From: Armin Ronacher Date: Fri, 6 Sep 2024 11:02:42 +0200 Subject: [PATCH 2/2] Added changelog entry --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index abc9f8a3..9fe9a664 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,9 @@ All notable changes to insta and cargo-insta are documented here. - Enable Filters to be created from `IntoIterator` types, rather than just `Vec`s. #570 +- Implemented total sort order for an internal `Key` type correctly. This prevents potential + crashes introduced by the new sort algorithm in Rust 1.81. #586 + ## 1.39.0 - Fixed a bug in `require_full_match`. #485