Auto merge of #73971 - ssomers:slice_slasher, r=Mark-Simulacrum

BTreeMap mutable iterators should not take any reference to visited nodes during iteration

Fixes #73915, overlapping mutable references during BTreeMap iteration

r? `@RalfJung`

Author: bors

library/alloc/src/collections/btree/map.rs

@@ -2796,8 +2796,8 @@ enum UnderflowResult<'a, K, V> {
     Stole(bool),
 }
 
-fn handle_underfull_node<K, V>(
-    node: NodeRef<marker::Mut<'_>, K, V, marker::LeafOrInternal>,
+fn handle_underfull_node<'a, K: 'a, V: 'a>(
+    node: NodeRef<marker::Mut<'a>, K, V, marker::LeafOrInternal>,
 ) -> UnderflowResult<'_, K, V> {
     let parent = match node.ascend() {
         Ok(parent) => parent,

library/alloc/src/collections/btree/map/tests.rs

@@ -77,7 +77,8 @@ impl<'a, K: 'a, V: 'a> BTreeMap<K, V> {
                     let min_len = if is_root { 0 } else { node::MIN_LEN };
                     assert!(node.len() >= min_len, "{} < {}", node.len(), min_len);
 
-                    for &key in node.keys() {
+                    for idx in 0..node.len() {
+                        let key = *unsafe { node.key_at(idx) };
                         checker.is_ascending(key);
                     }
                     leaf_length += node.len();
@@ -120,7 +121,13 @@ impl<'a, K: 'a, V: 'a> BTreeMap<K, V> {
                 Position::Leaf(leaf) => {
                     let depth = root_node.height();
                     let indent = "  ".repeat(depth);
-                    result += &format!("\n{}{:?}", indent, leaf.keys())
+                    result += &format!("\n{}", indent);
+                    for idx in 0..leaf.len() {
+                        if idx > 0 {
+                            result += ", ";
+                        }
+                        result += &format!("{:?}", unsafe { leaf.key_at(idx) });
+                    }
                 }
                 Position::Internal(_) => {}
                 Position::InternalKV(kv) => {
@@ -432,7 +439,6 @@ fn test_iter_mut_mutation() {
 }
 
 #[test]
-#[cfg_attr(miri, ignore)] // FIXME: fails in Miri <https://github.com/rust-lang/rust/issues/73915>
 fn test_values_mut() {
     let mut a: BTreeMap<_, _> = (0..MIN_INSERTS_HEIGHT_2).map(|i| (i, i)).collect();
     test_all_refs(&mut 13, a.values_mut());
@@ -455,7 +461,6 @@ fn test_values_mut_mutation() {
 }
 
 #[test]
-#[cfg_attr(miri, ignore)] // FIXME: fails in Miri <https://github.com/rust-lang/rust/issues/73915>
 fn test_iter_entering_root_twice() {
     let mut map: BTreeMap<_, _> = (0..2).map(|i| (i, i)).collect();
     let mut it = map.iter_mut();
@@ -471,7 +476,6 @@ fn test_iter_entering_root_twice() {
 }
 
 #[test]
-#[cfg_attr(miri, ignore)] // FIXME: fails in Miri <https://github.com/rust-lang/rust/issues/73915>
 fn test_iter_descending_to_same_node_twice() {
     let mut map: BTreeMap<_, _> = (0..MIN_INSERTS_HEIGHT_1).map(|i| (i, i)).collect();
     let mut it = map.iter_mut();
@@ -515,7 +519,6 @@ fn test_iter_mixed() {
 }
 
 #[test]
-#[cfg_attr(miri, ignore)] // FIXME: fails in Miri <https://github.com/rust-lang/rust/issues/73915>
 fn test_iter_min_max() {
     let mut a = BTreeMap::new();
     assert_eq!(a.iter().min(), None);

library/alloc/src/collections/btree/navigate.rs

@@ -366,7 +366,6 @@ impl<'a, K, V> Handle<NodeRef<marker::Immut<'a>, K, V, marker::Leaf>, marker::Ed
 impl<'a, K, V> Handle<NodeRef<marker::ValMut<'a>, K, V, marker::Leaf>, marker::Edge> {
     /// Moves the leaf edge handle to the next leaf edge and returns references to the
     /// key and value in between.
-    /// The returned references might be invalidated when the updated handle is used again.
     ///
     /// # Safety
     /// There must be another KV in the direction travelled.
@@ -376,14 +375,12 @@ impl<'a, K, V> Handle<NodeRef<marker::ValMut<'a>, K, V, marker::Leaf>, marker::E
             let kv = unsafe { unwrap_unchecked(kv.ok()) };
             (unsafe { ptr::read(&kv) }.next_leaf_edge(), kv)
         });
-        // Doing the descend (and perhaps another move) invalidates the references
-        // returned by `into_kv_valmut`, so we have to do this last.
+        // Doing this last is faster, according to benchmarks.
         kv.into_kv_valmut()
     }
 
     /// Moves the leaf edge handle to the previous leaf and returns references to the
     /// key and value in between.
-    /// The returned references might be invalidated when the updated handle is used again.
     ///
     /// # Safety
     /// There must be another KV in the direction travelled.
@@ -393,8 +390,7 @@ impl<'a, K, V> Handle<NodeRef<marker::ValMut<'a>, K, V, marker::Leaf>, marker::E
             let kv = unsafe { unwrap_unchecked(kv.ok()) };
             (unsafe { ptr::read(&kv) }.next_back_leaf_edge(), kv)
         });
-        // Doing the descend (and perhaps another move) invalidates the references
-        // returned by `into_kv_valmut`, so we have to do this last.
+        // Doing this last is faster, according to benchmarks.
         kv.into_kv_valmut()
     }
 }