Skip to content

Commit

Permalink
Rollup merge of rust-lang#78437 - ssomers:btree_no_ord_at_node_level,…
Browse files Browse the repository at this point in the history
… 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```````
  • Loading branch information
Dylan-DPC authored Nov 8, 2020
2 parents 7f28849 + e099138 commit f9c2a5d
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 41 deletions.
26 changes: 22 additions & 4 deletions library/alloc/src/collections/btree/map/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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() {
Expand Down
37 changes: 0 additions & 37 deletions library/alloc/src/collections/btree/node/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit f9c2a5d

Please sign in to comment.