Auto merge of #2427 - Nilstrieb:doc-fix, r=saethlin

[rust.git] / src / mono_hash_map.rs

diff --git a/src/mono_hash_map.rs b/src/mono_hash_map.rs
index 124ae68760260b93ca6aa76f0b0d030c9cb4935d..45057632df9b528274b767cbc76c4f87a8eeeb52 100644 (file)
--- a/src/mono_hash_map.rs
+++ b/src/mono_hash_map.rs
@@ -1,6 +1,6 @@
-//! This is a "monotonic `HashMap`": A `HashMap` that, when shared, can be pushed to but not
+//! This is a "monotonic `FxHashMap`": A `FxHashMap` that, when shared, can be pushed to but not
 //! otherwise mutated. We also box items in the map. This means we can safely provide
-//! shared references into existing items in the `HashMap`, because they will not be dropped
+//! shared references into existing items in the `FxHashMap`, because they will not be dropped
 //! (from being removed) or moved (because they are boxed).
 //! The API is is completely tailored to what `memory.rs` needs. It is still in
 //! a separate file to minimize the amount of code that has to care about the unsafety.
@@ -21,8 +21,8 @@ impl<K: Hash + Eq, V> MonoHashMap<K, V> {
     /// This function exists for priroda to be able to iterate over all evaluator memory.
     ///
     /// The function is somewhat roundabout with the closure argument because internally the
-    /// `MonoHashMap` uses a `RefCell`. When iterating over the `HashMap` inside the `RefCell`,
-    /// we need to keep a borrow to the `HashMap` inside the iterator. The borrow is only alive
+    /// `MonoHashMap` uses a `RefCell`. When iterating over the `FxHashMap` inside the `RefCell`,
+    /// we need to keep a borrow to the `FxHashMap` inside the iterator. The borrow is only alive
     /// as long as the `Ref` returned by `RefCell::borrow()` is alive. So we can't return the
     /// iterator, as that would drop the `Ref`. We can't return both, as it's not possible in Rust
     /// to have a struct/tuple with a field that refers to another field.
@@ -61,10 +61,10 @@ fn remove<Q: ?Sized + Hash + Eq>(&mut self, k: &Q) -> Option<V>
 
     #[inline(always)]
     fn filter_map_collect<T>(&self, mut f: impl FnMut(&K, &V) -> Option<T>) -> Vec<T> {
-        self.0.borrow().iter().filter_map(move |(k, v)| f(k, &*v)).collect()
+        self.0.borrow().iter().filter_map(move |(k, v)| f(k, v)).collect()
     }
 
-    /// The most interesting method: Providing a shared ref without
+    /// The most interesting method: Providing a shared reference without
     /// holding the `RefCell` open, and inserting new data if the key
     /// is not used yet.
     /// `vacant` is called if the key is not found in the map;
@@ -72,15 +72,31 @@ fn filter_map_collect<T>(&self, mut f: impl FnMut(&K, &V) -> Option<T>) -> Vec<T
     /// returns owned data, that is put into the map and returned.
     #[inline(always)]
     fn get_or<E>(&self, k: K, vacant: impl FnOnce() -> Result<V, E>) -> Result<&V, E> {
-        let val: *const V = match self.0.borrow_mut().entry(k) {
-            Entry::Occupied(entry) => &**entry.get(),
-            Entry::Vacant(entry) => &**entry.insert(Box::new(vacant()?)),
-        };
+        // We cannot hold borrow_mut while calling `vacant`, since that might have to do lookups in this very map.
+        if let Some(v) = self.0.borrow().get(&k) {
+            let val: *const V = &**v;
+            // This is safe because `val` points into a `Box`, that we know will not move and
+            // will also not be dropped as long as the shared reference `self` is live.
+            return unsafe { Ok(&*val) };
+        }
+        let new_val = Box::new(vacant()?);
+        let val: *const V = &**self.0.borrow_mut().try_insert(k, new_val).ok().unwrap();
         // This is safe because `val` points into a `Box`, that we know will not move and
         // will also not be dropped as long as the shared reference `self` is live.
         unsafe { Ok(&*val) }
     }
 
+    /// Read-only lookup (avoid read-acquiring the RefCell).
+    fn get(&self, k: K) -> Option<&V> {
+        let val: *const V = match self.0.borrow().get(&k) {
+            Some(v) => &**v,
+            None => return None,
+        };
+        // This is safe because `val` points into a `Box`, that we know will not move and
+        // will also not be dropped as long as the shared reference `self` is live.
+        unsafe { Some(&*val) }
+    }
+
     #[inline(always)]
     fn get_mut_or<E>(&mut self, k: K, vacant: impl FnOnce() -> Result<V, E>) -> Result<&mut V, E> {
         match self.0.get_mut().entry(k) {