From 29a4f1904efce0b44b14e891e3600adaf0944318 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 19 Jan 2021 17:29:59 -0800 Subject: [PATCH 1/2] Change OccupiedEntry::key() to return the existing key in the map The new behavior is consistent with `HashMap` entries in the standard library, and it has been our intent to match that API when possible. --- src/map.rs | 22 ++++++++++++++++++++++ src/map/core.rs | 3 +++ src/map/core/raw.rs | 7 ++++++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/map.rs b/src/map.rs index bb182ed3..e16e064a 100644 --- a/src/map.rs +++ b/src/map.rs @@ -1648,6 +1648,28 @@ mod tests { assert_eq!(&mut TestEnum::DefaultValue, map.entry(2).or_default()); } + #[test] + fn occupied_entry_key() { + // These keys match hash and equality, but their addresses are distinct. + let (k1, k2) = (&mut 1, &mut 1); + let k1_ptr = k1 as *const i32; + let k2_ptr = k2 as *const i32; + assert_ne!(k1_ptr, k2_ptr); + + let mut map = IndexMap::new(); + map.insert(k1, "value"); + match map.entry(k2) { + Entry::Occupied(ref e) => { + // `OccupiedEntry::key` should reference the key in the map, + // not the key that was used to find the entry. + let ptr = *e.key() as *const i32; + assert_eq!(ptr, k1_ptr); + assert_ne!(ptr, k2_ptr); + }, + Entry::Vacant(_) => panic!(), + } + } + #[test] fn keys() { let vec = vec![(1, 'a'), (2, 'b'), (3, 'c')]; diff --git a/src/map/core.rs b/src/map/core.rs index 02e99e07..9482b2f5 100644 --- a/src/map/core.rs +++ b/src/map/core.rs @@ -464,6 +464,8 @@ impl<'a, K, V> Entry<'a, K, V> { } } + /// Gets a reference to the entry's key, either within the map if occupied, + /// or else the new key that was used to find the entry. pub fn key(&self) -> &K { match *self { Entry::Occupied(ref entry) => entry.key(), @@ -583,6 +585,7 @@ pub struct VacantEntry<'a, K, V> { } impl<'a, K, V> VacantEntry<'a, K, V> { + /// Gets a reference to the key that was used to find the entry. pub fn key(&self) -> &K { &self.key } diff --git a/src/map/core/raw.rs b/src/map/core/raw.rs index 3388f123..be91b3d6 100644 --- a/src/map/core/raw.rs +++ b/src/map/core/raw.rs @@ -104,8 +104,13 @@ unsafe impl Sync for OccupiedEntry<'_, K, V> {} // The parent module also adds methods that don't threaten the unsafe encapsulation. impl<'a, K, V> OccupiedEntry<'a, K, V> { + /// Gets a reference to the entry's key in the map. + /// + /// Note that this is not the key that was used to find the entry. There may be an observable + /// difference if the key type has any distinguishing features outside of `Hash` and `Eq`, like + /// extra fields or the memory address of an allocation. pub fn key(&self) -> &K { - &self.key + &self.map.entries[self.index()].key } pub fn get(&self) -> &V { From 0d801a6c24195099c73612cfedeb96db3bead30f Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 19 Jan 2021 17:37:54 -0800 Subject: [PATCH 2/2] Document all entry methods --- src/map/core.rs | 9 +++++++++ src/map/core/raw.rs | 7 +++++++ 2 files changed, 16 insertions(+) diff --git a/src/map/core.rs b/src/map/core.rs index 9482b2f5..1c261a51 100644 --- a/src/map/core.rs +++ b/src/map/core.rs @@ -445,6 +445,9 @@ pub enum Entry<'a, K, V> { } impl<'a, K, V> Entry<'a, K, V> { + /// Inserts the given default value in the entry if it is vacant and returns a mutable + /// reference to it. Otherwise a mutable reference to an already existent value is returned. + /// /// Computes in **O(1)** time (amortized average). pub fn or_insert(self, default: V) -> &'a mut V { match self { @@ -453,6 +456,9 @@ impl<'a, K, V> Entry<'a, K, V> { } } + /// Inserts the result of the `call` function in the entry if it is vacant and returns a mutable + /// reference to it. Otherwise a mutable reference to an already existent value is returned. + /// /// Computes in **O(1)** time (amortized average). pub fn or_insert_with(self, call: F) -> &'a mut V where @@ -590,6 +596,7 @@ impl<'a, K, V> VacantEntry<'a, K, V> { &self.key } + /// Takes ownership of the key, leaving the entry vacant. pub fn into_key(self) -> K { self.key } @@ -599,6 +606,8 @@ impl<'a, K, V> VacantEntry<'a, K, V> { self.map.len() } + /// Inserts the entry's key and the given value into the map, and returns a mutable reference + /// to the value. pub fn insert(self, value: V) -> &'a mut V { let i = self.map.push(self.hash, self.key, value); &mut self.map.entries[i].value diff --git a/src/map/core/raw.rs b/src/map/core/raw.rs index be91b3d6..0115b07f 100644 --- a/src/map/core/raw.rs +++ b/src/map/core/raw.rs @@ -113,10 +113,15 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> { &self.map.entries[self.index()].key } + /// Gets a reference to the entry's value in the map. pub fn get(&self) -> &V { &self.map.entries[self.index()].value } + /// Gets a mutable reference to the entry's value in the map. + /// + /// If you need a reference which may outlive the destruction of the + /// `Entry` value, see `into_mut`. pub fn get_mut(&mut self) -> &mut V { let index = self.index(); &mut self.map.entries[index].value @@ -136,6 +141,8 @@ impl<'a, K, V> OccupiedEntry<'a, K, V> { unsafe { self.raw_bucket.read() } } + /// Converts into a mutable reference to the entry's value in the map, + /// with a lifetime bound to the map itself. pub fn into_mut(self) -> &'a mut V { let index = self.index(); &mut self.map.entries[index].value