Skip to content

Commit

Permalink
Change OccupiedEntry::key() to return the existing key in the map
Browse files Browse the repository at this point in the history
The new behavior is consistent with `HashMap` entries in the standard
library, and it has been our intent to match that API when possible.
  • Loading branch information
cuviper committed Jan 20, 2021
1 parent 015eeea commit 29a4f19
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
22 changes: 22 additions & 0 deletions src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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')];
Expand Down
3 changes: 3 additions & 0 deletions src/map/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -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
}
Expand Down
7 changes: 6 additions & 1 deletion src/map/core/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,13 @@ unsafe impl<K: Sync, V: Sync> 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 {
Expand Down

0 comments on commit 29a4f19

Please sign in to comment.