Skip to content

Commit

Permalink
Rollup merge of #88040 - nbdd0121:btreemap, r=m-ou-se
Browse files Browse the repository at this point in the history
BTree: remove Ord bound from new

`K: Ord` bound is unnecessary on `BTree{Map,Set}::new` and their `Default` impl. No elements exist so there are nothing to compare anyway, so I don't think "future proof" would be a blocker here. This is analogous to `HashMap::new` not having a `K: Eq + Hash` bound.

#79245 originally does this and for some reason drops the change to `new` and `Default`. I can see why changes to other methods like `entry` or `symmetric_difference` need to be careful but I couldn't find out any reason not to do it on `new`.

Removing the bound also makes the stabilisation of `const fn new` not depending on const trait bounds.

cc `@steffahn` who suggests me to make this PR.

r? `@dtolnay`
  • Loading branch information
m-ou-se authored Sep 1, 2021
2 parents dcefd68 + f33f266 commit 5878780
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 44 deletions.
13 changes: 4 additions & 9 deletions library/alloc/src/collections/btree/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,7 @@ impl<K: Clone, V: Clone> Clone for BTreeMap<K, V> {
}

if self.is_empty() {
// Ideally we'd call `BTreeMap::new` here, but that has the `K:
// Ord` constraint, which this method lacks.
BTreeMap { root: None, length: 0 }
BTreeMap::new()
} else {
clone_subtree(self.root.as_ref().unwrap().reborrow()) // unwrap succeeds because not empty
}
Expand Down Expand Up @@ -499,10 +497,7 @@ impl<K, 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>
where
K: Ord,
{
pub const fn new() -> BTreeMap<K, V> {
BTreeMap { root: None, length: 0 }
}

Expand All @@ -522,7 +517,7 @@ impl<K, V> BTreeMap<K, V> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
pub fn clear(&mut self) {
*self = BTreeMap { root: None, length: 0 };
*self = BTreeMap::new();
}

/// Returns a reference to the value corresponding to the key.
Expand Down Expand Up @@ -1957,7 +1952,7 @@ impl<K: Hash, V: Hash> Hash for BTreeMap<K, V> {
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<K: Ord, V> Default for BTreeMap<K, V> {
impl<K, V> Default for BTreeMap<K, V> {
/// Creates an empty `BTreeMap`.
fn default() -> BTreeMap<K, V> {
BTreeMap::new()
Expand Down
8 changes: 7 additions & 1 deletion library/alloc/src/collections/btree/map/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1745,7 +1745,7 @@ fn test_send() {
}
}

#[allow(dead_code)]
#[test]
fn test_ord_absence() {
fn map<K>(mut map: BTreeMap<K, ()>) {
map.is_empty();
Expand Down Expand Up @@ -1784,6 +1784,12 @@ fn test_ord_absence() {
fn map_clone<K: Clone>(mut map: BTreeMap<K, ()>) {
map.clone_from(&map.clone());
}

#[derive(Debug, Clone)]
struct NonOrd;
map(BTreeMap::<NonOrd, _>::new());
map_debug(BTreeMap::<NonOrd, _>::new());
map_clone(BTreeMap::<NonOrd, _>::default());
}

#[test]
Expand Down
7 changes: 2 additions & 5 deletions library/alloc/src/collections/btree/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,7 @@ impl<T> BTreeSet<T> {
/// ```
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_btree_new", issue = "71835")]
pub const fn new() -> BTreeSet<T>
where
T: Ord,
{
pub const fn new() -> BTreeSet<T> {
BTreeSet { map: BTreeMap::new() }
}

Expand Down Expand Up @@ -1192,7 +1189,7 @@ impl<'a, T: 'a + Ord + Copy> Extend<&'a T> for BTreeSet<T> {
}

#[stable(feature = "rust1", since = "1.0.0")]
impl<T: Ord> Default for BTreeSet<T> {
impl<T> Default for BTreeSet<T> {
/// Creates an empty `BTreeSet`.
fn default() -> BTreeSet<T> {
BTreeSet::new()
Expand Down
8 changes: 7 additions & 1 deletion library/alloc/src/collections/btree/set/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ fn test_send() {
}
}

#[allow(dead_code)]
#[test]
fn test_ord_absence() {
fn set<K>(mut set: BTreeSet<K>) {
set.is_empty();
Expand All @@ -626,6 +626,12 @@ fn test_ord_absence() {
fn set_clone<K: Clone>(mut set: BTreeSet<K>) {
set.clone_from(&set.clone());
}

#[derive(Debug, Clone)]
struct NonOrd;
set(BTreeSet::<NonOrd>::new());
set_debug(BTreeSet::<NonOrd>::new());
set_clone(BTreeSet::<NonOrd>::default());
}

#[test]
Expand Down
32 changes: 4 additions & 28 deletions library/alloc/tests/const_fns.rs
Original file line number Diff line number Diff line change
@@ -1,29 +1,5 @@
// Test const functions in the library

use core::cmp::Ordering;

// FIXME remove this struct once we put `K: ?const Ord` on BTreeMap::new.
#[derive(PartialEq, Eq, PartialOrd)]
pub struct MyType;

impl const Ord for MyType {
fn cmp(&self, _: &Self) -> Ordering {
Ordering::Equal
}

fn max(self, _: Self) -> Self {
Self
}

fn min(self, _: Self) -> Self {
Self
}

fn clamp(self, _: Self, _: Self) -> Self {
Self
}
}

pub const MY_VEC: Vec<usize> = Vec::new();
pub const MY_VEC2: Vec<usize> = Default::default();

Expand All @@ -32,13 +8,13 @@ pub const MY_STRING2: String = Default::default();

use std::collections::{BTreeMap, BTreeSet};

pub const MY_BTREEMAP: BTreeMap<MyType, MyType> = BTreeMap::new();
pub const MAP: &'static BTreeMap<MyType, MyType> = &MY_BTREEMAP;
pub const MY_BTREEMAP: BTreeMap<usize, usize> = BTreeMap::new();
pub const MAP: &'static BTreeMap<usize, usize> = &MY_BTREEMAP;
pub const MAP_LEN: usize = MAP.len();
pub const MAP_IS_EMPTY: bool = MAP.is_empty();

pub const MY_BTREESET: BTreeSet<MyType> = BTreeSet::new();
pub const SET: &'static BTreeSet<MyType> = &MY_BTREESET;
pub const MY_BTREESET: BTreeSet<usize> = BTreeSet::new();
pub const SET: &'static BTreeSet<usize> = &MY_BTREESET;
pub const SET_LEN: usize = SET.len();
pub const SET_IS_EMPTY: bool = SET.is_empty();

Expand Down

0 comments on commit 5878780

Please sign in to comment.