From 625b18027d4d8d9b2092cf3706b29a5812f76357 Mon Sep 17 00:00:00 2001 From: Karl Meakin Date: Fri, 25 Jul 2025 19:51:21 +0100 Subject: [PATCH] Optimize `SliceIndex::get` impl for `RangeInclusive` The checks for `self.end() == usize::MAX` and `self.end() + 1 > slice.len()` can be replaced with `self.end() >= slice.len()`, since `self.end() < slice.len()` implies both `self.end() <= slice.len()` and `self.end() < usize::MAX`. --- library/core/src/slice/index.rs | 38 ++++++++++++---------- tests/codegen-llvm/slice-range-indexing.rs | 26 ++++++--------- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/library/core/src/slice/index.rs b/library/core/src/slice/index.rs index 31d9931e474a..3d769010ea1c 100644 --- a/library/core/src/slice/index.rs +++ b/library/core/src/slice/index.rs @@ -663,7 +663,6 @@ unsafe impl 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")] @@ -673,12 +672,12 @@ unsafe impl const SliceIndex<[T]> for ops::RangeInclusive { #[inline] fn get(self, slice: &[T]) -> Option<&[T]> { - 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 [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] @@ -950,8 +949,7 @@ where R: ops::RangeBounds, { let len = bounds.end; - let r = into_range(len, (range.start_bound().copied(), range.end_bound().copied()))?; - if r.start > r.end || r.end > len { None } else { Some(r) } + into_range(len, (range.start_bound().copied(), range.end_bound().copied())) } /// Converts a pair of `ops::Bound`s into `ops::Range` without performing any @@ -982,21 +980,27 @@ pub(crate) const fn into_range( len: usize, (start, end): (ops::Bound, ops::Bound), ) -> Option> { - 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) } diff --git a/tests/codegen-llvm/slice-range-indexing.rs b/tests/codegen-llvm/slice-range-indexing.rs index 5c47cab6234e..c4b3492dc4dc 100644 --- a/tests/codegen-llvm/slice-range-indexing.rs +++ b/tests/codegen-llvm/slice-range-indexing.rs @@ -33,17 +33,14 @@ macro_rules! tests { // CHECK: ret tests!(Range, get_range, index_range); -// 3 comparisons required: -// end != usize::MAX && end + 1 <= len && start <= end + 1 - -// CHECK-LABEL: @get_range_inclusive -// CHECK-COUNT-3: %{{.+}} = icmp -// CHECK-NOT: %{{.+}} = icmp -// CHECK: ret - // 2 comparisons required: // end < len && start <= end + 1 +// CHECK-LABEL: @get_range_inclusive +// CHECK-COUNT-2: %{{.+}} = icmp +// CHECK-NOT: %{{.+}} = icmp +// CHECK: ret + // CHECK-LABEL: @index_range_inclusive // CHECK-COUNT-2: %{{.+}} = icmp // CHECK-NOT: %{{.+}} = icmp @@ -64,17 +61,14 @@ tests!(RangeInclusive, get_range_inclusive, index_range_inclusive); // CHECK: ret tests!(RangeTo, get_range_to, index_range_to); -// 2 comparisons required: -// end != usize::MAX && end + 1 <= len - -// CHECK-LABEL: @get_range_to_inclusive -// CHECK-COUNT-2: %{{.+}} = icmp -// CHECK-NOT: %{{.+}} = icmp -// CHECK: ret - // 1 comparison required: // end < len +// CHECK-LABEL: @get_range_to_inclusive +// CHECK-COUNT-1: %{{.+}} = icmp +// CHECK-NOT: %{{.+}} = icmp +// CHECK: ret + // CHECK-LABEL: @index_range_to_inclusive // CHECK-COUNT-1: %{{.+}} = icmp // CHECK-NOT: %{{.+}} = icmp