Skip to content

Optimize indexing slices and strs with inclusive ranges #145024

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
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
4 changes: 2 additions & 2 deletions library/alloctests/tests/str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -631,13 +631,13 @@ mod slice_index {
// note: using 0 specifically ensures that the result of overflowing is 0..0,
// so that `get` doesn't simply return None for the wrong reason.
bad: data[0..=usize::MAX];
message: "maximum usize";
message: "out of bounds";
}

in mod rangetoinclusive {
data: "hello";
bad: data[..=usize::MAX];
message: "maximum usize";
message: "out of bounds";
}
}
}
Expand Down
9 changes: 0 additions & 9 deletions library/core/src/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -333,15 +333,6 @@ impl<Idx: Step> RangeInclusive<Idx> {
}
}

impl RangeInclusive<usize> {
/// Converts to an exclusive `Range` for `SliceIndex` implementations.
/// The caller is responsible for dealing with `end == usize::MAX`.
#[inline]
pub(crate) const fn into_slice_range(self) -> Range<usize> {
Range { start: self.start, end: self.end + 1 }
}
}

#[unstable(feature = "new_range_api", issue = "125687")]
impl<T> RangeBounds<T> for RangeInclusive<T> {
fn start_bound(&self) -> Bound<&T> {
Expand Down
64 changes: 41 additions & 23 deletions library/core/src/slice/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,6 @@ unsafe impl<T> const SliceIndex<[T]> for ops::RangeFull {
}

/// The methods `index` and `index_mut` panic if:
/// - the end of the range is `usize::MAX` or
/// - the start of the range is greater than the end of the range or
/// - the end of the range is out of bounds.
#[stable(feature = "inclusive_range", since = "1.26.0")]
Expand All @@ -660,12 +659,14 @@ unsafe impl<T> const SliceIndex<[T]> for ops::RangeInclusive<usize> {

#[inline]
fn get(self, slice: &[T]) -> Option<&[T]> {
if *self.end() == usize::MAX { None } else { self.into_slice_range().get(slice) }
// `self.into_slice_range()` cannot overflow, because `*self.end() <
// slice.len()` implies `*self.end() < usize::MAX`.
if *self.end() >= slice.len() { None } else { self.into_slice_range().get(slice) }
}

#[inline]
fn get_mut(self, slice: &mut [T]) -> Option<&mut [T]> {
if *self.end() == usize::MAX { None } else { self.into_slice_range().get_mut(slice) }
if *self.end() >= slice.len() { None } else { self.into_slice_range().get_mut(slice) }
}

#[inline]
Expand Down Expand Up @@ -865,6 +866,7 @@ where

ops::Bound::Excluded(&end) if end > len => slice_index_fail(0, end, len),
ops::Bound::Excluded(&end) => end,

ops::Bound::Unbounded => len,
};

Expand Down Expand Up @@ -920,19 +922,29 @@ where
{
let len = bounds.end;

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

let end = match range.end_bound() {
ops::Bound::Included(end) => end.checked_add(1)?,
ops::Bound::Included(&end) if end >= len => return None,
// Cannot overflow because `end < len` implies `end < usize::MAX`.
ops::Bound::Included(end) => end + 1,

ops::Bound::Excluded(&end) if end > len => return None,
ops::Bound::Excluded(&end) => end,

ops::Bound::Unbounded => len,
};

if start > end || end > len { None } else { Some(ops::Range { start, end }) }
let start = match range.start_bound() {
ops::Bound::Excluded(&start) if start >= end => return None,
// Cannot overflow because `start < end` implies `start < usize::MAX`.
ops::Bound::Excluded(&start) => start + 1,

ops::Bound::Included(&start) if start > end => return None,
ops::Bound::Included(&start) => start,

ops::Bound::Unbounded => 0,
};

Some(ops::Range { start, end })
}

/// Converts a pair of `ops::Bound`s into `ops::Range` without performing any
Expand Down Expand Up @@ -961,21 +973,27 @@ pub(crate) fn into_range(
len: usize,
(start, end): (ops::Bound<usize>, ops::Bound<usize>),
) -> Option<ops::Range<usize>> {
use ops::Bound;
let start = match start {
Bound::Included(start) => start,
Bound::Excluded(start) => start.checked_add(1)?,
Bound::Unbounded => 0,
};

let end = match end {
Bound::Included(end) => end.checked_add(1)?,
Bound::Excluded(end) => end,
Bound::Unbounded => len,
ops::Bound::Included(end) if end >= len => return None,
// Cannot overflow because `end < len` implies `end < usize::MAX`.
ops::Bound::Included(end) => end + 1,

ops::Bound::Excluded(end) if end > len => return None,
ops::Bound::Excluded(end) => end,

ops::Bound::Unbounded => len,
};

// Don't bother with checking `start < end` and `end <= len`
// since these checks are handled by `Range` impls
let start = match start {
ops::Bound::Excluded(start) if start >= end => return None,
// Cannot overflow because `start < end` implies `start < usize::MAX`.
ops::Bound::Excluded(start) => start + 1,

ops::Bound::Included(start) if start > end => return None,
ops::Bound::Included(start) => start,

ops::Bound::Unbounded => 0,
};

Some(start..end)
}
Expand Down
2 changes: 1 addition & 1 deletion library/core/src/str/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ fn slice_error_fail_rt(s: &str, begin: usize, end: usize) -> ! {
let ellipsis = if trunc_len < s.len() { "[...]" } else { "" };

// 1. out of bounds
if begin > s.len() || end > s.len() {
if begin > s.len() || end > s.len() || (end == s.len() && s.is_char_boundary(begin)) {
let oob_index = if begin > s.len() { begin } else { end };
panic!("byte index {oob_index} is out of bounds of `{s_trunc}`{ellipsis}");
}
Expand Down
37 changes: 12 additions & 25 deletions library/core/src/str/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,6 @@ where
}
}

#[inline(never)]
#[cold]
#[track_caller]
const fn str_index_overflow_fail() -> ! {
panic!("attempted to index str up to maximum usize");
}

/// Implements substring slicing with syntax `&self[..]` or `&mut self[..]`.
///
/// Returns a slice of the whole string, i.e., returns `&self` or `&mut
Expand Down Expand Up @@ -639,11 +632,11 @@ unsafe impl const SliceIndex<str> for ops::RangeInclusive<usize> {
type Output = str;
#[inline]
fn get(self, slice: &str) -> Option<&Self::Output> {
if *self.end() == usize::MAX { None } else { self.into_slice_range().get(slice) }
if *self.end() >= slice.len() { None } else { self.into_slice_range().get(slice) }
}
#[inline]
fn get_mut(self, slice: &mut str) -> Option<&mut Self::Output> {
if *self.end() == usize::MAX { None } else { self.into_slice_range().get_mut(slice) }
if *self.end() >= slice.len() { None } else { self.into_slice_range().get_mut(slice) }
}
#[inline]
unsafe fn get_unchecked(self, slice: *const str) -> *const Self::Output {
Expand All @@ -657,15 +650,15 @@ unsafe impl const SliceIndex<str> for ops::RangeInclusive<usize> {
}
#[inline]
fn index(self, slice: &str) -> &Self::Output {
if *self.end() == usize::MAX {
str_index_overflow_fail();
if *self.end() >= slice.len() {
super::slice_error_fail(slice, *self.start(), *self.end());
}
self.into_slice_range().index(slice)
}
#[inline]
fn index_mut(self, slice: &mut str) -> &mut Self::Output {
if *self.end() == usize::MAX {
str_index_overflow_fail();
if *self.end() >= slice.len() {
super::slice_error_fail(slice, *self.start(), *self.end());
}
self.into_slice_range().index_mut(slice)
}
Expand All @@ -677,35 +670,29 @@ unsafe impl const SliceIndex<str> for range::RangeInclusive<usize> {
type Output = str;
#[inline]
fn get(self, slice: &str) -> Option<&Self::Output> {
if self.end == usize::MAX { None } else { self.into_slice_range().get(slice) }
ops::RangeInclusive::from(self).get(slice)
}
#[inline]
fn get_mut(self, slice: &mut str) -> Option<&mut Self::Output> {
if self.end == usize::MAX { None } else { self.into_slice_range().get_mut(slice) }
ops::RangeInclusive::from(self).get_mut(slice)
}
#[inline]
unsafe fn get_unchecked(self, slice: *const str) -> *const Self::Output {
// SAFETY: the caller must uphold the safety contract for `get_unchecked`.
unsafe { self.into_slice_range().get_unchecked(slice) }
unsafe { ops::RangeInclusive::from(self).get_unchecked(slice) }
}
#[inline]
unsafe fn get_unchecked_mut(self, slice: *mut str) -> *mut Self::Output {
// SAFETY: the caller must uphold the safety contract for `get_unchecked_mut`.
unsafe { self.into_slice_range().get_unchecked_mut(slice) }
unsafe { ops::RangeInclusive::from(self).get_unchecked_mut(slice) }
}
#[inline]
fn index(self, slice: &str) -> &Self::Output {
if self.end == usize::MAX {
str_index_overflow_fail();
}
self.into_slice_range().index(slice)
ops::RangeInclusive::from(self).index(slice)
}
#[inline]
fn index_mut(self, slice: &mut str) -> &mut Self::Output {
if self.end == usize::MAX {
str_index_overflow_fail();
}
self.into_slice_range().index_mut(slice)
ops::RangeInclusive::from(self).index_mut(slice)
}
}

Expand Down
Loading