Skip to content
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

Improve design of assert_len #81154

Merged
merged 5 commits into from
Feb 23, 2021
Merged
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
2 changes: 1 addition & 1 deletion library/alloc/src/collections/vec_deque/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1063,7 +1063,7 @@ impl<T> VecDeque<T> {
where
R: RangeBounds<usize>,
{
let Range { start, end } = range.assert_len(self.len());
let Range { start, end } = slice::range(range, ..self.len());
let tail = self.wrap_add(self.tail, start);
let head = self.wrap_add(self.tail, end);
(tail, head)
Expand Down
2 changes: 1 addition & 1 deletion library/alloc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,14 @@
#![feature(or_patterns)]
#![feature(pattern)]
#![feature(ptr_internals)]
#![feature(range_bounds_assert_len)]
#![feature(rustc_attrs)]
#![feature(receiver_trait)]
#![cfg_attr(bootstrap, feature(min_const_generics))]
#![feature(min_specialization)]
#![feature(set_ptr_value)]
#![feature(slice_ptr_get)]
#![feature(slice_ptr_len)]
#![feature(slice_range)]
#![feature(staged_api)]
#![feature(str_internals)]
#![feature(trusted_len)]
Expand Down
2 changes: 2 additions & 0 deletions library/alloc/src/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ use crate::borrow::ToOwned;
use crate::boxed::Box;
use crate::vec::Vec;

#[unstable(feature = "slice_range", issue = "76393")]
pub use core::slice::range;
#[unstable(feature = "array_chunks", issue = "74985")]
pub use core::slice::ArrayChunks;
#[unstable(feature = "array_chunks", issue = "74985")]
Expand Down
5 changes: 3 additions & 2 deletions library/alloc/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ use core::iter::{FromIterator, FusedIterator};
use core::ops::Bound::{Excluded, Included, Unbounded};
use core::ops::{self, Add, AddAssign, Index, IndexMut, Range, RangeBounds};
use core::ptr;
use core::slice;
use core::str::{lossy, pattern::Pattern};

use crate::borrow::{Cow, ToOwned};
Expand Down Expand Up @@ -1510,14 +1511,14 @@ 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 Range { start, end } = range.assert_len(self.len());
let Range { start, end } = slice::range(range, ..self.len());
assert!(self.is_char_boundary(start));
assert!(self.is_char_boundary(end));

// Take out two simultaneous borrows. The &mut String won't be accessed
// until iteration is over, in Drop.
let self_ptr = self as *mut _;
// SAFETY: `assert_len` and `is_char_boundary` do the appropriate bounds checks.
// SAFETY: `slice::range` and `is_char_boundary` do the appropriate bounds checks.
let chars_iter = unsafe { self.get_unchecked(start..end) }.chars();

Drain { start, end, iter: chars_iter, string: self_ptr }
Expand Down
6 changes: 3 additions & 3 deletions library/alloc/src/vec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1650,7 +1650,7 @@ impl<T, A: Allocator> Vec<T, A> {
// the hole, and the vector length is restored to the new length.
//
let len = self.len();
let Range { start, end } = range.assert_len(len);
let Range { start, end } = slice::range(range, ..len);

unsafe {
// set self.vec length's to start, to be safe in case Drain is leaked
Expand Down Expand Up @@ -2036,11 +2036,11 @@ impl<T: Clone, A: Allocator> Vec<T, A> {
where
R: RangeBounds<usize>,
{
let range = src.assert_len(self.len());
let range = slice::range(src, ..self.len());
self.reserve(range.len());

// SAFETY:
// - `assert_len` guarantees that the given range is valid for indexing self
// - `slice::range` guarantees that the given range is valid for indexing self
unsafe {
self.spec_extend_from_within(range);
}
Expand Down
90 changes: 0 additions & 90 deletions library/core/src/ops/range.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
use crate::fmt;
use crate::hash::Hash;
use crate::slice::index::{
slice_end_index_len_fail, slice_end_index_overflow_fail, slice_index_order_fail,
slice_start_index_overflow_fail,
};

/// An unbounded range (`..`).
///
Expand Down Expand Up @@ -764,92 +760,6 @@ pub trait RangeBounds<T: ?Sized> {
#[stable(feature = "collections_range", since = "1.28.0")]
fn end_bound(&self) -> Bound<&T>;

/// Performs bounds-checking of this range.
///
/// The returned [`Range`] is safe to pass to [`slice::get_unchecked`] and
/// [`slice::get_unchecked_mut`] for slices of the given length.
///
/// [`slice::get_unchecked`]: ../../std/primitive.slice.html#method.get_unchecked
/// [`slice::get_unchecked_mut`]: ../../std/primitive.slice.html#method.get_unchecked_mut
///
/// # Panics
///
/// Panics if the range would be out of bounds.
///
/// # Examples
///
/// ```
/// #![feature(range_bounds_assert_len)]
///
/// use std::ops::RangeBounds;
///
/// let v = [10, 40, 30];
/// assert_eq!(1..2, (1..2).assert_len(v.len()));
/// assert_eq!(0..2, (..2).assert_len(v.len()));
/// assert_eq!(1..3, (1..).assert_len(v.len()));
/// ```
///
/// Panics when [`Index::index`] would panic:
///
/// ```should_panic
/// #![feature(range_bounds_assert_len)]
///
/// use std::ops::RangeBounds;
///
/// (2..1).assert_len(3);
/// ```
///
/// ```should_panic
/// #![feature(range_bounds_assert_len)]
///
/// use std::ops::RangeBounds;
///
/// (1..4).assert_len(3);
/// ```
///
/// ```should_panic
/// #![feature(range_bounds_assert_len)]
///
/// use std::ops::RangeBounds;
///
/// (1..=usize::MAX).assert_len(3);
/// ```
///
/// [`Index::index`]: crate::ops::Index::index
#[track_caller]
#[unstable(feature = "range_bounds_assert_len", issue = "76393")]
fn assert_len(self, len: usize) -> Range<usize>
where
Self: RangeBounds<usize>,
{
let start: Bound<&usize> = self.start_bound();
let start = match start {
Bound::Included(&start) => start,
Bound::Excluded(start) => {
start.checked_add(1).unwrap_or_else(|| slice_start_index_overflow_fail())
}
Bound::Unbounded => 0,
};

let end: Bound<&usize> = self.end_bound();
let end = match end {
Bound::Included(end) => {
end.checked_add(1).unwrap_or_else(|| slice_end_index_overflow_fail())
}
Bound::Excluded(&end) => end,
Bound::Unbounded => len,
};

if start > end {
slice_index_order_fail(start, end);
}
if end > len {
slice_end_index_len_fail(end, len);
}

Range { start, end }
}

/// Returns `true` if `item` is contained in the range.
///
/// # Examples
Expand Down
105 changes: 101 additions & 4 deletions library/core/src/slice/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,28 +37,28 @@ fn slice_start_index_len_fail(index: usize, len: usize) -> ! {
#[inline(never)]
#[cold]
#[track_caller]
pub(crate) fn slice_end_index_len_fail(index: usize, len: usize) -> ! {
fn slice_end_index_len_fail(index: usize, len: usize) -> ! {
panic!("range end index {} out of range for slice of length {}", index, len);
}

#[inline(never)]
#[cold]
#[track_caller]
pub(crate) fn slice_index_order_fail(index: usize, end: usize) -> ! {
fn slice_index_order_fail(index: usize, end: usize) -> ! {
panic!("slice index starts at {} but ends at {}", index, end);
}

#[inline(never)]
#[cold]
#[track_caller]
pub(crate) fn slice_start_index_overflow_fail() -> ! {
fn slice_start_index_overflow_fail() -> ! {
panic!("attempted to index slice from after maximum usize");
}

#[inline(never)]
#[cold]
#[track_caller]
pub(crate) fn slice_end_index_overflow_fail() -> ! {
fn slice_end_index_overflow_fail() -> ! {
panic!("attempted to index slice up to maximum usize");
}

Expand Down Expand Up @@ -449,3 +449,100 @@ unsafe impl<T> SliceIndex<[T]> for ops::RangeToInclusive<usize> {
(0..=self.end).index_mut(slice)
}
}

/// Performs bounds-checking of a range.
///
/// This method is similar to [`Index::index`] for slices, but it returns a
/// [`Range`] equivalent to `range`. You can use this method to turn any range
/// into `start` and `end` values.
///
/// `bounds` is the range of the slice to use for bounds-checking. It should
/// be a [`RangeTo`] range that ends at the length of the slice.
///
/// The returned [`Range`] is safe to pass to [`slice::get_unchecked`] and
/// [`slice::get_unchecked_mut`] for slices with the given range.
///
/// [`Range`]: ops::Range
/// [`RangeTo`]: ops::RangeTo
/// [`slice::get_unchecked`]: ../../std/primitive.slice.html#method.get_unchecked
/// [`slice::get_unchecked_mut`]: ../../std/primitive.slice.html#method.get_unchecked_mut
///
/// # Panics
///
/// Panics if `range` would be out of bounds.
///
/// # Examples
///
/// ```
/// #![feature(slice_range)]
///
/// use std::slice;
///
/// let v = [10, 40, 30];
/// assert_eq!(1..2, slice::range(1..2, ..v.len()));
/// assert_eq!(0..2, slice::range(..2, ..v.len()));
/// assert_eq!(1..3, slice::range(1.., ..v.len()));
/// ```
///
/// Panics when [`Index::index`] would panic:
///
/// ```should_panic
/// #![feature(slice_range)]
///
/// use std::slice;
///
/// slice::range(2..1, ..3);
/// ```
///
/// ```should_panic
/// #![feature(slice_range)]
///
/// use std::slice;
///
/// slice::range(1..4, ..3);
/// ```
///
/// ```should_panic
/// #![feature(slice_range)]
///
/// use std::slice;
///
/// slice::range(1..=usize::MAX, ..3);
/// ```
///
/// [`Index::index`]: ops::Index::index
#[track_caller]
#[unstable(feature = "slice_range", issue = "76393")]
pub fn range<R>(range: R, bounds: ops::RangeTo<usize>) -> ops::Range<usize>
where
R: ops::RangeBounds<usize>,
{
let len = bounds.end;

let start: ops::Bound<&usize> = range.start_bound();
let start = match start {
ops::Bound::Included(&start) => start,
ops::Bound::Excluded(start) => {
start.checked_add(1).unwrap_or_else(|| slice_start_index_overflow_fail())
}
ops::Bound::Unbounded => 0,
};

let end: ops::Bound<&usize> = range.end_bound();
let end = match end {
ops::Bound::Included(end) => {
end.checked_add(1).unwrap_or_else(|| slice_end_index_overflow_fail())
}
ops::Bound::Excluded(&end) => end,
ops::Bound::Unbounded => len,
};

if start > end {
slice_index_order_fail(start, end);
}
if end > len {
slice_end_index_len_fail(end, len);
}

ops::Range { start, end }
}
8 changes: 6 additions & 2 deletions library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::option::Option::{None, Some};
use crate::ptr;
use crate::result::Result;
use crate::result::Result::{Err, Ok};
use crate::slice;

#[unstable(
feature = "slice_internals",
Expand All @@ -29,7 +30,7 @@ pub mod memchr;

mod ascii;
mod cmp;
pub(crate) mod index;
mod index;
mod iter;
mod raw;
mod rotate;
Expand Down Expand Up @@ -76,6 +77,9 @@ pub use sort::heapsort;
#[stable(feature = "slice_get_slice", since = "1.28.0")]
pub use index::SliceIndex;

#[unstable(feature = "slice_range", issue = "76393")]
pub use index::range;

#[lang = "slice"]
#[cfg(not(test))]
impl<T> [T] {
Expand Down Expand Up @@ -3052,7 +3056,7 @@ impl<T> [T] {
where
T: Copy,
{
let Range { start: src_start, end: src_end } = src.assert_len(self.len());
let Range { start: src_start, end: src_end } = slice::range(src, ..self.len());
let count = src_end - src_start;
assert!(dest <= self.len() - count, "dest is out of bounds");
// SAFETY: the conditions for `ptr::copy` have all been checked above,
Expand Down

This file was deleted.