Skip to content
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

Check for exhaustion in RangeInclusive::contains and slicing #78109

Merged
merged 3 commits into from
Oct 24, 2020
Merged
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
29 changes: 29 additions & 0 deletions library/alloc/tests/str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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] == "";
Expand All @@ -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";
Expand Down
32 changes: 31 additions & 1 deletion library/core/src/ops/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}
}

Expand Down
16 changes: 6 additions & 10 deletions library/core/src/slice/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
16 changes: 6 additions & 10 deletions library/core/src/str/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
30 changes: 30 additions & 0 deletions library/core/tests/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand All @@ -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];
Expand Down