Rollup merge of #81610 - ssomers:btree_emphasize_ord_bound, r=dtolnay

BTreeMap: make Ord bound explicit, compile-test its absence

Most `BTreeMap` and `BTreeSet` members are subject to an `Ord` bound but a fair number of methods are not. To better convey and perhaps later tune the `Ord` bound, make it stand out in individual `where` clauses, instead of once far away at the beginning of an `impl` block. This PR does not introduce or remove any bounds.

Also adds compilation test cases checking that the bound doesn't creep in unintended on the historically unbounded methods.
This commit is contained in:
Mara Bos 2021-02-06 00:14:11 +01:00 committed by GitHub
commit 78be1aa226
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 191 additions and 54 deletions

View File

@ -460,7 +460,7 @@ impl<K: fmt::Debug, V: fmt::Debug> fmt::Debug for RangeMut<'_, K, V> {
}
}
impl<K: Ord, V> BTreeMap<K, V> {
impl<K, V> BTreeMap<K, V> {
/// Makes a new, empty `BTreeMap`.
///
/// Does not allocate anything on its own.
@ -479,7 +479,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_btree_new", issue = "71835")]
pub const fn new() -> BTreeMap<K, V> {
pub const fn new() -> BTreeMap<K, V>
where
K: Ord,
{
BTreeMap { root: None, length: 0 }
}
@ -498,7 +501,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// assert!(a.is_empty());
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn clear(&mut self) {
pub fn clear(&mut self)
where
K: Ord,
{
*self = BTreeMap::new();
}
@ -522,7 +528,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
#[stable(feature = "rust1", since = "1.0.0")]
pub fn get<Q: ?Sized>(&self, key: &Q) -> Option<&V>
where
K: Borrow<Q>,
K: Borrow<Q> + Ord,
Q: Ord,
{
let root_node = self.root.as_ref()?.reborrow();
@ -550,7 +556,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
#[stable(feature = "map_get_key_value", since = "1.40.0")]
pub fn get_key_value<Q: ?Sized>(&self, k: &Q) -> Option<(&K, &V)>
where
K: Borrow<Q>,
K: Borrow<Q> + Ord,
Q: Ord,
{
let root_node = self.root.as_ref()?.reborrow();
@ -578,7 +584,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// assert_eq!(map.first_key_value(), Some((&1, &"b")));
/// ```
#[unstable(feature = "map_first_last", issue = "62924")]
pub fn first_key_value(&self) -> Option<(&K, &V)> {
pub fn first_key_value(&self) -> Option<(&K, &V)>
where
K: Ord,
{
let root_node = self.root.as_ref()?.reborrow();
root_node.first_leaf_edge().right_kv().ok().map(Handle::into_kv)
}
@ -604,7 +613,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// assert_eq!(*map.get(&2).unwrap(), "b");
/// ```
#[unstable(feature = "map_first_last", issue = "62924")]
pub fn first_entry(&mut self) -> Option<OccupiedEntry<'_, K, V>> {
pub fn first_entry(&mut self) -> Option<OccupiedEntry<'_, K, V>>
where
K: Ord,
{
let (map, dormant_map) = DormantMutRef::new(self);
let root_node = map.root.as_mut()?.borrow_mut();
let kv = root_node.first_leaf_edge().right_kv().ok()?;
@ -631,7 +643,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// assert!(map.is_empty());
/// ```
#[unstable(feature = "map_first_last", issue = "62924")]
pub fn pop_first(&mut self) -> Option<(K, V)> {
pub fn pop_first(&mut self) -> Option<(K, V)>
where
K: Ord,
{
self.first_entry().map(|entry| entry.remove_entry())
}
@ -652,7 +667,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// assert_eq!(map.last_key_value(), Some((&2, &"a")));
/// ```
#[unstable(feature = "map_first_last", issue = "62924")]
pub fn last_key_value(&self) -> Option<(&K, &V)> {
pub fn last_key_value(&self) -> Option<(&K, &V)>
where
K: Ord,
{
let root_node = self.root.as_ref()?.reborrow();
root_node.last_leaf_edge().left_kv().ok().map(Handle::into_kv)
}
@ -678,7 +696,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// assert_eq!(*map.get(&2).unwrap(), "last");
/// ```
#[unstable(feature = "map_first_last", issue = "62924")]
pub fn last_entry(&mut self) -> Option<OccupiedEntry<'_, K, V>> {
pub fn last_entry(&mut self) -> Option<OccupiedEntry<'_, K, V>>
where
K: Ord,
{
let (map, dormant_map) = DormantMutRef::new(self);
let root_node = map.root.as_mut()?.borrow_mut();
let kv = root_node.last_leaf_edge().left_kv().ok()?;
@ -705,7 +726,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// assert!(map.is_empty());
/// ```
#[unstable(feature = "map_first_last", issue = "62924")]
pub fn pop_last(&mut self) -> Option<(K, V)> {
pub fn pop_last(&mut self) -> Option<(K, V)>
where
K: Ord,
{
self.last_entry().map(|entry| entry.remove_entry())
}
@ -729,7 +753,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
#[stable(feature = "rust1", since = "1.0.0")]
pub fn contains_key<Q: ?Sized>(&self, key: &Q) -> bool
where
K: Borrow<Q>,
K: Borrow<Q> + Ord,
Q: Ord,
{
self.get(key).is_some()
@ -758,7 +782,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
#[stable(feature = "rust1", since = "1.0.0")]
pub fn get_mut<Q: ?Sized>(&mut self, key: &Q) -> Option<&mut V>
where
K: Borrow<Q>,
K: Borrow<Q> + Ord,
Q: Ord,
{
let root_node = self.root.as_mut()?.borrow_mut();
@ -795,7 +819,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// assert_eq!(map[&37], "c");
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn insert(&mut self, key: K, value: V) -> Option<V> {
pub fn insert(&mut self, key: K, value: V) -> Option<V>
where
K: Ord,
{
match self.entry(key) {
Occupied(mut entry) => Some(entry.insert(value)),
Vacant(entry) => {
@ -827,7 +854,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
#[stable(feature = "rust1", since = "1.0.0")]
pub fn remove<Q: ?Sized>(&mut self, key: &Q) -> Option<V>
where
K: Borrow<Q>,
K: Borrow<Q> + Ord,
Q: Ord,
{
self.remove_entry(key).map(|(_, v)| v)
@ -854,7 +881,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
#[stable(feature = "btreemap_remove_entry", since = "1.45.0")]
pub fn remove_entry<Q: ?Sized>(&mut self, key: &Q) -> Option<(K, V)>
where
K: Borrow<Q>,
K: Borrow<Q> + Ord,
Q: Ord,
{
let (map, dormant_map) = DormantMutRef::new(self);
@ -886,6 +913,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
#[unstable(feature = "btree_retain", issue = "79025")]
pub fn retain<F>(&mut self, mut f: F)
where
K: Ord,
F: FnMut(&K, &mut V) -> bool,
{
self.drain_filter(|k, v| !f(k, v));
@ -920,7 +948,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// assert_eq!(a[&5], "f");
/// ```
#[stable(feature = "btree_append", since = "1.11.0")]
pub fn append(&mut self, other: &mut Self) {
pub fn append(&mut self, other: &mut Self)
where
K: Ord,
{
// Do we have to append anything at all?
if other.is_empty() {
return;
@ -971,7 +1002,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
pub fn range<T: ?Sized, R>(&self, range: R) -> Range<'_, K, V>
where
T: Ord,
K: Borrow<T>,
K: Borrow<T> + Ord,
R: RangeBounds<T>,
{
if let Some(root) = &self.root {
@ -1017,7 +1048,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
pub fn range_mut<T: ?Sized, R>(&mut self, range: R) -> RangeMut<'_, K, V>
where
T: Ord,
K: Borrow<T>,
K: Borrow<T> + Ord,
R: RangeBounds<T>,
{
if let Some(root) = &mut self.root {
@ -1048,7 +1079,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// assert_eq!(count["a"], 3);
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn entry(&mut self, key: K) -> Entry<'_, K, V> {
pub fn entry(&mut self, key: K) -> Entry<'_, K, V>
where
K: Ord,
{
// FIXME(@porglezomp) Avoid allocating if we don't insert
let (map, dormant_map) = DormantMutRef::new(self);
let root_node = Self::ensure_is_owned(&mut map.root).borrow_mut();
@ -1092,7 +1126,7 @@ impl<K: Ord, V> BTreeMap<K, V> {
#[stable(feature = "btree_split_off", since = "1.11.0")]
pub fn split_off<Q: ?Sized + Ord>(&mut self, key: &Q) -> Self
where
K: Borrow<Q>,
K: Borrow<Q> + Ord,
{
if self.is_empty() {
return Self::new();
@ -1150,12 +1184,16 @@ impl<K: Ord, V> BTreeMap<K, V> {
#[unstable(feature = "btree_drain_filter", issue = "70530")]
pub fn drain_filter<F>(&mut self, pred: F) -> DrainFilter<'_, K, V, F>
where
K: Ord,
F: FnMut(&K, &mut V) -> bool,
{
DrainFilter { pred, inner: self.drain_filter_inner() }
}
pub(super) fn drain_filter_inner(&mut self) -> DrainFilterInner<'_, K, V> {
pub(super) fn drain_filter_inner(&mut self) -> DrainFilterInner<'_, K, V>
where
K: Ord,
{
if let Some(root) = self.root.as_mut() {
let (root, dormant_root) = DormantMutRef::new(root);
let front = root.borrow_mut().first_leaf_edge();
@ -1188,7 +1226,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// ```
#[inline]
#[unstable(feature = "map_into_keys_values", issue = "75294")]
pub fn into_keys(self) -> IntoKeys<K, V> {
pub fn into_keys(self) -> IntoKeys<K, V>
where
K: Ord,
{
IntoKeys { inner: self.into_iter() }
}
@ -1211,7 +1252,10 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// ```
#[inline]
#[unstable(feature = "map_into_keys_values", issue = "75294")]
pub fn into_values(self) -> IntoValues<K, V> {
pub fn into_values(self) -> IntoValues<K, V>
where
K: Ord,
{
IntoValues { inner: self.into_iter() }
}
}
@ -1968,9 +2012,9 @@ impl<K: Debug, V: Debug> Debug for BTreeMap<K, V> {
}
#[stable(feature = "rust1", since = "1.0.0")]
impl<K: Ord, Q: ?Sized, V> Index<&Q> for BTreeMap<K, V>
impl<K, Q: ?Sized, V> Index<&Q> for BTreeMap<K, V>
where
K: Borrow<Q>,
K: Borrow<Q> + Ord,
Q: Ord,
{
type Output = V;

View File

@ -1706,6 +1706,34 @@ fn test_send() {
}
}
#[allow(dead_code)]
fn test_ord_absence() {
fn map<K>(mut map: BTreeMap<K, ()>) {
map.is_empty();
map.len();
map.iter();
map.iter_mut();
map.keys();
map.values();
map.values_mut();
map.into_iter();
}
fn map_debug<K: Debug>(mut map: BTreeMap<K, ()>) {
format!("{:?}", map);
format!("{:?}", map.iter());
format!("{:?}", map.iter_mut());
format!("{:?}", map.keys());
format!("{:?}", map.values());
format!("{:?}", map.values_mut());
format!("{:?}", map.into_iter());
}
fn map_clone<K: Clone>(mut map: BTreeMap<K, ()>) {
map.clone_from(&map.clone());
}
}
#[allow(dead_code)]
fn test_const() {
const MAP: &'static BTreeMap<(), ()> = &BTreeMap::new();

View File

@ -222,7 +222,7 @@ impl<T: fmt::Debug> fmt::Debug for Union<'_, T> {
// and it's a power of two to make that division cheap.
const ITER_PERFORMANCE_TIPPING_SIZE_DIFF: usize = 16;
impl<T: Ord> BTreeSet<T> {
impl<T> BTreeSet<T> {
/// Makes a new, empty `BTreeSet`.
///
/// Does not allocate anything on its own.
@ -237,7 +237,10 @@ impl<T: Ord> BTreeSet<T> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_btree_new", issue = "71835")]
pub const fn new() -> BTreeSet<T> {
pub const fn new() -> BTreeSet<T>
where
T: Ord,
{
BTreeSet { map: BTreeMap::new() }
}
@ -267,7 +270,7 @@ impl<T: Ord> BTreeSet<T> {
pub fn range<K: ?Sized, R>(&self, range: R) -> Range<'_, T>
where
K: Ord,
T: Borrow<K>,
T: Borrow<K> + Ord,
R: RangeBounds<K>,
{
Range { iter: self.map.range(range) }
@ -294,7 +297,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert_eq!(diff, [1]);
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn difference<'a>(&'a self, other: &'a BTreeSet<T>) -> Difference<'a, T> {
pub fn difference<'a>(&'a self, other: &'a BTreeSet<T>) -> Difference<'a, T>
where
T: Ord,
{
let (self_min, self_max) =
if let (Some(self_min), Some(self_max)) = (self.first(), self.last()) {
(self_min, self_max)
@ -352,10 +358,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert_eq!(sym_diff, [1, 3]);
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn symmetric_difference<'a>(
&'a self,
other: &'a BTreeSet<T>,
) -> SymmetricDifference<'a, T> {
pub fn symmetric_difference<'a>(&'a self, other: &'a BTreeSet<T>) -> SymmetricDifference<'a, T>
where
T: Ord,
{
SymmetricDifference(MergeIterInner::new(self.iter(), other.iter()))
}
@ -380,7 +386,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert_eq!(intersection, [2]);
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn intersection<'a>(&'a self, other: &'a BTreeSet<T>) -> Intersection<'a, T> {
pub fn intersection<'a>(&'a self, other: &'a BTreeSet<T>) -> Intersection<'a, T>
where
T: Ord,
{
let (self_min, self_max) =
if let (Some(self_min), Some(self_max)) = (self.first(), self.last()) {
(self_min, self_max)
@ -428,7 +437,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert_eq!(union, [1, 2]);
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn union<'a>(&'a self, other: &'a BTreeSet<T>) -> Union<'a, T> {
pub fn union<'a>(&'a self, other: &'a BTreeSet<T>) -> Union<'a, T>
where
T: Ord,
{
Union(MergeIterInner::new(self.iter(), other.iter()))
}
@ -445,7 +457,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert!(v.is_empty());
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn clear(&mut self) {
pub fn clear(&mut self)
where
T: Ord,
{
self.map.clear()
}
@ -467,7 +482,7 @@ impl<T: Ord> BTreeSet<T> {
#[stable(feature = "rust1", since = "1.0.0")]
pub fn contains<Q: ?Sized>(&self, value: &Q) -> bool
where
T: Borrow<Q>,
T: Borrow<Q> + Ord,
Q: Ord,
{
self.map.contains_key(value)
@ -491,7 +506,7 @@ impl<T: Ord> BTreeSet<T> {
#[stable(feature = "set_recovery", since = "1.9.0")]
pub fn get<Q: ?Sized>(&self, value: &Q) -> Option<&T>
where
T: Borrow<Q>,
T: Borrow<Q> + Ord,
Q: Ord,
{
Recover::get(&self.map, value)
@ -515,7 +530,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert_eq!(a.is_disjoint(&b), false);
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn is_disjoint(&self, other: &BTreeSet<T>) -> bool {
pub fn is_disjoint(&self, other: &BTreeSet<T>) -> bool
where
T: Ord,
{
self.intersection(other).next().is_none()
}
@ -537,7 +555,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert_eq!(set.is_subset(&sup), false);
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn is_subset(&self, other: &BTreeSet<T>) -> bool {
pub fn is_subset(&self, other: &BTreeSet<T>) -> bool
where
T: Ord,
{
// Same result as self.difference(other).next().is_none()
// but the code below is faster (hugely in some cases).
if self.len() > other.len() {
@ -613,7 +634,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert_eq!(set.is_superset(&sub), true);
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn is_superset(&self, other: &BTreeSet<T>) -> bool {
pub fn is_superset(&self, other: &BTreeSet<T>) -> bool
where
T: Ord,
{
other.is_subset(self)
}
@ -636,7 +660,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert_eq!(map.first(), Some(&1));
/// ```
#[unstable(feature = "map_first_last", issue = "62924")]
pub fn first(&self) -> Option<&T> {
pub fn first(&self) -> Option<&T>
where
T: Ord,
{
self.map.first_key_value().map(|(k, _)| k)
}
@ -659,7 +686,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert_eq!(map.last(), Some(&2));
/// ```
#[unstable(feature = "map_first_last", issue = "62924")]
pub fn last(&self) -> Option<&T> {
pub fn last(&self) -> Option<&T>
where
T: Ord,
{
self.map.last_key_value().map(|(k, _)| k)
}
@ -681,7 +711,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert!(set.is_empty());
/// ```
#[unstable(feature = "map_first_last", issue = "62924")]
pub fn pop_first(&mut self) -> Option<T> {
pub fn pop_first(&mut self) -> Option<T>
where
T: Ord,
{
self.map.pop_first().map(|kv| kv.0)
}
@ -703,7 +736,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert!(set.is_empty());
/// ```
#[unstable(feature = "map_first_last", issue = "62924")]
pub fn pop_last(&mut self) -> Option<T> {
pub fn pop_last(&mut self) -> Option<T>
where
T: Ord,
{
self.map.pop_last().map(|kv| kv.0)
}
@ -728,7 +764,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert_eq!(set.len(), 1);
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn insert(&mut self, value: T) -> bool {
pub fn insert(&mut self, value: T) -> bool
where
T: Ord,
{
self.map.insert(value, ()).is_none()
}
@ -748,7 +787,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert_eq!(set.get(&[][..]).unwrap().capacity(), 10);
/// ```
#[stable(feature = "set_recovery", since = "1.9.0")]
pub fn replace(&mut self, value: T) -> Option<T> {
pub fn replace(&mut self, value: T) -> Option<T>
where
T: Ord,
{
Recover::replace(&mut self.map, value)
}
@ -774,7 +816,7 @@ impl<T: Ord> BTreeSet<T> {
#[stable(feature = "rust1", since = "1.0.0")]
pub fn remove<Q: ?Sized>(&mut self, value: &Q) -> bool
where
T: Borrow<Q>,
T: Borrow<Q> + Ord,
Q: Ord,
{
self.map.remove(value).is_some()
@ -798,7 +840,7 @@ impl<T: Ord> BTreeSet<T> {
#[stable(feature = "set_recovery", since = "1.9.0")]
pub fn take<Q: ?Sized>(&mut self, value: &Q) -> Option<T>
where
T: Borrow<Q>,
T: Borrow<Q> + Ord,
Q: Ord,
{
Recover::take(&mut self.map, value)
@ -823,6 +865,7 @@ impl<T: Ord> BTreeSet<T> {
#[unstable(feature = "btree_retain", issue = "79025")]
pub fn retain<F>(&mut self, mut f: F)
where
T: Ord,
F: FnMut(&T) -> bool,
{
self.drain_filter(|v| !f(v));
@ -857,7 +900,10 @@ impl<T: Ord> BTreeSet<T> {
/// assert!(a.contains(&5));
/// ```
#[stable(feature = "btree_append", since = "1.11.0")]
pub fn append(&mut self, other: &mut Self) {
pub fn append(&mut self, other: &mut Self)
where
T: Ord,
{
self.map.append(&mut other.map);
}
@ -893,7 +939,7 @@ impl<T: Ord> BTreeSet<T> {
#[stable(feature = "btree_split_off", since = "1.11.0")]
pub fn split_off<Q: ?Sized + Ord>(&mut self, key: &Q) -> Self
where
T: Borrow<Q>,
T: Borrow<Q> + Ord,
{
BTreeSet { map: self.map.split_off(key) }
}
@ -928,13 +974,12 @@ impl<T: Ord> BTreeSet<T> {
#[unstable(feature = "btree_drain_filter", issue = "70530")]
pub fn drain_filter<'a, F>(&'a mut self, pred: F) -> DrainFilter<'a, T, F>
where
T: Ord,
F: 'a + FnMut(&T) -> bool,
{
DrainFilter { pred, inner: self.map.drain_filter_inner() }
}
}
impl<T> BTreeSet<T> {
/// Gets an iterator that visits the values in the `BTreeSet` in ascending order.
///
/// # Examples

View File

@ -639,6 +639,26 @@ fn test_send() {
}
}
#[allow(dead_code)]
fn test_ord_absence() {
fn set<K>(set: BTreeSet<K>) {
set.is_empty();
set.len();
set.iter();
set.into_iter();
}
fn set_debug<K: Debug>(set: BTreeSet<K>) {
format!("{:?}", set);
format!("{:?}", set.iter());
format!("{:?}", set.into_iter());
}
fn set_clone<K: Clone>(mut set: BTreeSet<K>) {
set.clone_from(&set.clone());
}
}
#[test]
fn test_append() {
let mut a = BTreeSet::new();