Skip to content

Commit

Permalink
Auto merge of #76885 - dylni:move-slice-check-range-to-range-bounds, …
Browse files Browse the repository at this point in the history
…r=KodrAus

Move `slice::check_range` to `RangeBounds`

Since this method doesn't take a slice anymore (#76662), it makes more sense to define it on `RangeBounds`.

Questions:
- Should the new method be `assert_len` or `assert_length`?
  • Loading branch information
bors committed Oct 18, 2020
2 parents 4d247ad + f055b0b commit 187b877
Show file tree
Hide file tree
Showing 10 changed files with 112 additions and 101 deletions.
2 changes: 1 addition & 1 deletion library/alloc/src/collections/vec_deque.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,7 @@ impl<T> VecDeque<T> {
where
R: RangeBounds<usize>,
{
let Range { start, end } = slice::check_range(self.len(), range);
let Range { start, end } = range.assert_len(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 @@ -114,11 +114,11 @@
#![feature(or_patterns)]
#![feature(pattern)]
#![feature(ptr_internals)]
#![feature(range_bounds_assert_len)]
#![feature(raw_ref_op)]
#![feature(rustc_attrs)]
#![feature(receiver_trait)]
#![feature(min_specialization)]
#![feature(slice_check_range)]
#![feature(slice_ptr_get)]
#![feature(slice_ptr_len)]
#![feature(staged_api)]
Expand Down
2 changes: 0 additions & 2 deletions library/alloc/src/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ use crate::borrow::ToOwned;
use crate::boxed::Box;
use crate::vec::Vec;

#[unstable(feature = "slice_check_range", issue = "76393")]
pub use core::slice::check_range;
#[unstable(feature = "array_chunks", issue = "74985")]
pub use core::slice::ArrayChunks;
#[unstable(feature = "array_chunks", issue = "74985")]
Expand Down
5 changes: 2 additions & 3 deletions library/alloc/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ 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 @@ -1507,14 +1506,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 } = slice::check_range(self.len(), range);
let Range { start, end } = range.assert_len(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: `check_range` and `is_char_boundary` do the appropriate bounds checks.
// SAFETY: `assert_len` 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
2 changes: 1 addition & 1 deletion library/alloc/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1314,7 +1314,7 @@ impl<T> Vec<T> {
// the hole, and the vector length is restored to the new length.
//
let len = self.len();
let Range { start, end } = slice::check_range(len, range);
let Range { start, end } = range.assert_len(len);

unsafe {
// set self.vec length's to start, to be safe in case Drain is leaked
Expand Down
90 changes: 90 additions & 0 deletions library/core/src/ops/range.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
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 @@ -701,6 +705,92 @@ 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
83 changes: 5 additions & 78 deletions library/core/src/slice/index.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Indexing implementations for `[T]`.

use crate::ops::{self, Bound, Range, RangeBounds};
use crate::ops;
use crate::ptr;

#[stable(feature = "rust1", since = "1.0.0")]
Expand Down Expand Up @@ -37,104 +37,31 @@ fn slice_start_index_len_fail(index: usize, len: usize) -> ! {
#[inline(never)]
#[cold]
#[track_caller]
pub(super) fn slice_end_index_len_fail(index: usize, len: usize) -> ! {
pub(crate) 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(super) fn slice_index_order_fail(index: usize, end: usize) -> ! {
pub(crate) 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(super) fn slice_start_index_overflow_fail() -> ! {
pub(crate) fn slice_start_index_overflow_fail() -> ! {
panic!("attempted to index slice from after maximum usize");
}

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

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

let end = match range.end_bound() {
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 }
}

mod private_slice_index {
use super::ops;
#[stable(feature = "slice_get_slice", since = "1.28.0")]
Expand Down
7 changes: 2 additions & 5 deletions library/core/src/slice/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub mod memchr;

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

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

#[lang = "slice"]
#[cfg(not(test))]
impl<T> [T] {
Expand Down Expand Up @@ -2714,7 +2711,7 @@ impl<T> [T] {
where
T: Copy,
{
let Range { start: src_start, end: src_end } = check_range(self.len(), src);
let Range { start: src_start, end: src_end } = src.assert_len(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
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# `range_bounds_assert_len`

The tracking issue for this feature is: [#76393]

------------------------

This adds [`RangeBounds::assert_len`].

[#76393]: https://github.com/rust-lang/rust/issues/76393
[`RangeBounds::assert_len`]: https://doc.rust-lang.org/nightly/std/ops/trait.RangeBounds.html#method.assert_len
10 changes: 0 additions & 10 deletions src/doc/unstable-book/src/library-features/slice-check-range.md

This file was deleted.

0 comments on commit 187b877

Please sign in to comment.