Skip to content

Commit ef55a0a

Browse files
committed
Auto merge of #75207 - dylni:add-slice-check-range, r=KodrAus
Add `slice::check_range` This method is useful for [`RangeBounds`] parameters. It's even been [rewritten](https://github.com/rust-lang/rust/blob/22ee68dc586440f96b76b32fbd6087507c6afdb9/src/librustc_data_structures/sorted_map.rs#L214) [many](https://github.com/rust-lang/rust/blob/22ee68dc586440f96b76b32fbd6087507c6afdb9/library/alloc/src/vec.rs#L1299) [times](https://github.com/rust-lang/rust/blob/22ee68dc586440f96b76b32fbd6087507c6afdb9/library/core/src/slice/mod.rs#L2441) in the standard library, sometimes assuming that the bounds won't be [`usize::MAX`]. For example, [`Vec::drain`] creates an empty iterator when [`usize::MAX`] is used as an inclusive end bound: ```rust assert!(vec![1].drain(..=usize::max_value()).eq(iter::empty())); ``` If this PR is merged, I'll create another to use it for those methods. [`RangeBounds`]: https://doc.rust-lang.org/std/ops/trait.RangeBounds.html [`usize::MAX`]: https://doc.rust-lang.org/std/primitive.usize.html#associatedconstant.MAX [`Vec::drain`]: https://doc.rust-lang.org/std/vec/struct.Vec.html#method.drain
2 parents 4ffb5c5 + d9e877f commit ef55a0a

File tree

7 files changed

+111
-94
lines changed

7 files changed

+111
-94
lines changed

library/alloc/src/collections/vec_deque.rs

+13-26
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ use core::fmt;
1414
use core::hash::{Hash, Hasher};
1515
use core::iter::{once, repeat_with, FromIterator, FusedIterator};
1616
use core::mem::{self, replace, ManuallyDrop};
17-
use core::ops::Bound::{Excluded, Included, Unbounded};
18-
use core::ops::{Index, IndexMut, RangeBounds, Try};
17+
use core::ops::{Index, IndexMut, Range, RangeBounds, Try};
1918
use core::ptr::{self, NonNull};
2019
use core::slice;
2120

@@ -1090,24 +1089,18 @@ impl<T> VecDeque<T> {
10901089
self.tail == self.head
10911090
}
10921091

1093-
fn range_start_end<R>(&self, range: R) -> (usize, usize)
1092+
fn range_tail_head<R>(&self, range: R) -> (usize, usize)
10941093
where
10951094
R: RangeBounds<usize>,
10961095
{
1097-
let len = self.len();
1098-
let start = match range.start_bound() {
1099-
Included(&n) => n,
1100-
Excluded(&n) => n + 1,
1101-
Unbounded => 0,
1102-
};
1103-
let end = match range.end_bound() {
1104-
Included(&n) => n + 1,
1105-
Excluded(&n) => n,
1106-
Unbounded => len,
1107-
};
1108-
assert!(start <= end, "lower bound was too large");
1109-
assert!(end <= len, "upper bound was too large");
1110-
(start, end)
1096+
// SAFETY: This buffer is only used to check the range. It might be partially
1097+
// uninitialized, but `check_range` needs a contiguous slice.
1098+
// https://github.com/rust-lang/rust/pull/75207#discussion_r471193682
1099+
let buffer = unsafe { slice::from_raw_parts(self.ptr(), self.len()) };
1100+
let Range { start, end } = buffer.check_range(range);
1101+
let tail = self.wrap_add(self.tail, start);
1102+
let head = self.wrap_add(self.tail, end);
1103+
(tail, head)
11111104
}
11121105

11131106
/// Creates an iterator that covers the specified range in the `VecDeque`.
@@ -1138,9 +1131,7 @@ impl<T> VecDeque<T> {
11381131
where
11391132
R: RangeBounds<usize>,
11401133
{
1141-
let (start, end) = self.range_start_end(range);
1142-
let tail = self.wrap_add(self.tail, start);
1143-
let head = self.wrap_add(self.tail, end);
1134+
let (tail, head) = self.range_tail_head(range);
11441135
Iter {
11451136
tail,
11461137
head,
@@ -1181,9 +1172,7 @@ impl<T> VecDeque<T> {
11811172
where
11821173
R: RangeBounds<usize>,
11831174
{
1184-
let (start, end) = self.range_start_end(range);
1185-
let tail = self.wrap_add(self.tail, start);
1186-
let head = self.wrap_add(self.tail, end);
1175+
let (tail, head) = self.range_tail_head(range);
11871176
IterMut {
11881177
tail,
11891178
head,
@@ -1237,7 +1226,7 @@ impl<T> VecDeque<T> {
12371226
// When finished, the remaining data will be copied back to cover the hole,
12381227
// and the head/tail values will be restored correctly.
12391228
//
1240-
let (start, end) = self.range_start_end(range);
1229+
let (drain_tail, drain_head) = self.range_tail_head(range);
12411230

12421231
// The deque's elements are parted into three segments:
12431232
// * self.tail -> drain_tail
@@ -1255,8 +1244,6 @@ impl<T> VecDeque<T> {
12551244
// T t h H
12561245
// [. . . o o x x o o . . .]
12571246
//
1258-
let drain_tail = self.wrap_add(self.tail, start);
1259-
let drain_head = self.wrap_add(self.tail, end);
12601247
let head = self.head;
12611248

12621249
// "forget" about the values after the start of the drain until after

library/alloc/src/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@
119119
#![feature(rustc_attrs)]
120120
#![feature(receiver_trait)]
121121
#![feature(min_specialization)]
122+
#![feature(slice_check_range)]
122123
#![feature(slice_ptr_get)]
123124
#![feature(slice_ptr_len)]
124125
#![feature(staged_api)]

library/alloc/src/string.rs

+6-14
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ use core::fmt;
4747
use core::hash;
4848
use core::iter::{FromIterator, FusedIterator};
4949
use core::ops::Bound::{Excluded, Included, Unbounded};
50-
use core::ops::{self, Add, AddAssign, Index, IndexMut, RangeBounds};
50+
use core::ops::{self, Add, AddAssign, Index, IndexMut, Range, RangeBounds};
5151
use core::ptr;
5252
use core::str::{lossy, pattern::Pattern};
5353

@@ -1506,23 +1506,15 @@ impl String {
15061506
// of the vector version. The data is just plain bytes.
15071507
// Because the range removal happens in Drop, if the Drain iterator is leaked,
15081508
// the removal will not happen.
1509-
let len = self.len();
1510-
let start = match range.start_bound() {
1511-
Included(&n) => n,
1512-
Excluded(&n) => n + 1,
1513-
Unbounded => 0,
1514-
};
1515-
let end = match range.end_bound() {
1516-
Included(&n) => n + 1,
1517-
Excluded(&n) => n,
1518-
Unbounded => len,
1519-
};
1509+
let Range { start, end } = self.as_bytes().check_range(range);
1510+
assert!(self.is_char_boundary(start));
1511+
assert!(self.is_char_boundary(end));
15201512

15211513
// Take out two simultaneous borrows. The &mut String won't be accessed
15221514
// until iteration is over, in Drop.
15231515
let self_ptr = self as *mut _;
1524-
// slicing does the appropriate bounds checks
1525-
let chars_iter = self[start..end].chars();
1516+
// SAFETY: `check_range` and `is_char_boundary` do the appropriate bounds checks.
1517+
let chars_iter = unsafe { self.get_unchecked(start..end) }.chars();
15261518

15271519
Drain { start, end, iter: chars_iter, string: self_ptr }
15281520
}

library/alloc/src/vec.rs

+2-31
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,7 @@ use core::iter::{
6363
};
6464
use core::marker::PhantomData;
6565
use core::mem::{self, ManuallyDrop, MaybeUninit};
66-
use core::ops::Bound::{Excluded, Included, Unbounded};
67-
use core::ops::{self, Index, IndexMut, RangeBounds};
66+
use core::ops::{self, Index, IndexMut, Range, RangeBounds};
6867
use core::ptr::{self, NonNull};
6968
use core::slice::{self, SliceIndex};
7069

@@ -1306,35 +1305,7 @@ impl<T> Vec<T> {
13061305
// the hole, and the vector length is restored to the new length.
13071306
//
13081307
let len = self.len();
1309-
let start = match range.start_bound() {
1310-
Included(&n) => n,
1311-
Excluded(&n) => n + 1,
1312-
Unbounded => 0,
1313-
};
1314-
let end = match range.end_bound() {
1315-
Included(&n) => n + 1,
1316-
Excluded(&n) => n,
1317-
Unbounded => len,
1318-
};
1319-
1320-
#[cold]
1321-
#[inline(never)]
1322-
fn start_assert_failed(start: usize, end: usize) -> ! {
1323-
panic!("start drain index (is {}) should be <= end drain index (is {})", start, end);
1324-
}
1325-
1326-
#[cold]
1327-
#[inline(never)]
1328-
fn end_assert_failed(end: usize, len: usize) -> ! {
1329-
panic!("end drain index (is {}) should be <= len (is {})", end, len);
1330-
}
1331-
1332-
if start > end {
1333-
start_assert_failed(start, end);
1334-
}
1335-
if end > len {
1336-
end_assert_failed(end, len);
1337-
}
1308+
let Range { start, end } = self.check_range(range);
13381309

13391310
unsafe {
13401311
// set self.vec length's to start, to be safe in case Drain is leaked

library/core/src/slice/mod.rs

+86-21
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ use crate::intrinsics::{assume, exact_div, is_aligned_and_not_null, unchecked_su
2929
use crate::iter::*;
3030
use crate::marker::{self, Copy, Send, Sized, Sync};
3131
use crate::mem;
32-
use crate::ops::{self, FnMut, Range};
32+
use crate::ops::{self, Bound, FnMut, Range, RangeBounds};
3333
use crate::option::Option;
3434
use crate::option::Option::{None, Some};
3535
use crate::ptr::{self, NonNull};
@@ -355,6 +355,79 @@ impl<T> [T] {
355355
unsafe { &mut *index.get_unchecked_mut(self) }
356356
}
357357

358+
/// Converts a range over this slice to [`Range`].
359+
///
360+
/// The returned range is safe to pass to [`get_unchecked`] and [`get_unchecked_mut`].
361+
///
362+
/// [`get_unchecked`]: #method.get_unchecked
363+
/// [`get_unchecked_mut`]: #method.get_unchecked_mut
364+
///
365+
/// # Panics
366+
///
367+
/// Panics if the range is out of bounds.
368+
///
369+
/// # Examples
370+
///
371+
/// ```
372+
/// #![feature(slice_check_range)]
373+
///
374+
/// let v = [10, 40, 30];
375+
/// assert_eq!(1..2, v.check_range(1..2));
376+
/// assert_eq!(0..2, v.check_range(..2));
377+
/// assert_eq!(1..3, v.check_range(1..));
378+
/// ```
379+
///
380+
/// Panics when [`Index::index`] would panic:
381+
///
382+
/// ```should_panic
383+
/// #![feature(slice_check_range)]
384+
///
385+
/// [10, 40, 30].check_range(2..1);
386+
/// ```
387+
///
388+
/// ```should_panic
389+
/// #![feature(slice_check_range)]
390+
///
391+
/// [10, 40, 30].check_range(1..4);
392+
/// ```
393+
///
394+
/// ```should_panic
395+
/// #![feature(slice_check_range)]
396+
///
397+
/// [10, 40, 30].check_range(1..=usize::MAX);
398+
/// ```
399+
///
400+
/// [`Index::index`]: ops::Index::index
401+
#[track_caller]
402+
#[unstable(feature = "slice_check_range", issue = "none")]
403+
pub fn check_range<R: RangeBounds<usize>>(&self, range: R) -> Range<usize> {
404+
let start = match range.start_bound() {
405+
Bound::Included(&start) => start,
406+
Bound::Excluded(start) => {
407+
start.checked_add(1).unwrap_or_else(|| slice_start_index_overflow_fail())
408+
}
409+
Bound::Unbounded => 0,
410+
};
411+
412+
let len = self.len();
413+
let end = match range.end_bound() {
414+
Bound::Included(end) => {
415+
end.checked_add(1).unwrap_or_else(|| slice_end_index_overflow_fail())
416+
}
417+
Bound::Excluded(&end) => end,
418+
Bound::Unbounded => len,
419+
};
420+
421+
if start > end {
422+
slice_index_order_fail(start, end);
423+
}
424+
if end > len {
425+
slice_end_index_len_fail(end, len);
426+
}
427+
428+
Range { start, end }
429+
}
430+
358431
/// Returns a raw pointer to the slice's buffer.
359432
///
360433
/// The caller must ensure that the slice outlives the pointer this
@@ -2651,26 +2724,11 @@ impl<T> [T] {
26512724
/// ```
26522725
#[stable(feature = "copy_within", since = "1.37.0")]
26532726
#[track_caller]
2654-
pub fn copy_within<R: ops::RangeBounds<usize>>(&mut self, src: R, dest: usize)
2727+
pub fn copy_within<R: RangeBounds<usize>>(&mut self, src: R, dest: usize)
26552728
where
26562729
T: Copy,
26572730
{
2658-
let src_start = match src.start_bound() {
2659-
ops::Bound::Included(&n) => n,
2660-
ops::Bound::Excluded(&n) => {
2661-
n.checked_add(1).unwrap_or_else(|| slice_index_overflow_fail())
2662-
}
2663-
ops::Bound::Unbounded => 0,
2664-
};
2665-
let src_end = match src.end_bound() {
2666-
ops::Bound::Included(&n) => {
2667-
n.checked_add(1).unwrap_or_else(|| slice_index_overflow_fail())
2668-
}
2669-
ops::Bound::Excluded(&n) => n,
2670-
ops::Bound::Unbounded => self.len(),
2671-
};
2672-
assert!(src_start <= src_end, "src end is before src start");
2673-
assert!(src_end <= self.len(), "src is out of bounds");
2731+
let Range { start: src_start, end: src_end } = self.check_range(src);
26742732
let count = src_end - src_start;
26752733
assert!(dest <= self.len() - count, "dest is out of bounds");
26762734
// SAFETY: the conditions for `ptr::copy` have all been checked above,
@@ -3259,7 +3317,14 @@ fn slice_index_order_fail(index: usize, end: usize) -> ! {
32593317
#[inline(never)]
32603318
#[cold]
32613319
#[track_caller]
3262-
fn slice_index_overflow_fail() -> ! {
3320+
fn slice_start_index_overflow_fail() -> ! {
3321+
panic!("attempted to index slice from after maximum usize");
3322+
}
3323+
3324+
#[inline(never)]
3325+
#[cold]
3326+
#[track_caller]
3327+
fn slice_end_index_overflow_fail() -> ! {
32633328
panic!("attempted to index slice up to maximum usize");
32643329
}
32653330

@@ -3603,15 +3668,15 @@ unsafe impl<T> SliceIndex<[T]> for ops::RangeInclusive<usize> {
36033668
#[inline]
36043669
fn index(self, slice: &[T]) -> &[T] {
36053670
if *self.end() == usize::MAX {
3606-
slice_index_overflow_fail();
3671+
slice_end_index_overflow_fail();
36073672
}
36083673
(*self.start()..self.end() + 1).index(slice)
36093674
}
36103675

36113676
#[inline]
36123677
fn index_mut(self, slice: &mut [T]) -> &mut [T] {
36133678
if *self.end() == usize::MAX {
3614-
slice_index_overflow_fail();
3679+
slice_end_index_overflow_fail();
36153680
}
36163681
(*self.start()..self.end() + 1).index_mut(slice)
36173682
}

library/core/tests/slice.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1795,7 +1795,7 @@ fn test_copy_within() {
17951795
}
17961796

17971797
#[test]
1798-
#[should_panic(expected = "src is out of bounds")]
1798+
#[should_panic(expected = "range end index 14 out of range for slice of length 13")]
17991799
fn test_copy_within_panics_src_too_long() {
18001800
let mut bytes = *b"Hello, World!";
18011801
// The length is only 13, so 14 is out of bounds.
@@ -1810,7 +1810,7 @@ fn test_copy_within_panics_dest_too_long() {
18101810
bytes.copy_within(0..4, 10);
18111811
}
18121812
#[test]
1813-
#[should_panic(expected = "src end is before src start")]
1813+
#[should_panic(expected = "slice index starts at 2 but ends at 1")]
18141814
fn test_copy_within_panics_src_inverted() {
18151815
let mut bytes = *b"Hello, World!";
18161816
// 2 is greater than 1, so this range is invalid.

src/tools/linkchecker/main.rs

+1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ const LINKCHECK_EXCEPTIONS: &[(&str, &[&str])] = &[
3939
"#method.sort_by_key",
4040
"#method.make_ascii_uppercase",
4141
"#method.make_ascii_lowercase",
42+
"#method.get_unchecked_mut",
4243
],
4344
),
4445
// These try to link to std::collections, but are defined in alloc

0 commit comments

Comments
 (0)