Skip to content

Commit

Permalink
Fix Hash implementations
Browse files Browse the repository at this point in the history
They should hash the underlying range-value pairs, not the
wrapper structs.
  • Loading branch information
jeffparsons committed Feb 6, 2024
1 parent 3693ff1 commit bc926d1
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 49 deletions.
38 changes: 27 additions & 11 deletions src/inclusive_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -33,14 +34,27 @@ 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<K, V, StepFnsT = K> {
// Wrap ranges so that they are `Ord`.
// See `range_wrapper.rs` for explanation.
pub(crate) btm: BTreeMap<RangeInclusiveStartWrapper<K>, V>,
_phantom: PhantomData<StepFnsT>,
}

impl<K, V, StepFnsT> Hash for RangeInclusiveMap<K, V, StepFnsT>
where
K: Hash,
V: Hash,
{
fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
state.write_usize(self.btm.len());
for elt in self.iter() {
elt.hash(state);
}
}
}

impl<K, V, StepFnsT> PartialEq for RangeInclusiveMap<K, V, StepFnsT>
where
K: PartialEq,
Expand Down Expand Up @@ -80,6 +94,18 @@ where
}
}

impl<K, V, StepFnsT> RangeInclusiveMap<K, V, StepFnsT> {
/// Gets an iterator over all pairs of key range and value,
/// ordered by key range.
///
/// The iterator element type is `(&'a RangeInclusive<K>, &'a V)`.
pub fn iter(&self) -> Iter<'_, K, V> {
Iter {
inner: self.btm.iter(),
}
}
}

impl<K, V> RangeInclusiveMap<K, V, K>
where
K: Ord + Clone + StepLite,
Expand Down Expand Up @@ -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<K>, &'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();
Expand Down
34 changes: 1 addition & 33 deletions src/inclusive_set.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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)`.
///
Expand All @@ -26,37 +25,6 @@ pub struct RangeInclusiveSet<T, StepFnsT = T> {
rm: RangeInclusiveMap<T, (), StepFnsT>,
}

impl<T, StepFnsT> PartialEq for RangeInclusiveSet<T, StepFnsT>
where
T: PartialEq,
{
fn eq(&self, other: &Self) -> bool {
self.rm == other.rm
}
}

impl<T, StepFnsT> Eq for RangeInclusiveSet<T, StepFnsT> where T: Eq {}

impl<T, StepFnsT> PartialOrd for RangeInclusiveSet<T, StepFnsT>
where
T: PartialOrd,
{
#[inline]
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
self.rm.partial_cmp(&other.rm)
}
}

impl<T, StepFnsT> Ord for RangeInclusiveSet<T, StepFnsT>
where
T: Ord,
{
#[inline]
fn cmp(&self, other: &Self) -> Ordering {
self.rm.cmp(&other.rm)
}
}

impl<T> RangeInclusiveSet<T, T>
where
T: Ord + Clone + StepLite,
Expand Down
16 changes: 15 additions & 1 deletion src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::*;
Expand All @@ -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<K, V> {
// Wrap ranges so that they are `Ord`.
// See `range_wrapper.rs` for explanation.
pub(crate) btm: BTreeMap<RangeStartWrapper<K>, V>,
}

impl<K, V> Hash for RangeMap<K, V>
where
K: Hash,
V: Hash,
{
fn hash<H: core::hash::Hasher>(&self, state: &mut H) {
state.write_usize(self.btm.len());
for elt in self.iter() {
elt.hash(state);
}
}
}

impl<K, V> PartialEq for RangeMap<K, V>
where
K: PartialEq,
Expand Down
8 changes: 4 additions & 4 deletions src/range_wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use core::ops::{Deref, Range, RangeInclusive};
// Range start wrapper
//

#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone)]
pub struct RangeStartWrapper<T> {
pub end_wrapper: RangeEndWrapper<T>,
}
Expand Down Expand Up @@ -94,7 +94,7 @@ impl<T> Deref for RangeStartWrapper<T> {
// Range end wrapper
//

#[derive(Debug, Clone, Hash)]
#[derive(Debug, Clone)]
pub struct RangeEndWrapper<T> {
pub range: Range<T>,
}
Expand Down Expand Up @@ -148,7 +148,7 @@ impl<T> Deref for RangeEndWrapper<T> {
// RangeInclusive start wrapper
//

#[derive(Eq, Debug, Clone, Hash)]
#[derive(Eq, Debug, Clone)]
pub struct RangeInclusiveStartWrapper<T> {
pub end_wrapper: RangeInclusiveEndWrapper<T>,
}
Expand Down Expand Up @@ -208,7 +208,7 @@ impl<T> Deref for RangeInclusiveStartWrapper<T> {
// RangeInclusive end wrapper
//

#[derive(Eq, Debug, Clone, Hash)]
#[derive(Eq, Debug, Clone)]
pub struct RangeInclusiveEndWrapper<T> {
pub range: RangeInclusive<T>,
}
Expand Down

0 comments on commit bc926d1

Please sign in to comment.