Skip to content

Commit

Permalink
Rollup merge of #81610 - ssomers:btree_emphasize_ord_bound, r=dtolnay
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@m-ou-se
m-ou-se authored Feb 5, 2021
2 parents 43b3adb + 1020784 commit 78be1aa
Show file tree
Hide file tree
Showing 4 changed files with 191 additions and 54 deletions.
96 changes: 70 additions & 26 deletions library/alloc/src/collections/btree/map.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 }
}

Expand All @@ -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();
}

Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)
}
Expand All @@ -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()?;
Expand All @@ -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())
}

Expand All @@ -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)
}
Expand All @@ -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()?;
Expand All @@ -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())
}

Expand All @@ -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()
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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() }
}

Expand All @@ -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() }
}
}
Expand Down Expand Up @@ -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;
Expand Down
28 changes: 28 additions & 0 deletions library/alloc/src/collections/btree/map/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading

0 comments on commit 78be1aa

Please sign in to comment.