Check for exhaustion in SliceIndex for RangeInclusive

cuviper · cuviper · commit 9202fbdbdbd6 · 2020-10-20T17:18:08.000-07:00
diff --git a/library/alloc/tests/str.rs b/library/alloc/tests/str.rs
@@ -529,6 +529,13 @@ mod slice_index {
             message: "out of bounds";
         }
 
+        in mod rangeinclusive_len {
+            data: "abcdef";
+            good: data[0..=5] == "abcdef";
+            bad: data[0..=6];
+            message: "out of bounds";
+        }
+
         in mod range_len_len {
             data: "abcdef";
             good: data[6..6] == "";
@@ -544,6 +551,28 @@ mod slice_index {
         }
     }
 
+    panic_cases! {
+        in mod rangeinclusive_exhausted {
+            data: "abcdef";
+
+            good: data[0..=5] == "abcdef";
+            good: data[{
+                let mut iter = 0..=5;
+                iter.by_ref().count(); // exhaust it
+                iter
+            }] == "";
+
+            // 0..=6 is out of bounds before exhaustion, so it
+            // stands to reason that it still would be after.
+            bad: data[{
+                let mut iter = 0..=6;
+                iter.by_ref().count(); // exhaust it
+                iter
+            }];
+            message: "out of bounds";
+        }
+    }
+
     panic_cases! {
         in mod range_neg_width {
             data: "abcdef";
diff --git a/library/core/src/ops/range.rs b/library/core/src/ops/range.rs
@@ -446,6 +446,20 @@ impl<Idx> 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) fn into_slice_range(self) -> Range<usize> {
+        // If we're not exhausted, we want to simply slice `start..end + 1`.
+        // If we are exhausted, then slicing with `end + 1..end + 1` gives us an
+        // empty range that is still subject to bounds-checks for that endpoint.
+        let exclusive_end = self.end + 1;
+        let start = if self.exhausted { exclusive_end } else { self.start };
+        start..exclusive_end
+    }
+}
+
 #[stable(feature = "inclusive_range", since = "1.26.0")]
 impl<Idx: fmt::Debug> fmt::Debug for RangeInclusive<Idx> {
     fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
diff --git a/library/core/src/slice/index.rs b/library/core/src/slice/index.rs
@@ -376,44 +376,40 @@ unsafe impl<T> SliceIndex<[T]> for ops::RangeInclusive<usize> {
 
     #[inline]
     fn get(self, slice: &[T]) -> Option<&[T]> {
-        if *self.end() == usize::MAX { None } else { (*self.start()..self.end() + 1).get(slice) }
+        if *self.end() == usize::MAX { 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.start()..self.end() + 1).get_mut(slice)
-        }
+        if *self.end() == usize::MAX { None } else { self.into_slice_range().get_mut(slice) }
     }
 
     #[inline]
     unsafe fn get_unchecked(self, slice: *const [T]) -> *const [T] {
         // SAFETY: the caller has to uphold the safety contract for `get_unchecked`.
-        unsafe { (*self.start()..self.end() + 1).get_unchecked(slice) }
+        unsafe { self.into_slice_range().get_unchecked(slice) }
     }
 
     #[inline]
     unsafe fn get_unchecked_mut(self, slice: *mut [T]) -> *mut [T] {
         // SAFETY: the caller has to uphold the safety contract for `get_unchecked_mut`.
-        unsafe { (*self.start()..self.end() + 1).get_unchecked_mut(slice) }
+        unsafe { self.into_slice_range().get_unchecked_mut(slice) }
     }
 
     #[inline]
     fn index(self, slice: &[T]) -> &[T] {
         if *self.end() == usize::MAX {
             slice_end_index_overflow_fail();
         }
-        (*self.start()..self.end() + 1).index(slice)
+        self.into_slice_range().index(slice)
     }
 
     #[inline]
     fn index_mut(self, slice: &mut [T]) -> &mut [T] {
         if *self.end() == usize::MAX {
             slice_end_index_overflow_fail();
         }
-        (*self.start()..self.end() + 1).index_mut(slice)
+        self.into_slice_range().index_mut(slice)
     }
 }
 
diff --git a/library/core/src/str/traits.rs b/library/core/src/str/traits.rs
@@ -398,39 +398,35 @@ unsafe impl 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.start()..self.end() + 1).get(slice) }
+        if *self.end() == usize::MAX { 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.start()..self.end() + 1).get_mut(slice)
-        }
+        if *self.end() == usize::MAX { None } else { self.into_slice_range().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.start()..self.end() + 1).get_unchecked(slice) }
+        unsafe { self.into_slice_range().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.start()..self.end() + 1).get_unchecked_mut(slice) }
+        unsafe { self.into_slice_range().get_unchecked_mut(slice) }
     }
     #[inline]
     fn index(self, slice: &str) -> &Self::Output {
         if *self.end() == usize::MAX {
             str_index_overflow_fail();
         }
-        (*self.start()..self.end() + 1).index(slice)
+        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();
         }
-        (*self.start()..self.end() + 1).index_mut(slice)
+        self.into_slice_range().index_mut(slice)
     }
 }
 
diff --git a/library/core/tests/slice.rs b/library/core/tests/slice.rs
@@ -1341,6 +1341,14 @@ mod slice_index {
             message: "out of range";
         }
 
+        in mod rangeinclusive_len {
+            data: [0, 1, 2, 3, 4, 5];
+
+            good: data[0..=5] == [0, 1, 2, 3, 4, 5];
+            bad: data[0..=6];
+            message: "out of range";
+        }
+
         in mod range_len_len {
             data: [0, 1, 2, 3, 4, 5];
 
@@ -1358,6 +1366,28 @@ mod slice_index {
         }
     }
 
+    panic_cases! {
+        in mod rangeinclusive_exhausted {
+            data: [0, 1, 2, 3, 4, 5];
+
+            good: data[0..=5] == [0, 1, 2, 3, 4, 5];
+            good: data[{
+                let mut iter = 0..=5;
+                iter.by_ref().count(); // exhaust it
+                iter
+            }] == [];
+
+            // 0..=6 is out of range before exhaustion, so it
+            // stands to reason that it still would be after.
+            bad: data[{
+                let mut iter = 0..=6;
+                iter.by_ref().count(); // exhaust it
+                iter
+            }];
+            message: "out of range";
+        }
+    }
+
     panic_cases! {
         in mod range_neg_width {
             data: [0, 1, 2, 3, 4, 5];

Original file line number	Diff line number	Diff line change
`@@ -376,44 +376,40 @@ unsafe impl<T> SliceIndex<[T]> for ops::RangeInclusive<usize> {`
`376`	`376`
`377`	`377`	`#[inline]`
`378`	`378`	`fn get(self, slice: &[T]) -> Option<&[T]> {`
`379`		`- if self.end() == usize::MAX { None } else { (self.start()..self.end() + 1).get(slice) }`
	`379`	`+ if *self.end() == usize::MAX { None } else { self.into_slice_range().get(slice) }`
`380`	`380`	`}`
`381`	`381`
`382`	`382`	`#[inline]`
`383`	`383`	`fn get_mut(self, slice: &mut [T]) -> Option<&mut [T]> {`
`384`		`- if *self.end() == usize::MAX {`
`385`		`- None`
`386`		`- } else {`
`387`		`- (*self.start()..self.end() + 1).get_mut(slice)`
`388`		`- }`
	`384`	`+ if *self.end() == usize::MAX { None } else { self.into_slice_range().get_mut(slice) }`
`389`	`385`	`}`
`390`	`386`
`391`	`387`	`#[inline]`
`392`	`388`	`unsafe fn get_unchecked(self, slice: const [T]) -> const [T] {`
`393`	`389`	// SAFETY: the caller has to uphold the safety contract for `get_unchecked`.
`394`		`- unsafe { (*self.start()..self.end() + 1).get_unchecked(slice) }`
	`390`	`+ unsafe { self.into_slice_range().get_unchecked(slice) }`
`395`	`391`	`}`
`396`	`392`
`397`	`393`	`#[inline]`
`398`	`394`	`unsafe fn get_unchecked_mut(self, slice: mut [T]) -> mut [T] {`
`399`	`395`	// SAFETY: the caller has to uphold the safety contract for `get_unchecked_mut`.
`400`		`- unsafe { (*self.start()..self.end() + 1).get_unchecked_mut(slice) }`
	`396`	`+ unsafe { self.into_slice_range().get_unchecked_mut(slice) }`
`401`	`397`	`}`
`402`	`398`
`403`	`399`	`#[inline]`
`404`	`400`	`fn index(self, slice: &[T]) -> &[T] {`
`405`	`401`	`if *self.end() == usize::MAX {`
`406`	`402`	`slice_end_index_overflow_fail();`
`407`	`403`	`}`
`408`		`- (*self.start()..self.end() + 1).index(slice)`
	`404`	`+ self.into_slice_range().index(slice)`
`409`	`405`	`}`
`410`	`406`
`411`	`407`	`#[inline]`
`412`	`408`	`fn index_mut(self, slice: &mut [T]) -> &mut [T] {`
`413`	`409`	`if *self.end() == usize::MAX {`
`414`	`410`	`slice_end_index_overflow_fail();`
`415`	`411`	`}`
`416`		`- (*self.start()..self.end() + 1).index_mut(slice)`
	`412`	`+ self.into_slice_range().index_mut(slice)`
`417`	`413`	`}`
`418`	`414`	`}`
`419`	`415`
Original file line number	Diff line number	Diff line change
`@@ -398,39 +398,35 @@ unsafe impl SliceIndex<str> for ops::RangeInclusive<usize> {`
`398`	`398`	`type Output = str;`
`399`	`399`	`#[inline]`
`400`	`400`	`fn get(self, slice: &str) -> Option<&Self::Output> {`
`401`		`- if self.end() == usize::MAX { None } else { (self.start()..self.end() + 1).get(slice) }`
	`401`	`+ if *self.end() == usize::MAX { None } else { self.into_slice_range().get(slice) }`
`402`	`402`	`}`
`403`	`403`	`#[inline]`
`404`	`404`	`fn get_mut(self, slice: &mut str) -> Option<&mut Self::Output> {`
`405`		`- if *self.end() == usize::MAX {`
`406`		`- None`
`407`		`- } else {`
`408`		`- (*self.start()..self.end() + 1).get_mut(slice)`
`409`		`- }`
	`405`	`+ if *self.end() == usize::MAX { None } else { self.into_slice_range().get_mut(slice) }`
`410`	`406`	`}`
`411`	`407`	`#[inline]`
`412`	`408`	`unsafe fn get_unchecked(self, slice: const str) -> const Self::Output {`
`413`	`409`	// SAFETY: the caller must uphold the safety contract for `get_unchecked`.
`414`		`- unsafe { (*self.start()..self.end() + 1).get_unchecked(slice) }`
	`410`	`+ unsafe { self.into_slice_range().get_unchecked(slice) }`
`415`	`411`	`}`
`416`	`412`	`#[inline]`
`417`	`413`	`unsafe fn get_unchecked_mut(self, slice: mut str) -> mut Self::Output {`
`418`	`414`	// SAFETY: the caller must uphold the safety contract for `get_unchecked_mut`.
`419`		`- unsafe { (*self.start()..self.end() + 1).get_unchecked_mut(slice) }`
	`415`	`+ unsafe { self.into_slice_range().get_unchecked_mut(slice) }`
`420`	`416`	`}`
`421`	`417`	`#[inline]`
`422`	`418`	`fn index(self, slice: &str) -> &Self::Output {`
`423`	`419`	`if *self.end() == usize::MAX {`
`424`	`420`	`str_index_overflow_fail();`
`425`	`421`	`}`
`426`		`- (*self.start()..self.end() + 1).index(slice)`
	`422`	`+ self.into_slice_range().index(slice)`
`427`	`423`	`}`
`428`	`424`	`#[inline]`
`429`	`425`	`fn index_mut(self, slice: &mut str) -> &mut Self::Output {`
`430`	`426`	`if *self.end() == usize::MAX {`
`431`	`427`	`str_index_overflow_fail();`
`432`	`428`	`}`
`433`		`- (*self.start()..self.end() + 1).index_mut(slice)`
	`429`	`+ self.into_slice_range().index_mut(slice)`
`434`	`430`	`}`
`435`	`431`	`}`
`436`	`432`