Skip to content

refactor RangeArgument #39010

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.

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 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
70 changes: 45 additions & 25 deletions src/libcollections/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,79 +14,99 @@

//! Range syntax.

use core::ops::{RangeFull, Range, RangeTo, RangeFrom};
use core::ops::{RangeFull, Range, RangeTo, RangeFrom, RangeToInclusive, RangeInclusive};
use ::Bound;

/// **RangeArgument** is implemented by Rust's built-in range types, produced
/// by range syntax like `..`, `a..`, `..b` or `c..d`.
pub trait RangeArgument<T> {
/// Start index (inclusive)
///
/// Return start value if present, else `None`.
/// Lower bound of the range
///
/// # Examples
///
/// ```
/// #![feature(collections)]
/// #![feature(collections_range)]
/// #![feature(collections_bound)]
///
/// extern crate collections;
///
/// # fn main() {
/// use collections::range::RangeArgument;
/// use collections::Bound::*;
///
/// assert_eq!((..10).start(), None);
/// assert_eq!((3..10).start(), Some(&3));
/// assert_eq!((..10).lower(), Unbounded);
/// assert_eq!((3..10).lower(), Included(&3));
/// # }
/// ```
fn start(&self) -> Option<&T> {
None
fn lower(&self) -> Bound<&T> {
Bound::Unbounded
}

/// End index (exclusive)
///
/// Return end value if present, else `None`.
/// Upper bound of the range
///
/// # Examples
///
/// ```
/// #![feature(collections)]
/// #![feature(collections_range)]
/// #![feature(collections_bound)]
///
/// extern crate collections;
///
/// # fn main() {
/// use collections::range::RangeArgument;
/// use collections::Bound::*;
///
/// assert_eq!((3..).end(), None);
/// assert_eq!((3..10).end(), Some(&10));
/// assert_eq!((3..).upper(), Unbounded);
/// assert_eq!((3..10).upper(), Excluded(&10));
/// # }
/// ```
fn end(&self) -> Option<&T> {
None
fn upper(&self) -> Bound<&T> {
Bound::Unbounded
}
}

// FIXME add inclusive ranges to RangeArgument

impl<T> RangeArgument<T> for RangeFull {}

impl<T> RangeArgument<T> for RangeFrom<T> {
fn start(&self) -> Option<&T> {
Some(&self.start)
fn lower(&self) -> Bound<&T> {
Bound::Included(&self.start)
}
}

impl<T> RangeArgument<T> for RangeTo<T> {
fn end(&self) -> Option<&T> {
Some(&self.end)
fn upper(&self) -> Bound<&T> {
Bound::Excluded(&self.end)
}
}

impl<T> RangeArgument<T> for RangeToInclusive<T> {
fn upper(&self) -> Bound<&T> {
Bound::Included(&self.end)
}
}

impl<T> RangeArgument<T> for Range<T> {
fn start(&self) -> Option<&T> {
Some(&self.start)
fn lower(&self) -> Bound<&T> {
Bound::Included(&self.start)
}
fn upper(&self) -> Bound<&T> {
Bound::Excluded(&self.end)
}
}

impl<T> RangeArgument<T> for RangeInclusive<T> {
fn lower(&self) -> Bound<&T> {
match *self {
RangeInclusive::NonEmpty { ref start, .. } => Bound::Included(start),
RangeInclusive::Empty { ref at } => Bound::Included(at),
}
}
fn end(&self) -> Option<&T> {
Some(&self.end)
fn upper(&self) -> Bound<&T> {
match *self {
RangeInclusive::NonEmpty { ref end, .. } => Bound::Included(end),
RangeInclusive::Empty { ref at } => Bound::Excluded(at),
}
}
}
14 changes: 11 additions & 3 deletions src/libcollections/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ use range::RangeArgument;
use str::{self, FromStr, Utf8Error, Chars};
use vec::Vec;
use boxed::Box;
use ::Bound;

/// A UTF-8 encoded, growable string.
///
Expand Down Expand Up @@ -1349,9 +1350,16 @@ impl String {
// of the vector version. The data is just plain bytes.
// Because the range removal happens in Drop, if the Drain iterator is leaked,
// the removal will not happen.
let len = self.len();
let start = *range.start().unwrap_or(&0);
let end = *range.end().unwrap_or(&len);
let start = match range.lower() {
Bound::Included(&start) => start,
Bound::Excluded(&start) => start + 1,
Bound::Unbounded => 0,
};
let end = match range.upper() {
Bound::Included(&end) => end + 1,
Bound::Excluded(&end) => end,
Bound::Unbounded => self.len(),
};
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the contract wrt to wrapping of the bounds arithmetic here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No issue, containers can't be bigger than isize::max afaik


// Take out two simultaneous borrows. The &mut String won't be accessed
// until iteration is over, in Drop.
Expand Down
13 changes: 11 additions & 2 deletions src/libcollections/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ use core::ptr::Shared;
use core::slice;

use super::range::RangeArgument;
use ::Bound;

/// A contiguous growable array type, written `Vec<T>` but pronounced 'vector'.
///
Expand Down Expand Up @@ -1060,8 +1061,16 @@ impl<T> Vec<T> {
// the hole, and the vector length is restored to the new length.
//
let len = self.len();
let start = *range.start().unwrap_or(&0);
let end = *range.end().unwrap_or(&len);
let start = match range.lower() {
Bound::Included(&start) => start,
Bound::Excluded(&start) => start + 1,
Bound::Unbounded => 0,
};
let end = match range.upper() {
Bound::Included(&end) => end + 1,
Bound::Excluded(&end) => end,
Bound::Unbounded => len,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto re wrapping.

assert!(start <= end);
assert!(end <= len);

Expand Down
13 changes: 11 additions & 2 deletions src/libcollections/vec_deque.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use alloc::raw_vec::RawVec;

use super::range::RangeArgument;
use super::vec::Vec;
use super::Bound;

const INITIAL_CAPACITY: usize = 7; // 2^3 - 1
const MINIMUM_CAPACITY: usize = 1; // 2 - 1
Expand Down Expand Up @@ -852,8 +853,16 @@ impl<T> VecDeque<T> {
// and the head/tail values will be restored correctly.
//
let len = self.len();
let start = *range.start().unwrap_or(&0);
let end = *range.end().unwrap_or(&len);
let start = match range.lower() {
Bound::Included(&start) => start,
Bound::Excluded(&start) => start + 1,
Bound::Unbounded => 0,
};
let end = match range.upper() {
Bound::Included(&end) => end + 1,
Bound::Excluded(&end) => end,
Bound::Unbounded => len,
};
assert!(start <= end, "drain lower bound was too large");
assert!(end <= len, "drain upper bound was too large");

Expand Down
14 changes: 11 additions & 3 deletions src/librustc_data_structures/array_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::slice;
use std::fmt;
use std::mem;
use std::collections::range::RangeArgument;
use std::collections::Bound;

pub unsafe trait Array {
type Element;
Expand Down Expand Up @@ -119,8 +120,16 @@ impl<A: Array> ArrayVec<A> {
// the hole, and the vector length is restored to the new length.
//
let len = self.len();
let start = *range.start().unwrap_or(&0);
let end = *range.end().unwrap_or(&len);
let start = match range.lower() {
Bound::Included(&start) => start,
Bound::Excluded(&start) => start + 1,
Bound::Unbounded => 0,
};
let end = match range.upper() {
Bound::Included(&end) => end + 1,
Bound::Excluded(&end) => end,
Bound::Unbounded => len,
};
assert!(start <= end);
assert!(end <= len);

Expand Down Expand Up @@ -316,4 +325,3 @@ impl<T> Default for ManuallyDrop<T> {
ManuallyDrop::new()
}
}

1 change: 1 addition & 0 deletions src/librustc_data_structures/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#![feature(untagged_unions)]
#![feature(associated_consts)]
#![feature(unsize)]
#![feature(collections_bound)]

#![cfg_attr(unix, feature(libc))]
#![cfg_attr(test, feature(test))]
Expand Down