Rollup merge of rust-lang#78437 - ssomers:btree_no_ord_at_node_level, r=Mark-Simulacrum

BTreeMap: stop mistaking node for an orderly place

A second mistake in rust-lang#77612 was to ignore the node module's rightful comment "this module doesn't care whether the entries are sorted". And there's a much simpler way to visit the keys in order, if you check this separately from a single pass checking everything.

r? ``````@Mark-Simulacrum``````

Author: Dylan-DPC

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

@@ -42,7 +42,7 @@ fn test_all_refs<'a, T: 'a>(dummy: &mut T, iter: impl Iterator<Item = &'a mut T>
     }
 }
 
-impl<'a, K: 'a, V: 'a> BTreeMap<K, V> {
+impl<K, V> BTreeMap<K, V> {
     /// Panics if the map (or the code navigating it) is corrupted.
     fn check(&self)
     where
@@ -54,14 +54,14 @@ impl<'a, K: 'a, V: 'a> BTreeMap<K, V> {
             assert!(root_node.ascend().is_err());
             root_node.assert_back_pointers();
 
-            let counted = root_node.assert_ascending();
-            assert_eq!(self.length, counted);
             assert_eq!(self.length, root_node.calc_length());
 
             root_node.assert_min_len(if root_node.height() > 0 { 1 } else { 0 });
         } else {
             assert_eq!(self.length, 0);
         }
+
+        self.assert_ascending();
     }
 
     /// Returns the height of the root, if any.
@@ -79,10 +79,28 @@ impl<'a, K: 'a, V: 'a> BTreeMap<K, V> {
             String::from("not yet allocated")
         }
     }
+
+    /// Asserts that the keys are in strictly ascending order.
+    fn assert_ascending(&self)
+    where
+        K: Copy + Debug + Ord,
+    {
+        let mut num_seen = 0;
+        let mut keys = self.keys();
+        if let Some(mut previous) = keys.next() {
+            num_seen = 1;
+            for next in keys {
+                assert!(previous < next, "{:?} >= {:?}", previous, next);
+                previous = next;
+                num_seen += 1;
+            }
+        }
+        assert_eq!(num_seen, self.len());
+    }
 }
 
 impl<'a, K: 'a, V: 'a> NodeRef<marker::Immut<'a>, K, V, marker::LeafOrInternal> {
-    pub fn assert_min_len(self, min_len: usize) {
+    fn assert_min_len(self, min_len: usize) {
         assert!(self.len() >= min_len, "{} < {}", self.len(), min_len);
         if let node::ForceResult::Internal(node) = self.force() {
             for idx in 0..=node.len() {

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

@@ -17,43 +17,6 @@ impl<'a, K: 'a, V: 'a> NodeRef<marker::Immut<'a>, K, V, marker::LeafOrInternal>
         }
     }
 
-    /// Asserts that the keys are in strictly ascending order.
-    /// Returns how many keys it encountered.
-    pub fn assert_ascending(self) -> usize
-    where
-        K: Copy + Debug + Ord,
-    {
-        struct SeriesChecker<T> {
-            num_seen: usize,
-            previous: Option<T>,
-        }
-        impl<T: Copy + Debug + Ord> SeriesChecker<T> {
-            fn is_ascending(&mut self, next: T) {
-                if let Some(previous) = self.previous {
-                    assert!(previous < next, "{:?} >= {:?}", previous, next);
-                }
-                self.previous = Some(next);
-                self.num_seen += 1;
-            }
-        }
-
-        let mut checker = SeriesChecker { num_seen: 0, previous: None };
-        self.visit_nodes_in_order(|pos| match pos {
-            navigate::Position::Leaf(node) => {
-                for idx in 0..node.len() {
-                    let key = *unsafe { node.key_at(idx) };
-                    checker.is_ascending(key);
-                }
-            }
-            navigate::Position::InternalKV(kv) => {
-                let key = *kv.into_kv().0;
-                checker.is_ascending(key);
-            }
-            navigate::Position::Internal(_) => {}
-        });
-        checker.num_seen
-    }
-
     pub fn dump_keys(self) -> String
     where
         K: Debug,