From bc926d1b70d9f4b0df6eca4df867afd7b2263c2f Mon Sep 17 00:00:00 2001 From: Jeff Parsons Date: Tue, 6 Feb 2024 17:46:24 +1100 Subject: [PATCH] Fix Hash implementations They should hash the underlying range-value pairs, not the wrapper structs. --- src/inclusive_map.rs | 38 +++++++++++++++++++++++++++----------- src/inclusive_set.rs | 34 +--------------------------------- src/map.rs | 16 +++++++++++++++- src/range_wrapper.rs | 8 ++++---- 4 files changed, 47 insertions(+), 49 deletions(-) diff --git a/src/inclusive_map.rs b/src/inclusive_map.rs index 50e4399..0e60b1b 100644 --- a/src/inclusive_map.rs +++ b/src/inclusive_map.rs @@ -5,6 +5,7 @@ use alloc::collections::BTreeMap; use core::borrow::Borrow; use core::cmp::Ordering; use core::fmt::{self, Debug}; +use core::hash::Hash; use core::iter::{DoubleEndedIterator, FromIterator}; use core::marker::PhantomData; use core::ops::{RangeFrom, RangeInclusive}; @@ -33,7 +34,7 @@ use serde::{ /// you can provide equivalent free functions using the `StepFnsT` type parameter. /// [`StepLite`](crate::StepLite) is implemented for all standard integer types, /// but not for any third party crate types. -#[derive(Clone, Hash, Default)] +#[derive(Clone, Default)] pub struct RangeInclusiveMap { // Wrap ranges so that they are `Ord`. // See `range_wrapper.rs` for explanation. @@ -41,6 +42,19 @@ pub struct RangeInclusiveMap { _phantom: PhantomData, } +impl Hash for RangeInclusiveMap +where + K: Hash, + V: Hash, +{ + fn hash(&self, state: &mut H) { + state.write_usize(self.btm.len()); + for elt in self.iter() { + elt.hash(state); + } + } +} + impl PartialEq for RangeInclusiveMap where K: PartialEq, @@ -80,6 +94,18 @@ where } } +impl RangeInclusiveMap { + /// Gets an iterator over all pairs of key range and value, + /// ordered by key range. + /// + /// The iterator element type is `(&'a RangeInclusive, &'a V)`. + pub fn iter(&self) -> Iter<'_, K, V> { + Iter { + inner: self.btm.iter(), + } + } +} + impl RangeInclusiveMap where K: Ord + Clone + StepLite, @@ -163,16 +189,6 @@ where self.get(key).is_some() } - /// Gets an iterator over all pairs of key range and value, - /// ordered by key range. - /// - /// The iterator element type is `(&'a RangeInclusive, &'a V)`. - pub fn iter(&self) -> Iter<'_, K, V> { - Iter { - inner: self.btm.iter(), - } - } - /// Clears the map, removing all elements. pub fn clear(&mut self) { self.btm.clear(); diff --git a/src/inclusive_set.rs b/src/inclusive_set.rs index 004f1c4..7d3d90d 100644 --- a/src/inclusive_set.rs +++ b/src/inclusive_set.rs @@ -1,5 +1,4 @@ use core::borrow::Borrow; -use core::cmp::Ordering; use core::fmt::{self, Debug}; use core::iter::{DoubleEndedIterator, FromIterator}; use core::ops::RangeInclusive; @@ -15,7 +14,7 @@ use serde::{ use crate::std_ext::*; use crate::RangeInclusiveMap; -#[derive(Clone, Hash, Default)] +#[derive(Clone, Hash, Default, Eq, PartialEq, PartialOrd, Ord)] /// A set whose items are stored as ranges bounded /// inclusively below and above `(start..=end)`. /// @@ -26,37 +25,6 @@ pub struct RangeInclusiveSet { rm: RangeInclusiveMap, } -impl PartialEq for RangeInclusiveSet -where - T: PartialEq, -{ - fn eq(&self, other: &Self) -> bool { - self.rm == other.rm - } -} - -impl Eq for RangeInclusiveSet where T: Eq {} - -impl PartialOrd for RangeInclusiveSet -where - T: PartialOrd, -{ - #[inline] - fn partial_cmp(&self, other: &Self) -> Option { - self.rm.partial_cmp(&other.rm) - } -} - -impl Ord for RangeInclusiveSet -where - T: Ord, -{ - #[inline] - fn cmp(&self, other: &Self) -> Ordering { - self.rm.cmp(&other.rm) - } -} - impl RangeInclusiveSet where T: Ord + Clone + StepLite, diff --git a/src/map.rs b/src/map.rs index 6481088..8964992 100644 --- a/src/map.rs +++ b/src/map.rs @@ -5,6 +5,7 @@ use alloc::collections::BTreeMap; use core::borrow::Borrow; use core::cmp::Ordering; use core::fmt::{self, Debug}; +use core::hash::Hash; use core::iter::{DoubleEndedIterator, FromIterator}; use core::ops::{Bound, Range}; use core::prelude::v1::*; @@ -22,13 +23,26 @@ use serde::{ /// /// Contiguous and overlapping ranges that map to the same value /// are coalesced into a single range. -#[derive(Clone, Hash, Default, Eq)] +#[derive(Clone, Default, Eq)] pub struct RangeMap { // Wrap ranges so that they are `Ord`. // See `range_wrapper.rs` for explanation. pub(crate) btm: BTreeMap, V>, } +impl Hash for RangeMap +where + K: Hash, + V: Hash, +{ + fn hash(&self, state: &mut H) { + state.write_usize(self.btm.len()); + for elt in self.iter() { + elt.hash(state); + } + } +} + impl PartialEq for RangeMap where K: PartialEq, diff --git a/src/range_wrapper.rs b/src/range_wrapper.rs index c3804ba..0a54ee7 100644 --- a/src/range_wrapper.rs +++ b/src/range_wrapper.rs @@ -32,7 +32,7 @@ use core::ops::{Deref, Range, RangeInclusive}; // Range start wrapper // -#[derive(Debug, Clone, Hash)] +#[derive(Debug, Clone)] pub struct RangeStartWrapper { pub end_wrapper: RangeEndWrapper, } @@ -94,7 +94,7 @@ impl Deref for RangeStartWrapper { // Range end wrapper // -#[derive(Debug, Clone, Hash)] +#[derive(Debug, Clone)] pub struct RangeEndWrapper { pub range: Range, } @@ -148,7 +148,7 @@ impl Deref for RangeEndWrapper { // RangeInclusive start wrapper // -#[derive(Eq, Debug, Clone, Hash)] +#[derive(Eq, Debug, Clone)] pub struct RangeInclusiveStartWrapper { pub end_wrapper: RangeInclusiveEndWrapper, } @@ -208,7 +208,7 @@ impl Deref for RangeInclusiveStartWrapper { // RangeInclusive end wrapper // -#[derive(Eq, Debug, Clone, Hash)] +#[derive(Eq, Debug, Clone)] pub struct RangeInclusiveEndWrapper { pub range: RangeInclusive, }