implement entry interface with or_insert and tests succesfully #2
Conversation
…ec and waste less
…le instances and entries
… sense as a public API
…ing a value when we already have the key in our table
src/lib.rs
Outdated
| @@ -73,6 +73,38 @@ impl<K: std::hash::Hash + PartialEq, V> HashTable<K, V> { | |||
| } | |||
| } | |||
|
|
|||
| pub fn insert_mut(&mut self, k: K, v: V) -> &mut V { | |||
There was a problem hiding this comment.
Completely duplicates insert + reads the value back out of the table at the end just to get a mutable reference back...
| @@ -84,9 +116,45 @@ impl<K: std::hash::Hash + PartialEq, V> HashTable<K, V> { | |||
| None | |||
| } | |||
|
|
|||
| pub fn get_mut(&mut self, k: &K) -> Option<&mut V> { | |||
There was a problem hiding this comment.
Duplicates get just to get a mut reference which feels weird...
There was a problem hiding this comment.
I think this is how its meant to be. I am 85% confident.
src/lib.rs
Outdated
| } | ||
| } | ||
|
|
||
| self.buckets = new_buckets; | ||
| } | ||
|
|
||
| // then add the new item (give up ownership late so we can easily access the value for returning) | ||
| let hash = self.hasher.hash(&k); |
There was a problem hiding this comment.
hash is computed twice for calls from insert (insert_mut is not necessary anymore either; certainly not as a pub fn).
src/lib.rs
Outdated
| } | ||
| match to_remove { | ||
| Some(index) => { | ||
| let (_, ov) = self.buckets[bucket_index].swap_remove(index); |
There was a problem hiding this comment.
Do we need to do it like this? Can we just swap the key-value pairs entirely and just skip any of _insert?
| pub fn capacity(&self) -> usize { | ||
| self.buckets.len() | ||
| } | ||
|
|
||
| pub fn entry(&mut self, k: K) -> Entry<'_, K, V, H> { | ||
| if self.get_mut(&k).is_some() { |
There was a problem hiding this comment.
It is very silly that I call get_mut twice: once here to check which entry type to return and then again when borrowing the value during a call to entry's or_insert. I tried borrowing the value mutably for occupied entries and for vacant entries, borrowing self. I thought that since they were divergent paths this should have been fine, but it wasn't and the compiler was unhappy:
pub enum Entry<'a, K, V, H>
where
K: Hash,
H: SimpleHasher<K>,
{
Occupied {
v: &'a mut V,
},
Vacant {
ht: &'a mut HashTable<K, V, H>,
k: K,
},
}
error[E0499]: cannot borrow `*self` as mutable more than once at a time
--> src/lib.rs:180:40
|
175 | pub fn entry(&mut self, k: K) -> Entry<'_, K, V, H> {
| - let's call the lifetime of this reference `'1`
176 | let v = self.get_mut(&k);
| ---- first mutable borrow occurs here
177 | if let Some(ov) = v {
178 | return Entry::Occupied { v: ov };
| ------------------------- returning this value requires that `*self` is borrowed for `'1`
179 | } else {
180 | return Entry::Vacant { ht: self, k };
| ^^^^ second mutable borrow occurs here
I think this is related to the non lexical lifetime issues since my exact case is provided here: rust-lang/rust#21906 (comment)
For now, we will just need to call get_mut twice.
No description provided.