Rollup merge of #78109 - cuviper:exhausted-rangeinc, r=dtolnay

Check for exhaustion in RangeInclusive::contains and slicing When a range has finished iteration, `is_empty` returns true, so it should also be the case that `contains` returns false. Fixes #77941.
rust-lang · Oct 24, 2020 · d9acd7d · d9acd7d
2 parents fb92b70 + 9202fbd
commit d9acd7d
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 21 deletions.
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 {
@@ -479,6 +493,16 @@ impl<Idx: PartialOrd<Idx>> RangeInclusive<Idx> {
     /// assert!(!(0.0..=f32::NAN).contains(&0.0));
     /// assert!(!(f32::NAN..=1.0).contains(&1.0));
     /// ```
+    ///
+    /// This method always returns `false` after iteration has finished:
+    ///
+    /// ```
+    /// let mut r = 3..=5;
+    /// assert!(r.contains(&3) && r.contains(&5));
+    /// for _ in r.by_ref() {}
+    /// // Precise field values are unspecified here
+    /// assert!(!r.contains(&3) && !r.contains(&5));
+    /// ```
     #[stable(feature = "range_contains", since = "1.35.0")]
     pub fn contains<U>(&self, item: &U) -> bool
     where
@@ -881,7 +905,13 @@ impl<T> RangeBounds<T> for RangeInclusive<T> {
         Included(&self.start)
     }
     fn end_bound(&self) -> Bound<&T> {
-        Included(&self.end)
+        if self.exhausted {
+            // When the iterator is exhausted, we usually have start == end,
+            // but we want the range to appear empty, containing nothing.
+            Excluded(&self.end)
+        } else {
+            Included(&self.end)
+        }
     }
 }
 

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];