Skip to content

Commit

Permalink
Implement total ord for Key (#586)
Browse files Browse the repository at this point in the history
Fixes potential crashes on Rust 1.81.
  • Loading branch information
mitsuhiko authored Sep 6, 2024
1 parent a4231cc commit 8893db7
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 5 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
54 changes: 49 additions & 5 deletions insta/src/content/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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<Ordering> {
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() {
Expand Down

0 comments on commit 8893db7

Please sign in to comment.