1

I am implementing a cache which tries a lookup in a table, if that fails it tries a simple method to get the value, and if that fails, it goes and computes multiple new entries in the cache. The Entry system seems specifically designed for the first half of this, but I cannot get the borrow checker to allow me to complete the last half.

use std::collections::HashMap;
fn main() {
    let mut cache = Cache { cache: HashMap::new() };
    println!("{}", cache.get_from_cache(10));

}


struct Cache {
    cache: HashMap<u32, String>
}
impl Cache {
    fn get_from_cache<'a>(&'a mut self, i: u32) -> &'a String {
        match self.cache.entry(i) {
            std::collections::hash_map::Entry::Occupied(entry) => return entry.into_mut(),
            std::collections::hash_map::Entry::Vacant(entry) => {
                // Some values have an easy way to be computed...
                if i == 5 {
                    return entry.insert("my string".to_string())
                }
            }
        }

        // Neither look-up method succeeded, so we 'compute' values one-by-one
        for j in 1..=i {
            self.cache.insert(j, "another string".to_string()); // Borrow checker fails here
        }
        self.cache.get(&i).unwrap()
        
    }
    
}

The problem is that the Entry from self.cache.entry(i) borrows self.cache for the entire lifetime 'a even though I no longer need it at the point that I attempt to do self.cache.insert.

One solution to this (I think) would be if there were a way to turn my reference to the Entry into a reference to its HashMap and then insert through that reference. However, I see no way to do this with the entry interface. Is there some way to achieve this? Or to otherwise satisfy the borrow checker?

tgbrooks
  • 241
  • 1
  • 10
  • The `entry` API exists to avoid [issues like this](https://github.com/rust-lang/rust/issues/54663) caused by limitations of the borrow checker. I think you'll have to use a redundant `contains_key` check as a workaround for now. – PitaJ Nov 22 '22 at 19:32

1 Answers1

1

This is easily fixed by separating inserting values from returning the final result. You can first make sure the value is in the cache or insert it with some strategy if not, and then return the new value (now guaranteed to be in the HashMap):

fn get_from_cache<'a>(&'a mut self, i: u32) -> &'a String {
    // handle inserting the value if necessary:
    match self.cache.entry(i) {
        std::collections::hash_map::Entry::Occupied(entry) => (),
        // Some values have an easy way to be computed...
        std::collections::hash_map::Entry::Vacant(entry) if i == 5 => {
            entry.insert("my string".to_string());
        }
        // ... others aren't
        std::collections::hash_map::Entry::Vacant(entry) => {
            for j in 1..=i {
                self.cache.insert(j, "another string".to_string());
            }
        }
    }

    // The value is now definitely in `self.cache` so we can return a reference to it
    &self.cache[&i]
    
}
isaactfa
  • 5,461
  • 1
  • 10
  • 24
  • `&self.cache[&i]` is essentially the same as `self.cache.get(&i).unwrap()` and has a redundant check that may or may not be optimized out. – PitaJ Nov 22 '22 at 20:55
  • @PitaJ Really? Looking at the source code it looks like index literally just calls `self.get(key).expect(...)`. – isaactfa Nov 22 '22 at 21:20
  • `expect` is just `unwrap` with a specified message. – PitaJ Nov 22 '22 at 21:22
  • Yeah, so I don't get what the difference between `&self.cache[&i]` and `self.cache.get(&i).unwrap()` is supposed to be? – isaactfa Nov 22 '22 at 21:24
  • I think there's been a misunderstanding. My point was precisely that they're the same and that BOTH introduce a redundant check for the presence of the entry under `&i`. That redundant check would not be necessary if the [borrow checker was not limited](https://github.com/rust-lang/rust/issues/54663). – PitaJ Nov 22 '22 at 21:29
  • 1
    Ah, sorry, I see what you mean now. I didn't mean to imply that indexing wouldn't do a check that `get` does do (I just thought it looked cleaner) but I can see why you'd think that. Yeah, obviously it would be nice to just return the reference when we've already checked that there's a value there. Someday, Polonius, someday... – isaactfa Nov 22 '22 at 21:33
  • So why doesn't `entry` still have a mutable reference to `self.cache` at the point of doing `self.cache.insert` in this example? In this case, the NLL was able to deduce that entry wasn't used after that? It's disappointing that the solution is basically "don't use `entry`" even though `Entry` was made for problems pretty similar to this. – tgbrooks Nov 23 '22 at 11:59