From 8d1c3c116b6a3e26ab23f662fbf03d645068b228 Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Fri, 14 Aug 2020 14:50:30 +0200 Subject: [PATCH 1/2] BTreeMap: refactor splitpoint and move testing over to unit test --- library/alloc/src/collections/btree/node.rs | 42 +++++-------------- .../alloc/src/collections/btree/node/tests.rs | 25 +++++++++++ library/alloc/src/lib.rs | 1 + 3 files changed, 37 insertions(+), 31 deletions(-) create mode 100644 library/alloc/src/collections/btree/node/tests.rs diff --git a/library/alloc/src/collections/btree/node.rs b/library/alloc/src/collections/btree/node.rs index c0b75fd5eac..88d0ed9eb0c 100644 --- a/library/alloc/src/collections/btree/node.rs +++ b/library/alloc/src/collections/btree/node.rs @@ -43,6 +43,9 @@ use crate::boxed::Box; const B: usize = 6; pub const MIN_LEN: usize = B - 1; pub const CAPACITY: usize = 2 * B - 1; +const KV_IDX_CENTER: usize = B - 1; +const EDGE_IDX_LEFT_OF_CENTER: usize = B - 1; +const EDGE_IDX_RIGHT_OF_CENTER: usize = B; /// The underlying representation of leaf nodes. #[repr(C)] @@ -834,38 +837,12 @@ enum InsertionPlace { fn splitpoint(edge_idx: usize) -> (usize, InsertionPlace) { debug_assert!(edge_idx <= CAPACITY); // Rust issue #74834 tries to explain these symmetric rules. - let middle_kv_idx; - let insertion; - if edge_idx <= B - 2 { - middle_kv_idx = B - 2; - insertion = InsertionPlace::Left(edge_idx); - } else if edge_idx == B - 1 { - middle_kv_idx = B - 1; - insertion = InsertionPlace::Left(edge_idx); - } else if edge_idx == B { - middle_kv_idx = B - 1; - insertion = InsertionPlace::Right(0); - } else { - middle_kv_idx = B; - let new_edge_idx = edge_idx - (B + 1); - insertion = InsertionPlace::Right(new_edge_idx); + match edge_idx { + 0..EDGE_IDX_LEFT_OF_CENTER => (KV_IDX_CENTER - 1, InsertionPlace::Left(edge_idx)), + EDGE_IDX_LEFT_OF_CENTER => (KV_IDX_CENTER, InsertionPlace::Left(edge_idx)), + EDGE_IDX_RIGHT_OF_CENTER => (KV_IDX_CENTER, InsertionPlace::Right(0)), + _ => (KV_IDX_CENTER + 1, InsertionPlace::Right(edge_idx - (KV_IDX_CENTER + 1 + 1))), } - let mut left_len = middle_kv_idx; - let mut right_len = CAPACITY - middle_kv_idx - 1; - match insertion { - InsertionPlace::Left(edge_idx) => { - debug_assert!(edge_idx <= left_len); - left_len += 1; - } - InsertionPlace::Right(edge_idx) => { - debug_assert!(edge_idx <= right_len); - right_len += 1; - } - } - debug_assert!(left_len >= MIN_LEN); - debug_assert!(right_len >= MIN_LEN); - debug_assert!(left_len + right_len == CAPACITY); - (middle_kv_idx, insertion) } impl<'a, K, V, NodeType> Handle, K, V, NodeType>, marker::Edge> { @@ -1590,3 +1567,6 @@ unsafe fn slice_remove(slice: &mut [T], idx: usize) -> T { ret } } + +#[cfg(test)] +mod tests; diff --git a/library/alloc/src/collections/btree/node/tests.rs b/library/alloc/src/collections/btree/node/tests.rs new file mode 100644 index 00000000000..e2416974ddc --- /dev/null +++ b/library/alloc/src/collections/btree/node/tests.rs @@ -0,0 +1,25 @@ +use super::*; + +#[test] +fn test_splitpoint() { + for idx in 0..=CAPACITY { + let (middle_kv_idx, insertion) = splitpoint(idx); + + // Simulate performing the split: + let mut left_len = middle_kv_idx; + let mut right_len = CAPACITY - middle_kv_idx - 1; + match insertion { + InsertionPlace::Left(edge_idx) => { + assert!(edge_idx <= left_len); + left_len += 1; + } + InsertionPlace::Right(edge_idx) => { + assert!(edge_idx <= right_len); + right_len += 1; + } + } + assert!(left_len >= MIN_LEN); + assert!(right_len >= MIN_LEN); + assert!(left_len + right_len == CAPACITY); + } +} diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 9ac23886d4e..75a2c6be41c 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -93,6 +93,7 @@ #![feature(container_error_extra)] #![feature(dropck_eyepatch)] #![feature(exact_size_is_empty)] +#![feature(exclusive_range_pattern)] #![feature(extend_one)] #![feature(fmt_internals)] #![feature(fn_traits)] From ff45df2acf8cb31e50930840d48d5285b67b23ec Mon Sep 17 00:00:00 2001 From: Stein Somers Date: Fri, 14 Aug 2020 16:38:53 +0200 Subject: [PATCH 2/2] Move btree unit test to their native, privileged location --- library/alloc/src/collections/btree/map.rs | 3 +++ .../collections/btree/map/tests.rs} | 14 ++++++---- library/alloc/src/collections/btree/mod.rs | 27 +++++++++++++++++++ library/alloc/src/collections/btree/set.rs | 3 +++ .../collections/btree/set/tests.rs} | 23 +++------------- library/alloc/src/lib.rs | 3 +++ library/alloc/tests/btree/mod.rs | 27 ------------------- library/alloc/tests/btree_set_hash.rs | 19 +++++++++++++ library/alloc/tests/lib.rs | 5 +--- 9 files changed, 68 insertions(+), 56 deletions(-) rename library/alloc/{tests/btree/map.rs => src/collections/btree/map/tests.rs} (99%) rename library/alloc/{tests/btree/set.rs => src/collections/btree/set/tests.rs} (98%) delete mode 100644 library/alloc/tests/btree/mod.rs create mode 100644 library/alloc/tests/btree_set_hash.rs diff --git a/library/alloc/src/collections/btree/map.rs b/library/alloc/src/collections/btree/map.rs index e7d243bfcb0..567bed2fd67 100644 --- a/library/alloc/src/collections/btree/map.rs +++ b/library/alloc/src/collections/btree/map.rs @@ -3024,3 +3024,6 @@ impl> Iterator for MergeIter { } } } + +#[cfg(test)] +mod tests; diff --git a/library/alloc/tests/btree/map.rs b/library/alloc/src/collections/btree/map/tests.rs similarity index 99% rename from library/alloc/tests/btree/map.rs rename to library/alloc/src/collections/btree/map/tests.rs index 5777bd60907..910e7043092 100644 --- a/library/alloc/tests/btree/map.rs +++ b/library/alloc/src/collections/btree/map/tests.rs @@ -1,16 +1,20 @@ -use std::collections::btree_map::Entry::{Occupied, Vacant}; -use std::collections::BTreeMap; +use crate::boxed::Box; +use crate::collections::btree_map::Entry::{Occupied, Vacant}; +use crate::collections::BTreeMap; +use crate::fmt::Debug; +use crate::rc::Rc; +use crate::string::String; +use crate::string::ToString; +use crate::vec::Vec; use std::convert::TryFrom; -use std::fmt::Debug; use std::iter::FromIterator; use std::mem; use std::ops::Bound::{self, Excluded, Included, Unbounded}; use std::ops::RangeBounds; use std::panic::{catch_unwind, AssertUnwindSafe}; -use std::rc::Rc; use std::sync::atomic::{AtomicUsize, Ordering}; -use super::DeterministicRng; +use super::super::DeterministicRng; // Value of node::CAPACITY, thus capacity of a tree with a single level, // i.e. a tree who's root is a leaf node at height 0. diff --git a/library/alloc/src/collections/btree/mod.rs b/library/alloc/src/collections/btree/mod.rs index 543ff41a4d4..6c8a588eb58 100644 --- a/library/alloc/src/collections/btree/mod.rs +++ b/library/alloc/src/collections/btree/mod.rs @@ -25,3 +25,30 @@ pub unsafe fn unwrap_unchecked(val: Option) -> T { } }) } + +#[cfg(test)] +/// XorShiftRng +struct DeterministicRng { + x: u32, + y: u32, + z: u32, + w: u32, +} + +#[cfg(test)] +impl DeterministicRng { + fn new() -> Self { + DeterministicRng { x: 0x193a6754, y: 0xa8a7d469, z: 0x97830e05, w: 0x113ba7bb } + } + + fn next(&mut self) -> u32 { + let x = self.x; + let t = x ^ (x << 11); + self.x = self.y; + self.y = self.z; + self.z = self.w; + let w_ = self.w; + self.w = w_ ^ (w_ >> 19) ^ (t ^ (t >> 8)); + self.w + } +} diff --git a/library/alloc/src/collections/btree/set.rs b/library/alloc/src/collections/btree/set.rs index 35f4ef1d9b4..a559e87e4e2 100644 --- a/library/alloc/src/collections/btree/set.rs +++ b/library/alloc/src/collections/btree/set.rs @@ -1572,3 +1572,6 @@ impl<'a, T: Ord> Iterator for Union<'a, T> { #[stable(feature = "fused", since = "1.26.0")] impl FusedIterator for Union<'_, T> {} + +#[cfg(test)] +mod tests; diff --git a/library/alloc/tests/btree/set.rs b/library/alloc/src/collections/btree/set/tests.rs similarity index 98% rename from library/alloc/tests/btree/set.rs rename to library/alloc/src/collections/btree/set/tests.rs index b6c34b7c6c3..f4e957e22fe 100644 --- a/library/alloc/tests/btree/set.rs +++ b/library/alloc/src/collections/btree/set/tests.rs @@ -1,9 +1,10 @@ -use std::collections::BTreeSet; +use crate::collections::BTreeSet; +use crate::vec::Vec; use std::iter::FromIterator; use std::panic::{catch_unwind, AssertUnwindSafe}; use std::sync::atomic::{AtomicU32, Ordering}; -use super::DeterministicRng; +use super::super::DeterministicRng; #[test] fn test_clone_eq() { @@ -15,24 +16,6 @@ fn test_clone_eq() { assert_eq!(m.clone(), m); } -#[test] -fn test_hash() { - use crate::hash; - - let mut x = BTreeSet::new(); - let mut y = BTreeSet::new(); - - x.insert(1); - x.insert(2); - x.insert(3); - - y.insert(3); - y.insert(2); - y.insert(1); - - assert_eq!(hash(&x), hash(&y)); -} - #[test] fn test_iter_min_max() { let mut a = BTreeSet::new(); diff --git a/library/alloc/src/lib.rs b/library/alloc/src/lib.rs index 75a2c6be41c..2d25941a524 100644 --- a/library/alloc/src/lib.rs +++ b/library/alloc/src/lib.rs @@ -80,6 +80,7 @@ #![feature(arbitrary_self_types)] #![feature(box_patterns)] #![feature(box_syntax)] +#![feature(btree_drain_filter)] #![feature(cfg_sanitize)] #![feature(cfg_target_has_atomic)] #![feature(coerce_unsized)] @@ -102,6 +103,8 @@ #![feature(lang_items)] #![feature(layout_for_ptr)] #![feature(libc)] +#![feature(map_first_last)] +#![feature(map_into_keys_values)] #![feature(negative_impls)] #![feature(new_uninit)] #![feature(nll)] diff --git a/library/alloc/tests/btree/mod.rs b/library/alloc/tests/btree/mod.rs deleted file mode 100644 index 1d08ae13e05..00000000000 --- a/library/alloc/tests/btree/mod.rs +++ /dev/null @@ -1,27 +0,0 @@ -mod map; -mod set; - -/// XorShiftRng -struct DeterministicRng { - x: u32, - y: u32, - z: u32, - w: u32, -} - -impl DeterministicRng { - fn new() -> Self { - DeterministicRng { x: 0x193a6754, y: 0xa8a7d469, z: 0x97830e05, w: 0x113ba7bb } - } - - fn next(&mut self) -> u32 { - let x = self.x; - let t = x ^ (x << 11); - self.x = self.y; - self.y = self.z; - self.z = self.w; - let w_ = self.w; - self.w = w_ ^ (w_ >> 19) ^ (t ^ (t >> 8)); - self.w - } -} diff --git a/library/alloc/tests/btree_set_hash.rs b/library/alloc/tests/btree_set_hash.rs new file mode 100644 index 00000000000..e06a95ded94 --- /dev/null +++ b/library/alloc/tests/btree_set_hash.rs @@ -0,0 +1,19 @@ +use std::collections::BTreeSet; + +#[test] +fn test_hash() { + use crate::hash; + + let mut x = BTreeSet::new(); + let mut y = BTreeSet::new(); + + x.insert(1); + x.insert(2); + x.insert(3); + + y.insert(3); + y.insert(2); + y.insert(1); + + assert_eq!(hash(&x), hash(&y)); +} diff --git a/library/alloc/tests/lib.rs b/library/alloc/tests/lib.rs index 3aacd4a687e..f2ba1ab6481 100644 --- a/library/alloc/tests/lib.rs +++ b/library/alloc/tests/lib.rs @@ -1,10 +1,7 @@ #![feature(allocator_api)] #![feature(box_syntax)] -#![feature(btree_drain_filter)] #![feature(drain_filter)] #![feature(exact_size_is_empty)] -#![feature(map_first_last)] -#![feature(map_into_keys_values)] #![feature(new_uninit)] #![feature(pattern)] #![feature(str_split_once)] @@ -25,7 +22,7 @@ mod arc; mod binary_heap; mod borrow; mod boxed; -mod btree; +mod btree_set_hash; mod cow_str; mod fmt; mod heap;