Skip to content

relaunch RangeBounds #44518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

relaunch RangeBounds #44518

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f357ea8
Change trait RangeArgument to struct RangeBounds.
Jul 3, 2017
0c07435
review fixes
liigo Sep 8, 2017
dd30121
fix compile/docs errors
liigo Sep 12, 2017
723b069
link `BTreeMap` using absolute path for the moment
liigo Sep 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 29 additions & 30 deletions src/liballoc/btree/map.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,11 @@ use core::fmt::Debug;
use core::hash::{Hash, Hasher};
use core::iter::{FromIterator, Peekable, FusedIterator};
use core::marker::PhantomData;
use core::ops::Index;
use core::ops::{Index, RangeBounds};
use core::ops::Bound::{Excluded, Included, Unbounded};
use core::{fmt, intrinsics, mem, ptr};

use borrow::Borrow;
use Bound::{Excluded, Included, Unbounded};
use range::RangeArgument;

use super::node::{self, Handle, NodeRef, marker};
use super::search;
Expand Down Expand Up @@ -783,18 +782,18 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// map.insert(3, "a");
/// map.insert(5, "b");
/// map.insert(8, "c");
/// for (&key, &value) in map.range((Included(&4), Included(&8))) {
/// for (&key, &value) in map.range((Included(4), Included(8))) {
/// println!("{}: {}", key, value);
/// }
/// assert_eq!(Some((&5, &"b")), map.range(4..).next());
/// ```
#[stable(feature = "btree_range", since = "1.17.0")]
pub fn range<T: ?Sized, R>(&self, range: R) -> Range<K, V>
where T: Ord, K: Borrow<T>, R: RangeArgument<T>
pub fn range<T, R>(&self, range: R) -> Range<K, V>
where T: Ord, K: Borrow<T>, R: Into<RangeBounds<T>>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be RangeBounds<&T>?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I actually recall this being one of the things I struggled with making the original PR. This prevents things like "a".to_string()..="z".to_string(). Because Borrow works differently from AsRef, this prevents both String ranges and str ranges from working properly.

There may be a way to make it work but I was really stumped in the original PR and got busy and didn't pick it back up.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm stumped too.

{
let root1 = self.root.as_ref();
let root2 = self.root.as_ref();
let (f, b) = range_search(root1, root2, range);
let (f, b) = range_search(root1, root2, range.into());

Range { front: f, back: b}
}
Expand Down Expand Up @@ -829,12 +828,12 @@ impl<K: Ord, V> BTreeMap<K, V> {
/// }
/// ```
#[stable(feature = "btree_range", since = "1.17.0")]
pub fn range_mut<T: ?Sized, R>(&mut self, range: R) -> RangeMut<K, V>
where T: Ord, K: Borrow<T>, R: RangeArgument<T>
pub fn range_mut<T, R>(&mut self, range: R) -> RangeMut<K, V>
where T: Ord, K: Borrow<T>, R: Into<RangeBounds<T>>
{
let root1 = self.root.as_mut();
let root2 = unsafe { ptr::read(&root1) };
let (f, b) = range_search(root1, root2, range);
let (f, b) = range_search(root1, root2, range.into());

RangeMut {
front: f,
Expand Down Expand Up @@ -1780,21 +1779,21 @@ fn last_leaf_edge<BorrowType, K, V>
}
}

fn range_search<BorrowType, K, V, Q: ?Sized, R: RangeArgument<Q>>(
fn range_search<BorrowType, K, V, Q>(
root1: NodeRef<BorrowType, K, V, marker::LeafOrInternal>,
root2: NodeRef<BorrowType, K, V, marker::LeafOrInternal>,
range: R
range: RangeBounds<Q>
)-> (Handle<NodeRef<BorrowType, K, V, marker::Leaf>, marker::Edge>,
Handle<NodeRef<BorrowType, K, V, marker::Leaf>, marker::Edge>)
where Q: Ord, K: Borrow<Q>
{
match (range.start(), range.end()) {
(Excluded(s), Excluded(e)) if s==e =>
match (&range.start, &range.end) {
(&Excluded(ref s), &Excluded(ref e)) if *s == *e =>
panic!("range start and end are equal and excluded in BTreeMap"),
(Included(s), Included(e)) |
(Included(s), Excluded(e)) |
(Excluded(s), Included(e)) |
(Excluded(s), Excluded(e)) if s>e =>
(&Included(ref s), &Included(ref e)) |
(&Included(ref s), &Excluded(ref e)) |
(&Excluded(ref s), &Included(ref e)) |
(&Excluded(ref s), &Excluded(ref e)) if *s > *e =>
panic!("range start is greater than range end in BTreeMap"),
_ => {},
};
Expand All @@ -1806,32 +1805,32 @@ fn range_search<BorrowType, K, V, Q: ?Sized, R: RangeArgument<Q>>(
let mut diverged = false;

loop {
let min_edge = match (min_found, range.start()) {
(false, Included(key)) => match search::search_linear(&min_node, key) {
let min_edge = match (min_found, &range.start) {
(false, &Included(ref key)) => match search::search_linear(&min_node, key) {
(i, true) => { min_found = true; i },
(i, false) => i,
},
(false, Excluded(key)) => match search::search_linear(&min_node, key) {
(false, &Excluded(ref key)) => match search::search_linear(&min_node, &key) {
(i, true) => { min_found = true; i+1 },
(i, false) => i,
},
(_, Unbounded) => 0,
(true, Included(_)) => min_node.keys().len(),
(true, Excluded(_)) => 0,
(_, &Unbounded) => 0,
(true, &Included(_)) => min_node.keys().len(),
(true, &Excluded(_)) => 0,
};

let max_edge = match (max_found, range.end()) {
(false, Included(key)) => match search::search_linear(&max_node, key) {
let max_edge = match (max_found, &range.end) {
(false, &Included(ref key)) => match search::search_linear(&max_node, key) {
(i, true) => { max_found = true; i+1 },
(i, false) => i,
},
(false, Excluded(key)) => match search::search_linear(&max_node, key) {
(false, &Excluded(ref key)) => match search::search_linear(&max_node, key) {
(i, true) => { max_found = true; i },
(i, false) => i,
},
(_, Unbounded) => max_node.keys().len(),
(true, Included(_)) => 0,
(true, Excluded(_)) => max_node.keys().len(),
(_, &Unbounded) => max_node.keys().len(),
(true, &Included(_)) => 0,
(true, &Excluded(_)) => max_node.keys().len(),
};

if !diverged {
Expand Down
11 changes: 5 additions & 6 deletions src/liballoc/btree/set.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@ use core::cmp::{min, max};
use core::fmt::Debug;
use core::fmt;
use core::iter::{Peekable, FromIterator, FusedIterator};
use core::ops::{BitOr, BitAnd, BitXor, Sub};
use core::ops::{BitOr, BitAnd, BitXor, Sub, RangeBounds};

use borrow::Borrow;
use btree_map::{BTreeMap, Keys};
use super::Recover;
use range::RangeArgument;

// FIXME(conventions): implement bounded iterators

Expand Down Expand Up @@ -282,16 +281,16 @@ impl<T: Ord> BTreeSet<T> {
/// set.insert(3);
/// set.insert(5);
/// set.insert(8);
/// for &elem in set.range((Included(&4), Included(&8))) {
/// for &elem in set.range((Included(4), Included(8))) {
/// println!("{}", elem);
/// }
/// assert_eq!(Some(&5), set.range(4..).next());
/// ```
#[stable(feature = "btree_range", since = "1.17.0")]
pub fn range<K: ?Sized, R>(&self, range: R) -> Range<T>
where K: Ord, T: Borrow<K>, R: RangeArgument<K>
pub fn range<K, R>(&self, range: R) -> Range<T>
where K: Ord, T: Borrow<K>, R: Into<RangeBounds<K>>
{
Range { iter: self.map.range(range) }
Range { iter: self.map.range(range.into()) }
}
}

Expand Down
57 changes: 5 additions & 52 deletions src/liballoc/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
#![feature(pattern)]
#![feature(placement_in_syntax)]
#![feature(placement_new_protocol)]
#![feature(range_argument)]
#![feature(rustc_attrs)]
#![feature(shared)]
#![feature(slice_get_slice)]
Expand Down Expand Up @@ -171,7 +172,6 @@ mod btree;
pub mod borrow;
pub mod fmt;
pub mod linked_list;
pub mod range;
pub mod slice;
pub mod str;
pub mod string;
Expand All @@ -197,64 +197,17 @@ mod std {
pub use core::ops; // RangeFull
}

/// An endpoint of a range of keys.
///
/// # Examples
///
/// `Bound`s are range endpoints:
///
/// ```
/// #![feature(collections_range)]
///
/// use std::collections::range::RangeArgument;
/// use std::collections::Bound::*;
///
/// assert_eq!((..100).start(), Unbounded);
/// assert_eq!((1..12).start(), Included(&1));
/// assert_eq!((1..12).end(), Excluded(&12));
/// ```
///
/// Using a tuple of `Bound`s as an argument to [`BTreeMap::range`].
/// Note that in most cases, it's better to use range syntax (`1..5`) instead.
///
/// ```
/// use std::collections::BTreeMap;
/// use std::collections::Bound::{Excluded, Included, Unbounded};
///
/// let mut map = BTreeMap::new();
/// map.insert(3, "a");
/// map.insert(5, "b");
/// map.insert(8, "c");
///
/// for (key, value) in map.range((Excluded(3), Included(8))) {
/// println!("{}: {}", key, value);
/// }
///
/// assert_eq!(Some((&3, &"a")), map.range((Unbounded, Included(5))).next());
/// ```
///
/// [`BTreeMap::range`]: btree_map/struct.BTreeMap.html#method.range
#[stable(feature = "collections_bound", since = "1.17.0")]
#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
pub enum Bound<T> {
/// An inclusive bound.
#[stable(feature = "collections_bound", since = "1.17.0")]
Included(#[stable(feature = "collections_bound", since = "1.17.0")] T),
/// An exclusive bound.
#[stable(feature = "collections_bound", since = "1.17.0")]
Excluded(#[stable(feature = "collections_bound", since = "1.17.0")] T),
/// An infinite endpoint. Indicates that there is no bound in this direction.
#[stable(feature = "collections_bound", since = "1.17.0")]
Unbounded,
}

/// An intermediate trait for specialization of `Extend`.
#[doc(hidden)]
trait SpecExtend<I: IntoIterator> {
/// Extends `self` with the contents of the given iterator.
fn spec_extend(&mut self, iter: I);
}

#[rustc_deprecated(reason = "moved to core::ops", since = "1.22.0")]
#[unstable(feature = "range_argument", issue = "30877")]
pub use core::ops::Bound;

#[doc(no_inline)]
pub use binary_heap::BinaryHeap;
#[doc(no_inline)]
Expand Down
Loading