Rollup merge of #81154 - dylni:improve-design-of-assert-len, r=KodrAus

Improve design of `assert_len`

It was discussed in the [tracking issue](https://github.com/rust-lang/rust/issues/76393#issuecomment-761765448) that `assert_len`'s name and usage are confusing. This PR improves them based on a suggestion by ``@scottmcm`` in that issue.

I also improved the documentation to make it clearer when you might want to use this method.

Old example:

```rust
let range = range.assert_len(slice.len());
```

New example:

```rust
let range = range.ensure_subset_of(..slice.len());
```

Fixes #81157
This commit is contained in:
Dylan DPC 2021-02-23 02:51:43 +01:00 committed by GitHub
commit 72e6d51583
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 117 additions and 113 deletions

View File

@ -1063,7 +1063,7 @@ fn range_tail_head<R>(&self, range: R) -> (usize, usize)
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)

View File

@ -115,7 +115,6 @@
#![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))]
@ -123,6 +122,7 @@
#![feature(set_ptr_value)]
#![feature(slice_ptr_get)]
#![feature(slice_ptr_len)]
#![feature(slice_range)]
#![feature(staged_api)]
#![feature(str_internals)]
#![feature(trusted_len)]

View File

@ -92,6 +92,8 @@
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")]

View File

@ -49,6 +49,7 @@
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};
@ -1510,14 +1511,14 @@ pub fn drain<R>(&mut self, range: R) -> Drain<'_>
// 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 }

View File

@ -1651,7 +1651,7 @@ pub fn drain<R>(&mut self, range: R) -> Drain<'_, 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
@ -2037,11 +2037,11 @@ pub fn extend_from_within<R>(&mut self, src: R)
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);
}

View File

@ -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 (`..`).
///
@ -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

View File

@ -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");
}
@ -449,3 +449,100 @@ fn index_mut(self, slice: &mut [T]) -> &mut [T] {
(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 }
}

View File

@ -18,6 +18,7 @@
use crate::ptr;
use crate::result::Result;
use crate::result::Result::{Err, Ok};
use crate::slice;
#[unstable(
feature = "slice_internals",
@ -29,7 +30,7 @@
mod ascii;
mod cmp;
pub(crate) mod index;
mod index;
mod iter;
mod raw;
mod rotate;
@ -76,6 +77,9 @@
#[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] {
@ -3075,7 +3079,7 @@ pub fn copy_within<R: RangeBounds<usize>>(&mut self, src: R, dest: usize)
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,

View File

@ -1,10 +0,0 @@
# `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