diff --git a/src/external_trait_impls/rayon/map.rs b/src/external_trait_impls/rayon/map.rs index 9623ca747..b191bc7d4 100644 --- a/src/external_trait_impls/rayon/map.rs +++ b/src/external_trait_impls/rayon/map.rs @@ -296,7 +296,7 @@ impl HashMap { #[cfg_attr(feature = "inline-more", inline)] pub fn par_keys(&self) -> ParKeys<'_, K, V> { ParKeys { - inner: unsafe { self.table.par_iter() }, + inner: unsafe { self.table.raw.par_iter() }, marker: PhantomData, } } @@ -305,7 +305,7 @@ impl HashMap { #[cfg_attr(feature = "inline-more", inline)] pub fn par_values(&self) -> ParValues<'_, K, V> { ParValues { - inner: unsafe { self.table.par_iter() }, + inner: unsafe { self.table.raw.par_iter() }, marker: PhantomData, } } @@ -316,7 +316,7 @@ impl HashMap { #[cfg_attr(feature = "inline-more", inline)] pub fn par_values_mut(&mut self) -> ParValuesMut<'_, K, V> { ParValuesMut { - inner: unsafe { self.table.par_iter() }, + inner: unsafe { self.table.raw.par_iter() }, marker: PhantomData, } } @@ -326,7 +326,7 @@ impl HashMap { #[cfg_attr(feature = "inline-more", inline)] pub fn par_drain(&mut self) -> ParDrain<'_, K, V, A> { ParDrain { - inner: self.table.par_drain(), + inner: self.table.raw.par_drain(), } } } @@ -357,7 +357,7 @@ impl IntoParallelIterator for HashMap< #[cfg_attr(feature = "inline-more", inline)] fn into_par_iter(self) -> Self::Iter { IntoParIter { - inner: self.table.into_par_iter(), + inner: self.table.raw.into_par_iter(), } } } @@ -369,7 +369,7 @@ impl<'a, K: Sync, V: Sync, S, A: Allocator> IntoParallelIterator for &'a HashMap #[cfg_attr(feature = "inline-more", inline)] fn into_par_iter(self) -> Self::Iter { ParIter { - inner: unsafe { self.table.par_iter() }, + inner: unsafe { self.table.raw.par_iter() }, marker: PhantomData, } } @@ -382,7 +382,7 @@ impl<'a, K: Sync, V: Send, S, A: Allocator> IntoParallelIterator for &'a mut Has #[cfg_attr(feature = "inline-more", inline)] fn into_par_iter(self) -> Self::Iter { ParIterMut { - inner: unsafe { self.table.par_iter() }, + inner: unsafe { self.table.raw.par_iter() }, marker: PhantomData, } } diff --git a/src/map.rs b/src/map.rs index 378bcb0bb..91875bcd7 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1,7 +1,5 @@ -use crate::raw::{ - Allocator, Bucket, Global, RawDrain, RawExtractIf, RawIntoIter, RawIter, RawTable, -}; -use crate::{DefaultHashBuilder, Equivalent, TryReserveError}; +use crate::raw::{Allocator, Bucket, Global, RawExtractIf}; +use crate::{hash_table, DefaultHashBuilder, Equivalent, HashTable, TryReserveError}; use core::borrow::Borrow; use core::fmt::{self, Debug}; use core::hash::{BuildHasher, Hash}; @@ -184,7 +182,7 @@ pub use crate::raw_entry::*; /// ``` pub struct HashMap { pub(crate) hash_builder: S, - pub(crate) table: RawTable<(K, V), A>, + pub(crate) table: HashTable<(K, V), A>, } impl Clone for HashMap { @@ -204,7 +202,7 @@ impl Clone for HashMap(hash_builder: &S) -> impl Fn(&(Q, V)) -> u64 + '_ where @@ -215,7 +213,7 @@ where } /// Ensures that a single closure type across uses of this which, in turn prevents multiple -/// instances of any functions like `RawTable::reserve` from being generated +/// instances of any functions like `HashTable::reserve` from being generated #[cfg_attr(feature = "inline-more", inline)] pub(crate) fn equivalent_key(k: &Q) -> impl Fn(&(K, V)) -> bool + '_ where @@ -225,7 +223,7 @@ where } /// Ensures that a single closure type across uses of this which, in turn prevents multiple -/// instances of any functions like `RawTable::reserve` from being generated +/// instances of any functions like `HashTable::reserve` from being generated #[cfg_attr(feature = "inline-more", inline)] #[allow(dead_code)] pub(crate) fn equivalent(k: &Q) -> impl Fn(&K) -> bool + '_ @@ -457,7 +455,7 @@ impl HashMap { pub const fn with_hasher(hash_builder: S) -> Self { Self { hash_builder, - table: RawTable::new(), + table: HashTable::new(), } } @@ -499,7 +497,7 @@ impl HashMap { pub fn with_capacity_and_hasher(capacity: usize, hash_builder: S) -> Self { Self { hash_builder, - table: RawTable::with_capacity(capacity), + table: HashTable::with_capacity(capacity), } } } @@ -543,7 +541,7 @@ impl HashMap { pub const fn with_hasher_in(hash_builder: S, alloc: A) -> Self { Self { hash_builder, - table: RawTable::new_in(alloc), + table: HashTable::new_in(alloc), } } @@ -578,7 +576,7 @@ impl HashMap { pub fn with_capacity_and_hasher_in(capacity: usize, hash_builder: S, alloc: A) -> Self { Self { hash_builder, - table: RawTable::with_capacity_in(capacity, alloc), + table: HashTable::with_capacity_in(capacity, alloc), } } @@ -752,12 +750,8 @@ impl HashMap { /// ``` #[cfg_attr(feature = "inline-more", inline)] pub fn iter(&self) -> Iter<'_, K, V> { - // Here we tie the lifetime of self to the iter. - unsafe { - Iter { - inner: self.table.iter(), - marker: PhantomData, - } + Iter { + inner: self.table.iter(), } } @@ -797,10 +791,10 @@ impl HashMap { /// ``` #[cfg_attr(feature = "inline-more", inline)] pub fn iter_mut(&mut self) -> IterMut<'_, K, V> { - // Here we tie the lifetime of self to the iter. + // We need iter_unsafe_mut to make the keys variant unsafe { IterMut { - inner: self.table.iter(), + inner: self.table.iter_unsafe_mut(), marker: PhantomData, } } @@ -918,15 +912,8 @@ impl HashMap { where F: FnMut(&K, &mut V) -> bool, { - // Here we only use `iter` as a temporary, preventing use-after-free - unsafe { - for item in self.table.iter() { - let &mut (ref key, ref mut value) = item.as_mut(); - if !f(key, value) { - self.table.erase(item); - } - } - } + self.table + .retain(move |&mut (ref key, ref mut val)| f(key, val)) } /// Drains elements which are true under the given predicate, @@ -980,8 +967,8 @@ impl HashMap { ExtractIf { f, inner: RawExtractIf { - iter: unsafe { self.table.iter() }, - table: &mut self.table, + iter: unsafe { self.table.raw.iter() }, + table: &mut self.table.raw, }, } } @@ -1228,18 +1215,17 @@ where #[cfg_attr(feature = "inline-more", inline)] pub fn entry(&mut self, key: K) -> Entry<'_, K, V, S, A> { let hash = make_hash::(&self.hash_builder, &key); - if let Some(elem) = self.table.find(hash, equivalent_key(&key)) { - Entry::Occupied(OccupiedEntry { - hash, - elem, - table: self, - }) - } else { - Entry::Vacant(VacantEntry { - hash, + let hasher = make_hasher(&self.hash_builder); + match self.table.entry(hash, equivalent_key(&key), hasher) { + hash_table::Entry::Occupied(inner) => Entry::Occupied(OccupiedEntry { + inner, + marker: PhantomData, + }), + hash_table::Entry::Vacant(inner) => Entry::Vacant(VacantEntry { + inner, key, - table: self, - }) + marker: PhantomData, + }), } } @@ -1266,18 +1252,17 @@ where Q: Hash + Equivalent + ?Sized, { let hash = make_hash::(&self.hash_builder, key); - if let Some(elem) = self.table.find(hash, equivalent_key(key)) { - EntryRef::Occupied(OccupiedEntry { - hash, - elem, - table: self, - }) - } else { - EntryRef::Vacant(VacantEntryRef { - hash, + let hasher = make_hasher(&self.hash_builder); + match self.table.entry(hash, equivalent_key(key), hasher) { + hash_table::Entry::Occupied(inner) => EntryRef::Occupied(OccupiedEntry { + inner, + marker: PhantomData, + }), + hash_table::Entry::Vacant(inner) => EntryRef::Vacant(VacantEntryRef { + inner, key, - table: self, - }) + marker: PhantomData, + }), } } @@ -1308,7 +1293,7 @@ where // Avoid `Option::map` because it bloats LLVM IR. if !self.table.is_empty() { let hash = make_hash::(&self.hash_builder, k); - match self.table.get(hash, equivalent_key(k)) { + match self.table.find(hash, equivalent_key(k)) { Some((_, v)) => Some(v), None => None, } @@ -1344,7 +1329,7 @@ where // Avoid `Option::map` because it bloats LLVM IR. if !self.table.is_empty() { let hash = make_hash::(&self.hash_builder, k); - match self.table.get(hash, equivalent_key(k)) { + match self.table.find(hash, equivalent_key(k)) { Some((key, value)) => Some((key, value)), None => None, } @@ -1384,7 +1369,7 @@ where // Avoid `Option::map` because it bloats LLVM IR. if !self.table.is_empty() { let hash = make_hash::(&self.hash_builder, k); - match self.table.get_mut(hash, equivalent_key(k)) { + match self.table.find_mut(hash, equivalent_key(k)) { Some(&mut (ref key, ref mut value)) => Some((key, value)), None => None, } @@ -1419,7 +1404,7 @@ where { if !self.table.is_empty() { let hash = make_hash::(&self.hash_builder, k); - self.table.get(hash, equivalent_key(k)).is_some() + self.table.find(hash, equivalent_key(k)).is_some() } else { false } @@ -1456,7 +1441,7 @@ where // Avoid `Option::map` because it bloats LLVM IR. if !self.table.is_empty() { let hash = make_hash::(&self.hash_builder, k); - match self.table.get_mut(hash, equivalent_key(k)) { + match self.table.find_mut(hash, equivalent_key(k)) { Some(&mut (_, ref mut v)) => Some(v), None => None, } @@ -1839,13 +1824,10 @@ where /// ``` #[cfg_attr(feature = "inline-more", inline)] pub fn insert(&mut self, k: K, v: V) -> Option { - let hash = make_hash::(&self.hash_builder, &k); - match self.find_or_find_insert_index(hash, &k) { - Ok(bucket) => Some(mem::replace(unsafe { &mut bucket.as_mut().1 }, v)), - Err(index) => { - unsafe { - self.table.insert_at_index(hash, index, (k, v)); - } + match self.entry(k) { + Entry::Occupied(mut entry) => Some(entry.insert(v)), + Entry::Vacant(entry) => { + entry.insert(v); None } } @@ -1860,7 +1842,7 @@ where where Q: Equivalent + ?Sized, { - self.table.find_or_find_insert_index( + self.table.raw.find_or_find_insert_index( hash, equivalent_key(key), make_hasher(&self.hash_builder), @@ -1927,10 +1909,10 @@ where #[cfg_attr(feature = "inline-more", inline)] pub unsafe fn insert_unique_unchecked(&mut self, k: K, v: V) -> (&K, &mut V) { let hash = make_hash::(&self.hash_builder, &k); - let bucket = self - .table - .insert(hash, (k, v), make_hasher::<_, V, S>(&self.hash_builder)); - let (k_ref, v_ref) = unsafe { bucket.as_mut() }; + let entry = + self.table + .insert_unique(hash, (k, v), make_hasher::<_, V, S>(&self.hash_builder)); + let (k_ref, v_ref) = entry.into_mut(); (k_ref, v_ref) } @@ -2046,7 +2028,7 @@ where Q: Hash + Equivalent + ?Sized, { let hash = make_hash::(&self.hash_builder, k); - self.table.remove_entry(hash, equivalent_key(k)) + self.table.raw.remove_entry(hash, equivalent_key(k)) } /// Returns the total amount of memory allocated internally by the hash @@ -2204,8 +2186,7 @@ where /// assert_eq!(iter.next(), None); /// ``` pub struct Iter<'a, K, V> { - inner: RawIter<(K, V)>, - marker: PhantomData<(&'a K, &'a V)>, + inner: hash_table::Iter<'a, (K, V)>, } // FIXME(#26925) Remove in favor of `#[derive(Clone)]` @@ -2214,7 +2195,6 @@ impl Clone for Iter<'_, K, V> { fn clone(&self) -> Self { Iter { inner: self.inner.clone(), - marker: PhantomData, } } } @@ -2253,7 +2233,7 @@ impl fmt::Debug for Iter<'_, K, V> { /// assert_eq!(map.get(&2).unwrap(), &"Two Mississippi".to_owned()); /// ``` pub struct IterMut<'a, K, V> { - inner: RawIter<(K, V)>, + inner: hash_table::IterUnsafeMut<'a, (K, V)>, // To ensure invariance with respect to V marker: PhantomData<(&'a K, &'a mut V)>, } @@ -2268,8 +2248,7 @@ impl IterMut<'_, K, V> { #[cfg_attr(feature = "inline-more", inline)] pub(super) fn iter(&self) -> Iter<'_, K, V> { Iter { - inner: self.inner.clone(), - marker: PhantomData, + inner: self.inner.iter(), } } } @@ -2305,7 +2284,7 @@ impl IterMut<'_, K, V> { /// assert_eq!(iter.next(), None); /// ``` pub struct IntoIter { - inner: RawIntoIter<(K, V), A>, + inner: hash_table::IntoIter<(K, V), A>, } impl IntoIter { @@ -2314,7 +2293,6 @@ impl IntoIter { pub(super) fn iter(&self) -> Iter<'_, K, V> { Iter { inner: self.inner.iter(), - marker: PhantomData, } } } @@ -2601,7 +2579,7 @@ impl fmt::Debug for Values<'_, K, V> { /// assert_eq!(drain_iter.next(), None); /// ``` pub struct Drain<'a, K, V, A: Allocator = Global> { - inner: RawDrain<'a, (K, V), A>, + inner: hash_table::Drain<'a, (K, V), A>, } impl Drain<'_, K, V, A> { @@ -2610,7 +2588,6 @@ impl Drain<'_, K, V, A> { pub(super) fn iter(&self) -> Iter<'_, K, V> { Iter { inner: self.inner.iter(), - marker: PhantomData, } } } @@ -2829,9 +2806,8 @@ impl Debug for Entry<'_, K, V, S, A> { /// assert_eq!(map.len(), 2); /// ``` pub struct OccupiedEntry<'a, K, V, S = DefaultHashBuilder, A: Allocator = Global> { - hash: u64, - elem: Bucket<(K, V)>, - table: &'a mut HashMap, + inner: hash_table::OccupiedEntry<'a, (K, V), A>, + marker: PhantomData<&'a mut S>, } unsafe impl Send for OccupiedEntry<'_, K, V, S, A> @@ -2891,9 +2867,9 @@ impl Debug for OccupiedEntry<'_, K, V, S, A /// assert!(map[&"b"] == 20 && map.len() == 2); /// ``` pub struct VacantEntry<'a, K, V, S = DefaultHashBuilder, A: Allocator = Global> { - hash: u64, + inner: hash_table::VacantEntry<'a, (K, V), A>, key: K, - table: &'a mut HashMap, + marker: PhantomData<&'a mut S>, } impl Debug for VacantEntry<'_, K, V, S, A> { @@ -3035,9 +3011,9 @@ where /// assert!(map["b"] == 20 && map.len() == 2); /// ``` pub struct VacantEntryRef<'map, 'key, K, Q: ?Sized, V, S, A: Allocator = Global> { - hash: u64, + inner: hash_table::VacantEntry<'map, (K, V), A>, key: &'key Q, - table: &'map mut HashMap, + marker: PhantomData<&'map mut S>, } impl Debug for VacantEntryRef<'_, '_, K, Q, V, S, A> @@ -3208,21 +3184,17 @@ impl Default for Iter<'_, K, V> { fn default() -> Self { Self { inner: Default::default(), - marker: PhantomData, } } } -impl<'a, K, V> Iterator for Iter<'a, K, V> { +impl<'a, K: 'a, V: 'a> Iterator for Iter<'a, K, V> { type Item = (&'a K, &'a V); #[cfg_attr(feature = "inline-more", inline)] fn next(&mut self) -> Option<(&'a K, &'a V)> { // Avoid `Option::map` because it bloats LLVM IR. match self.inner.next() { - Some(x) => unsafe { - let r = x.as_ref(); - Some((&r.0, &r.1)) - }, + Some(x) => Some((&x.0, &x.1)), None => None, } } @@ -3236,20 +3208,17 @@ impl<'a, K, V> Iterator for Iter<'a, K, V> { Self: Sized, F: FnMut(B, Self::Item) -> B, { - self.inner.fold(init, |acc, x| unsafe { - let (k, v) = x.as_ref(); - f(acc, (k, v)) - }) + self.inner.fold(init, |acc, (k, v)| f(acc, (k, v))) } } -impl ExactSizeIterator for Iter<'_, K, V> { +impl<'a, K: 'a, V: 'a> ExactSizeIterator for Iter<'a, K, V> { #[cfg_attr(feature = "inline-more", inline)] fn len(&self) -> usize { self.inner.len() } } -impl FusedIterator for Iter<'_, K, V> {} +impl<'a, K: 'a, V: 'a> FusedIterator for Iter<'a, K, V> {} impl Default for IterMut<'_, K, V> { #[cfg_attr(feature = "inline-more", inline)] @@ -3267,10 +3236,7 @@ impl<'a, K, V> Iterator for IterMut<'a, K, V> { fn next(&mut self) -> Option<(&'a K, &'a mut V)> { // Avoid `Option::map` because it bloats LLVM IR. match self.inner.next() { - Some(x) => unsafe { - let r = x.as_mut(); - Some((&r.0, &mut r.1)) - }, + Some(x) => Some((&x.0, &mut x.1)), None => None, } } @@ -3284,10 +3250,7 @@ impl<'a, K, V> Iterator for IterMut<'a, K, V> { Self: Sized, F: FnMut(B, Self::Item) -> B, { - self.inner.fold(init, |acc, x| unsafe { - let (k, v) = x.as_mut(); - f(acc, (k, v)) - }) + self.inner.fold(init, |acc, (k, v)| f(acc, (k, v))) } } impl ExactSizeIterator for IterMut<'_, K, V> { @@ -3847,7 +3810,7 @@ impl<'a, K, V, S, A: Allocator> OccupiedEntry<'a, K, V, S, A> { /// ``` #[cfg_attr(feature = "inline-more", inline)] pub fn key(&self) -> &K { - unsafe { &self.elem.as_ref().0 } + &self.inner.get().0 } /// Take the ownership of the key and value from the map. @@ -3876,7 +3839,7 @@ impl<'a, K, V, S, A: Allocator> OccupiedEntry<'a, K, V, S, A> { /// ``` #[cfg_attr(feature = "inline-more", inline)] pub fn remove_entry(self) -> (K, V) { - unsafe { self.table.table.remove(self.elem).0 } + self.inner.remove().0 } /// Gets a reference to the value in the entry. @@ -3897,7 +3860,7 @@ impl<'a, K, V, S, A: Allocator> OccupiedEntry<'a, K, V, S, A> { /// ``` #[cfg_attr(feature = "inline-more", inline)] pub fn get(&self) -> &V { - unsafe { &self.elem.as_ref().1 } + &self.inner.get().1 } /// Gets a mutable reference to the value in the entry. @@ -3929,7 +3892,7 @@ impl<'a, K, V, S, A: Allocator> OccupiedEntry<'a, K, V, S, A> { /// ``` #[cfg_attr(feature = "inline-more", inline)] pub fn get_mut(&mut self) -> &mut V { - unsafe { &mut self.elem.as_mut().1 } + &mut self.inner.get_mut().1 } /// Converts the `OccupiedEntry` into a mutable reference to the value in the entry @@ -3960,7 +3923,43 @@ impl<'a, K, V, S, A: Allocator> OccupiedEntry<'a, K, V, S, A> { /// ``` #[cfg_attr(feature = "inline-more", inline)] pub fn into_mut(self) -> &'a mut V { - unsafe { &mut self.elem.as_mut().1 } + &mut self.inner.into_mut().1 + } + + /// Converts the `OccupiedEntry` into a reference to the key and a + /// mutable reference to the value in the entry with a lifetime bound to the + /// map itself. + /// + /// If you need multiple references to the `OccupiedEntry`, see [`key`] and + /// [`get_mut`]. + /// + /// [`key`]: Self::key + /// [`get_mut`]: Self::get_mut + /// + /// # Examples + /// + /// ``` + /// use hashbrown::hash_map::{Entry, HashMap}; + /// + /// let mut map: HashMap<&str, u32> = HashMap::new(); + /// map.entry("poneyland").or_insert(12); + /// + /// assert_eq!(map["poneyland"], 12); + /// + /// let key_val: (&&str, &mut u32); + /// match map.entry("poneyland") { + /// Entry::Occupied(entry) => key_val = entry.into_entry(), + /// Entry::Vacant(_) => panic!(), + /// } + /// *key_val.1 += 10; + /// + /// assert_eq!(key_val, (&"poneyland", &mut 22)); + /// assert_eq!(map["poneyland"], 22); + /// ``` + #[cfg_attr(feature = "inline-more", inline)] + pub fn into_entry(self) -> (&'a K, &'a mut V) { + let (key, val) = self.inner.into_mut(); + (key, val) } /// Sets the value of the entry, and returns the entry's old value. @@ -4066,29 +4065,25 @@ impl<'a, K, V, S, A: Allocator> OccupiedEntry<'a, K, V, S, A> { where F: FnOnce(&K, V) -> Option, { - unsafe { - let mut spare_key = None; + let mut spare_key = None; - self.table - .table - .replace_bucket_with(self.elem.clone(), |(key, value)| { - if let Some(new_value) = f(&key, value) { - Some((key, new_value)) - } else { - spare_key = Some(key); - None - } - }); - - if let Some(key) = spare_key { - Entry::Vacant(VacantEntry { - hash: self.hash, - key, - table: self.table, - }) + match self.inner.replace_entry_with(|(key, value)| { + if let Some(new_value) = f(&key, value) { + Some((key, new_value)) } else { - Entry::Occupied(self) + spare_key = Some(key); + None } + }) { + hash_table::Entry::Vacant(inner) => Entry::Vacant(VacantEntry { + inner, + key: unsafe { spare_key.unwrap_unchecked() }, + marker: PhantomData, + }), + hash_table::Entry::Occupied(inner) => Entry::Occupied(OccupiedEntry { + inner, + marker: PhantomData, + }), } } } @@ -4151,13 +4146,7 @@ impl<'a, K, V, S, A: Allocator> VacantEntry<'a, K, V, S, A> { K: Hash, S: BuildHasher, { - let table = &mut self.table.table; - let entry = table.insert_entry( - self.hash, - (self.key, value), - make_hasher::<_, V, S>(&self.table.hash_builder), - ); - &mut entry.1 + &mut self.inner.insert((self.key, value)).into_mut().1 } /// Sets the value of the entry with the [`VacantEntry`]'s key, @@ -4182,15 +4171,9 @@ impl<'a, K, V, S, A: Allocator> VacantEntry<'a, K, V, S, A> { K: Hash, S: BuildHasher, { - let elem = self.table.table.insert( - self.hash, - (self.key, value), - make_hasher::<_, V, S>(&self.table.hash_builder), - ); OccupiedEntry { - hash: self.hash, - elem, - table: self.table, + inner: self.inner.insert((self.key, value)), + marker: PhantomData, } } } @@ -4490,13 +4473,7 @@ impl<'map, 'key, K, Q: ?Sized, V, S, A: Allocator> VacantEntryRef<'map, 'key, K, &'key Q: Into, S: BuildHasher, { - let table = &mut self.table.table; - let entry = table.insert_entry( - self.hash, - (self.key.into(), value), - make_hasher::<_, V, S>(&self.table.hash_builder), - ); - &mut entry.1 + &mut self.inner.insert((self.key.into(), value)).into_mut().1 } /// Sets the key and value of the entry and returns a mutable reference to @@ -4535,17 +4512,7 @@ impl<'map, 'key, K, Q: ?Sized, V, S, A: Allocator> VacantEntryRef<'map, 'key, K, Q: Equivalent, S: BuildHasher, { - let table = &mut self.table.table; - assert!( - (self.key).equivalent(&key), - "key used for Entry creation is not equivalent to the one used for insertion" - ); - let entry = table.insert_entry( - self.hash, - (key, value), - make_hasher::<_, V, S>(&self.table.hash_builder), - ); - &mut entry.1 + self.insert_entry_with_key(key, value).into_mut() } /// Sets the value of the entry with the [`VacantEntryRef`]'s key, @@ -4571,15 +4538,54 @@ impl<'map, 'key, K, Q: ?Sized, V, S, A: Allocator> VacantEntryRef<'map, 'key, K, &'key Q: Into, S: BuildHasher, { - let elem = self.table.table.insert( - self.hash, - (self.key.into(), value), - make_hasher::<_, V, S>(&self.table.hash_builder), + OccupiedEntry { + inner: self.inner.insert((self.key.into(), value)), + marker: PhantomData, + } + } + + /// Sets the key and value of the entry and returns an [`OccupiedEntry`]. + /// + /// Unlike [`VacantEntryRef::insert_entry`], this method allows the key to + /// be explicitly specified, which is useful for key types that don't + /// implement `K: From<&Q>`. + /// + /// # Panics + /// + /// This method panics if `key` is not equivalent to the key used to create + /// the `VacantEntryRef`. + /// + /// # Example + /// + /// ``` + /// use hashbrown::hash_map::EntryRef; + /// use hashbrown::HashMap; + /// + /// let mut map = HashMap::<(String, String), char>::new(); + /// let k = ("c".to_string(), "C".to_string()); + /// let r = match map.entry_ref(&k) { + /// // Insert cannot be used here because tuples do not implement From. + /// // However this works because we can manually clone instead. + /// EntryRef::Vacant(r) => r.insert_entry_with_key(k.clone(), 'c'), + /// // In this branch we avoid the clone. + /// EntryRef::Occupied(r) => r, + /// }; + /// assert_eq!(r.get(), &'c'); + /// ``` + #[cfg_attr(feature = "inline-more", inline)] + pub fn insert_entry_with_key(self, key: K, value: V) -> OccupiedEntry<'map, K, V, S, A> + where + K: Hash, + Q: Equivalent, + S: BuildHasher, + { + assert!( + (self.key).equivalent(&key), + "key used for Entry creation is not equivalent to the one used for insertion" ); OccupiedEntry { - hash: self.hash, - elem, - table: self.table, + inner: self.inner.insert((key, value)), + marker: PhantomData, } } } @@ -6417,7 +6423,7 @@ mod test_map { "panic_in_drop can be set with a type that doesn't need to be dropped", )); } - guard.table.insert( + guard.table.raw.insert( count, ( count, @@ -6587,8 +6593,8 @@ mod test_map { // the returned `RawIter / RawIterRange` iterator. assert_eq!(map.len(), 0); assert_eq!(map.iter().count(), 0); - assert_eq!(unsafe { map.table.iter().count() }, 0); - assert_eq!(unsafe { map.table.iter().iter.count() }, 0); + assert_eq!(unsafe { map.table.raw.iter().count() }, 0); + assert_eq!(unsafe { map.table.raw.iter().iter.count() }, 0); for idx in 0..map.table.num_buckets() { let idx = idx as u64; @@ -6655,8 +6661,8 @@ mod test_map { // the returned `RawIter / RawIterRange` iterator. assert_eq!(map.len(), 0); assert_eq!(map.iter().count(), 0); - assert_eq!(unsafe { map.table.iter().count() }, 0); - assert_eq!(unsafe { map.table.iter().iter.count() }, 0); + assert_eq!(unsafe { map.table.raw.iter().count() }, 0); + assert_eq!(unsafe { map.table.raw.iter().iter.count() }, 0); for idx in 0..map.table.num_buckets() { let idx = idx as u64; diff --git a/src/raw/mod.rs b/src/raw/mod.rs index 3564120d4..0e1664c7f 100644 --- a/src/raw/mod.rs +++ b/src/raw/mod.rs @@ -1096,14 +1096,6 @@ impl RawTable { } } - /// Inserts a new element into the table, and returns a mutable reference to it. - /// - /// This does not check if the given element already exists in the table. - #[cfg_attr(feature = "inline-more", inline)] - pub fn insert_entry(&mut self, hash: u64, value: T, hasher: impl Fn(&T) -> u64) -> &mut T { - unsafe { self.insert(hash, value, hasher).as_mut() } - } - /// Inserts a new element into the table, without growing the table. /// /// There must be enough space in the table to insert the new element. @@ -1124,14 +1116,14 @@ impl RawTable { bucket } - /// Temporary removes a bucket, applying the given function to the removed + /// Temporarily removes a bucket, applying the given function to the removed /// element and optionally put back the returned value in the same bucket. /// - /// Returns `true` if the bucket still contains an element + /// Returns tag for bucket if the bucket is emptied out. /// /// This does not check if the given bucket is actually occupied. #[cfg_attr(feature = "inline-more", inline)] - pub unsafe fn replace_bucket_with(&mut self, bucket: Bucket, f: F) -> bool + pub(crate) unsafe fn replace_bucket_with(&mut self, bucket: Bucket, f: F) -> Option where F: FnOnce(T) -> Option, { @@ -1145,9 +1137,9 @@ impl RawTable { self.table.set_ctrl(index, old_ctrl); self.table.items += 1; self.bucket(index).write(new_item); - true + None } else { - false + Some(old_ctrl) } } diff --git a/src/raw_entry.rs b/src/raw_entry.rs index 480ebdbe1..72d2de6e6 100644 --- a/src/raw_entry.rs +++ b/src/raw_entry.rs @@ -600,14 +600,14 @@ impl<'a, K, V, S, A: Allocator> RawEntryBuilderMut<'a, K, V, S, A> { where for<'b> F: FnMut(&'b K) -> bool, { - match self.map.table.find(hash, |(k, _)| is_match(k)) { + match self.map.table.raw.find(hash, |(k, _)| is_match(k)) { Some(elem) => RawEntryMut::Occupied(RawOccupiedEntryMut { elem, - table: &mut self.map.table, + table: &mut self.map.table.raw, hash_builder: &self.map.hash_builder, }), None => RawEntryMut::Vacant(RawVacantEntryMut { - table: &mut self.map.table, + table: &mut self.map.table.raw, hash_builder: &self.map.hash_builder, }), } @@ -671,7 +671,7 @@ impl<'a, K, V, S, A: Allocator> RawEntryBuilder<'a, K, V, S, A> { where F: FnMut(&K) -> bool, { - match self.map.table.get(hash, |(k, _)| is_match(k)) { + match self.map.table.raw.get(hash, |(k, _)| is_match(k)) { Some((key, value)) => Some((key, value)), None => None, } @@ -1282,13 +1282,13 @@ impl<'a, K, V, S, A: Allocator> RawOccupiedEntryMut<'a, K, V, S, A> { F: FnOnce(&K, V) -> Option, { unsafe { - let still_occupied = self + let tag = self .table .replace_bucket_with(self.elem.clone(), |(key, value)| { f(&key, value).map(|new_value| (key, new_value)) }); - if still_occupied { + if tag.is_none() { RawEntryMut::Occupied(self) } else { RawEntryMut::Vacant(RawVacantEntryMut { @@ -1365,11 +1365,15 @@ impl<'a, K, V, S, A: Allocator> RawVacantEntryMut<'a, K, V, S, A> { K: Hash, S: BuildHasher, { - let &mut (ref mut k, ref mut v) = self.table.insert_entry( - hash, - (key, value), - make_hasher::<_, V, S>(self.hash_builder), - ); + let &mut (ref mut k, ref mut v) = unsafe { + self.table + .insert( + hash, + (key, value), + make_hasher::<_, V, S>(self.hash_builder), + ) + .as_mut() + }; (k, v) } @@ -1420,9 +1424,11 @@ impl<'a, K, V, S, A: Allocator> RawVacantEntryMut<'a, K, V, S, A> { where H: Fn(&K) -> u64, { - let &mut (ref mut k, ref mut v) = self - .table - .insert_entry(hash, (key, value), |x| hasher(&x.0)); + let &mut (ref mut k, ref mut v) = unsafe { + self.table + .insert(hash, (key, value), |x| hasher(&x.0)) + .as_mut() + }; (k, v) } diff --git a/src/rustc_entry.rs b/src/rustc_entry.rs index 233fe7a2d..446bac151 100644 --- a/src/rustc_entry.rs +++ b/src/rustc_entry.rs @@ -33,10 +33,10 @@ where #[cfg_attr(feature = "inline-more", inline)] pub fn rustc_entry(&mut self, key: K) -> RustcEntry<'_, K, V, A> { let hash = make_hash(&self.hash_builder, &key); - if let Some(elem) = self.table.find(hash, |q| q.0.eq(&key)) { + if let Some(elem) = self.table.raw.find(hash, |q| q.0.eq(&key)) { RustcEntry::Occupied(RustcOccupiedEntry { elem, - table: &mut self.table, + table: &mut self.table.raw, }) } else { // Ideally we would put this in VacantEntry::insert, but Entry is not @@ -47,7 +47,7 @@ where RustcEntry::Vacant(RustcVacantEntry { hash, key, - table: &mut self.table, + table: &mut self.table.raw, }) } } diff --git a/src/set.rs b/src/set.rs index a875d2f50..a9d470785 100644 --- a/src/set.rs +++ b/src/set.rs @@ -408,8 +408,8 @@ impl HashSet { ExtractIf { f, inner: RawExtractIf { - iter: unsafe { self.map.table.iter() }, - table: &mut self.map.table, + iter: unsafe { self.map.table.raw.iter() }, + table: &mut self.map.table.raw, }, } } @@ -912,12 +912,12 @@ where /// ``` #[cfg_attr(feature = "inline-more", inline)] pub fn get_or_insert(&mut self, value: T) -> &T { - let hash = make_hash(&self.map.hash_builder, &value); - let bucket = match self.map.find_or_find_insert_index(hash, &value) { - Ok(bucket) => bucket, - Err(index) => unsafe { self.map.table.insert_at_index(hash, index, (value, ())) }, - }; - unsafe { &bucket.as_ref().0 } + match self.map.entry(value) { + map::Entry::Occupied(entry) => entry, + map::Entry::Vacant(entry) => entry.insert_entry(()), + } + .into_entry() + .0 } /// Inserts a value computed from `f` into the set if the given `value` is @@ -951,16 +951,12 @@ where Q: Hash + Equivalent + ?Sized, F: FnOnce(&Q) -> T, { - let hash = make_hash(&self.map.hash_builder, value); - let bucket = match self.map.find_or_find_insert_index(hash, value) { - Ok(bucket) => bucket, - Err(index) => { - let new = f(value); - assert!(value.equivalent(&new), "new value is not equivalent"); - unsafe { self.map.table.insert_at_index(hash, index, (new, ())) } - } - }; - unsafe { &bucket.as_ref().0 } + match self.map.entry_ref(value) { + map::EntryRef::Occupied(entry) => entry, + map::EntryRef::Vacant(entry) => entry.insert_entry_with_key(f(value), ()), + } + .into_entry() + .0 } /// Gets the given value's corresponding entry in the set for in-place manipulation. @@ -1143,7 +1139,7 @@ where Ok(bucket) => Some(mem::replace(unsafe { &mut bucket.as_mut().0 }, value)), Err(index) => { unsafe { - self.map.table.insert_at_index(hash, index, (value, ())); + self.map.table.raw.insert_at_index(hash, index, (value, ())); } None } @@ -1588,11 +1584,12 @@ where let hash = make_hash(&self.map.hash_builder, item); match self.map.find_or_find_insert_index(hash, item) { Ok(bucket) => unsafe { - self.map.table.remove(bucket); + self.map.table.raw.remove(bucket); }, Err(index) => unsafe { self.map .table + .raw .insert_at_index(hash, index, (item.clone(), ())); }, } diff --git a/src/table.rs b/src/table.rs index 7a99bd7ca..7012d2e7f 100644 --- a/src/table.rs +++ b/src/table.rs @@ -66,6 +66,7 @@ impl HashTable { /// assert_eq!(table.len(), 0); /// assert_eq!(table.capacity(), 0); /// ``` + #[cfg_attr(feature = "rustc-dep-of-std", rustc_const_stable_indirect)] pub const fn new() -> Self { Self { raw: RawTable::new(), @@ -133,6 +134,7 @@ where /// # test() /// # } /// ``` + #[cfg_attr(feature = "rustc-dep-of-std", rustc_const_stable_indirect)] pub const fn new_in(alloc: A) -> Self { Self { raw: RawTable::new_in(alloc), @@ -1071,6 +1073,24 @@ where } } + /// A version of [`iter_mut`](Self::iter_mut) which is variant over + /// elements, and thus unsafe. + /// + /// See the documentation for [`IterUnsafeMut`] for more information on how + /// to correctly use this. + /// + /// # Safety + /// + /// Any part of the returned elements which is mutated must be made invariant. + /// See the documentation for [`IterUnsafeMut`] for an example and further + /// explanation. + pub unsafe fn iter_unsafe_mut(&mut self) -> IterUnsafeMut<'_, T> { + IterUnsafeMut { + inner: unsafe { self.raw.iter() }, + marker: PhantomData, + } + } + /// An iterator producing the `usize` indices of all occupied buckets. /// /// The order in which the iterator yields indices is unspecified @@ -2246,6 +2266,79 @@ where pub fn bucket_index(&self) -> usize { unsafe { self.table.raw.bucket_index(&self.bucket) } } + + /// Provides owned access to the value of the entry and allows to replace or + /// remove it based on the value of the returned option. + /// + /// The hash of the new item should be the same as the old item. + /// + /// # Examples + /// + /// ``` + /// # #[cfg(feature = "nightly")] + /// # fn test() { + /// use hashbrown::{HashTable, DefaultHashBuilder}; + /// use hashbrown::hash_table::Entry; + /// use std::hash::BuildHasher; + /// + /// let mut table = HashTable::new(); + /// let hasher = DefaultHashBuilder::default(); + /// let hasher = |(ref key, _): &_| hasher.hash_one(key); + /// table.insert_unique(hasher(&("poneyland", 42)), ("poneyland", 42), hasher); + /// + /// let entry = match table.entry(hasher(&("poneyland", 42)), |entry| entry.0 == "poneyland", hasher) { + /// Entry::Occupied(e) => unsafe { + /// e.replace_entry_with(|(k, v)| { + /// assert_eq!(k, "poneyland"); + /// assert_eq!(v, 42); + /// Some(("poneyland", v + 1)) + /// }) + /// } + /// Entry::Vacant(_) => panic!(), + /// }; + /// + /// match entry { + /// Entry::Occupied(e) => { + /// assert_eq!(e.get(), &("poneyland", 43)); + /// } + /// Entry::Vacant(_) => panic!(), + /// } + /// + /// let entry = match table.entry(hasher(&("poneyland", 43)), |entry| entry.0 == "poneyland", hasher) { + /// Entry::Occupied(e) => unsafe { e.replace_entry_with(|(_k, _v)| None) }, + /// Entry::Vacant(_) => panic!(), + /// }; + /// + /// match entry { + /// Entry::Vacant(e) => { + /// // nice! + /// } + /// Entry::Occupied(_) => panic!(), + /// } + /// + /// assert!(table.is_empty()); + /// # } + /// # fn main() { + /// # #[cfg(feature = "nightly")] + /// # test() + /// # } + /// ``` + #[cfg_attr(feature = "inline-more", inline)] + pub fn replace_entry_with(self, f: F) -> Entry<'a, T, A> + where + F: FnOnce(T) -> Option, + { + unsafe { + match self.table.raw.replace_bucket_with(self.bucket.clone(), f) { + None => Entry::Occupied(self), + Some(tag) => Entry::Vacant(VacantEntry { + tag, + index: self.bucket_index(), + table: self.table, + }), + } + } + } } /// A view into a vacant entry in a `HashTable`. @@ -2508,6 +2601,16 @@ pub struct IterMut<'a, T> { inner: RawIter, marker: PhantomData<&'a mut T>, } +impl<'a, T> IterMut<'a, T> { + /// Returns a iterator of references over the remaining items. + #[cfg_attr(feature = "inline-more", inline)] + pub fn iter(&self) -> Iter<'_, T> { + Iter { + inner: self.inner.clone(), + marker: PhantomData, + } + } +} impl Default for IterMut<'_, T> { #[cfg_attr(feature = "inline-more", inline)] @@ -2556,12 +2659,113 @@ where T: fmt::Debug, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_list() - .entries(Iter { - inner: self.inner.clone(), - marker: PhantomData, - }) - .finish() + f.debug_list().entries(self.iter()).finish() + } +} + +/// An unsafe version of [`IterMut`] which is *variant* over `T`. +/// +/// This is used for implementations of mutable iterators where mutation is only +/// allowed on part of the value in the table. For example, a mutable iterator +/// for a map may return an immutable key alongside a mutable value, even though +/// these are both stored inside the table. +/// +/// # Safety +/// +/// In order to correctly use this iterator, it should be wrapped in a safe +/// iterator struct with the appropriate [`PhantomData`] marker to indicate the +/// correct variance. +/// +/// For example, a [`hash_map::IterMut`] implementation correctly returning +/// a variant key, and an invariant value: +/// +/// [`hash_map::IterMut`]: crate::hash_map::IterMut +/// +/// ```rust +/// use core::marker::PhantomData; +/// use hashbrown::hash_table; +/// +/// pub struct IterMut<'a, K, V> { +/// inner: hash_table::IterMut<'a, (K, V)>, +/// // Variant over keys, invariant over values +/// marker: PhantomData<(&'a K, &'a mut V)>, +/// } +/// impl<'a, K, V> Iterator for IterMut<'a, K, V> { +/// // Immutable keys, mutable values +/// type Item = (&'a K, &'a mut V); +/// +/// fn next(&mut self) -> Option { +/// // Even though the key is mutably borrowed here, this is sound +/// // because we never actually mutate the key before yielding the +/// // immutable reference +/// let (ref key, ref mut val) = self.inner.next()?; +/// Some((key, val)) +/// } +/// } +/// ``` +pub struct IterUnsafeMut<'a, T> { + inner: RawIter, + marker: PhantomData<&'a T>, +} +impl<'a, T> IterUnsafeMut<'a, T> { + /// Returns a iterator of references over the remaining items. + #[cfg_attr(feature = "inline-more", inline)] + pub fn iter(&self) -> Iter<'_, T> { + Iter { + inner: self.inner.clone(), + marker: PhantomData, + } + } +} + +impl Default for IterUnsafeMut<'_, T> { + #[cfg_attr(feature = "inline-more", inline)] + fn default() -> Self { + IterUnsafeMut { + inner: Default::default(), + marker: PhantomData, + } + } +} +impl<'a, T> Iterator for IterUnsafeMut<'a, T> { + type Item = &'a mut T; + + fn next(&mut self) -> Option { + // Avoid `Option::map` because it bloats LLVM IR. + match self.inner.next() { + Some(bucket) => Some(unsafe { bucket.as_mut() }), + None => None, + } + } + + fn size_hint(&self) -> (usize, Option) { + self.inner.size_hint() + } + + fn fold(self, init: B, mut f: F) -> B + where + Self: Sized, + F: FnMut(B, Self::Item) -> B, + { + self.inner + .fold(init, |acc, bucket| unsafe { f(acc, bucket.as_mut()) }) + } +} + +impl ExactSizeIterator for IterUnsafeMut<'_, T> { + fn len(&self) -> usize { + self.inner.len() + } +} + +impl FusedIterator for IterUnsafeMut<'_, T> {} + +impl fmt::Debug for IterUnsafeMut<'_, T> +where + T: fmt::Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_list().entries(self.iter()).finish() } } @@ -2815,6 +3019,19 @@ where { inner: RawIntoIter, } +impl IntoIter +where + A: Allocator, +{ + /// Returns a iterator of references over the remaining items. + #[cfg_attr(feature = "inline-more", inline)] + pub fn iter(&self) -> Iter<'_, T> { + Iter { + inner: self.inner.iter(), + marker: PhantomData, + } + } +} impl Default for IntoIter { #[cfg_attr(feature = "inline-more", inline)] @@ -2865,12 +3082,7 @@ where A: Allocator, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_list() - .entries(Iter { - inner: self.inner.iter(), - marker: PhantomData, - }) - .finish() + f.debug_list().entries(self.iter()).finish() } } @@ -2884,6 +3096,16 @@ where pub struct Drain<'a, T, A: Allocator = Global> { inner: RawDrain<'a, T, A>, } +impl<'a, T, A: Allocator> Drain<'a, T, A> { + /// Returns a iterator of references over the remaining items. + #[cfg_attr(feature = "inline-more", inline)] + pub fn iter(&self) -> Iter<'_, T> { + Iter { + inner: self.inner.iter(), + marker: PhantomData, + } + } +} impl Iterator for Drain<'_, T, A> { type Item = T; @@ -2915,12 +3137,7 @@ impl FusedIterator for Drain<'_, T, A> {} impl fmt::Debug for Drain<'_, T, A> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_list() - .entries(Iter { - inner: self.inner.iter(), - marker: PhantomData, - }) - .finish() + f.debug_list().entries(self.iter()).finish() } }