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.

Sign up for GitHub

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

Jump to bottom

split_array: remove runtime panics #109049

Closed
wants to merge 4 commits into from
Closed

split_array: remove runtime panics #109049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8510cf6
Update slice::split_array_*() methods to never panic
Xiretza Nov 8, 2022
1fc0dc1
Update array::split_array_*() methods to never panic
Xiretza May 21, 2023
ddbfc0f
Make split_array_*() methods const
Xiretza Mar 12, 2023
34356db
core: fix indentation in split_array doc comments
Xiretza May 21, 2023
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
52 changes: 22 additions & 30 deletions library/core/src/array/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,10 +631,6 @@ impl<T, const N: usize> [T; N] {
/// the index `M` itself) and the second will contain all
/// indices from `[M, N)` (excluding the index `N` itself).
///
/// # Panics
///
/// Panics if `M > N`.
///
/// # Examples
///
/// ```
Expand All @@ -643,9 +639,9 @@ impl<T, const N: usize> [T; N] {
/// let v = [1, 2, 3, 4, 5, 6];
///
/// {
/// let (left, right) = v.split_array_ref::<0>();
/// assert_eq!(left, &[]);
/// assert_eq!(right, &[1, 2, 3, 4, 5, 6]);
/// let (left, right) = v.split_array_ref::<0>();
/// assert_eq!(left, &[]);
/// assert_eq!(right, &[1, 2, 3, 4, 5, 6]);
/// }
///
/// {
Expand All @@ -666,8 +662,10 @@ impl<T, const N: usize> [T; N] {
issue = "90091"
)]
#[inline]
pub fn split_array_ref<const M: usize>(&self) -> (&[T; M], &[T]) {
(&self[..]).split_array_ref::<M>()
pub const fn split_array_ref<const M: usize>(&self) -> (&[T; M], &[T]) {
// FIXME: remove once constraint is encoded in return type
const { assert!(M <= N) }
self.as_slice().split_array_ref::<M>().unwrap()
}

/// Divides one mutable array reference into two at an index.
Expand All @@ -676,10 +674,6 @@ impl<T, const N: usize> [T; N] {
/// the index `M` itself) and the second will contain all
/// indices from `[M, N)` (excluding the index `N` itself).
///
/// # Panics
///
/// Panics if `M > N`.
///
/// # Examples
///
/// ```
Expand All @@ -699,8 +693,10 @@ impl<T, const N: usize> [T; N] {
issue = "90091"
)]
#[inline]
pub fn split_array_mut<const M: usize>(&mut self) -> (&mut [T; M], &mut [T]) {
(&mut self[..]).split_array_mut::<M>()
pub const fn split_array_mut<const M: usize>(&mut self) -> (&mut [T; M], &mut [T]) {
// FIXME: remove once constraint is encoded in return type
const { assert!(M <= N) }
(self as &mut [T]).split_array_mut::<M>().unwrap()
}

/// Divides one array reference into two at an index from the end.
Expand All @@ -709,10 +705,6 @@ impl<T, const N: usize> [T; N] {
/// the index `N - M` itself) and the second will contain all
/// indices from `[N - M, N)` (excluding the index `N` itself).
///
/// # Panics
///
/// Panics if `M > N`.
///
/// # Examples
///
/// ```
Expand All @@ -721,9 +713,9 @@ impl<T, const N: usize> [T; N] {
/// let v = [1, 2, 3, 4, 5, 6];
///
/// {
/// let (left, right) = v.rsplit_array_ref::<0>();
/// assert_eq!(left, &[1, 2, 3, 4, 5, 6]);
/// assert_eq!(right, &[]);
/// let (left, right) = v.rsplit_array_ref::<0>();
/// assert_eq!(left, &[1, 2, 3, 4, 5, 6]);
/// assert_eq!(right, &[]);
/// }
///
/// {
Expand All @@ -744,8 +736,10 @@ impl<T, const N: usize> [T; N] {
issue = "90091"
)]
#[inline]
pub fn rsplit_array_ref<const M: usize>(&self) -> (&[T], &[T; M]) {
(&self[..]).rsplit_array_ref::<M>()
pub const fn rsplit_array_ref<const M: usize>(&self) -> (&[T], &[T; M]) {
// FIXME: remove once constraint is encoded in return type
const { assert!(M <= N) }
self.as_slice().rsplit_array_ref::<M>().unwrap()
}

/// Divides one mutable array reference into two at an index from the end.
Expand All @@ -754,10 +748,6 @@ impl<T, const N: usize> [T; N] {
/// the index `N - M` itself) and the second will contain all
/// indices from `[N - M, N)` (excluding the index `N` itself).
///
/// # Panics
///
/// Panics if `M > N`.
///
/// # Examples
///
/// ```
Expand All @@ -777,8 +767,10 @@ impl<T, const N: usize> [T; N] {
issue = "90091"
)]
#[inline]
pub fn rsplit_array_mut<const M: usize>(&mut self) -> (&mut [T], &mut [T; M]) {
(&mut self[..]).rsplit_array_mut::<M>()
pub const fn rsplit_array_mut<const M: usize>(&mut self) -> (&mut [T], &mut [T; M]) {
// FIXME: remove once constraint is encoded in return type
const { assert!(M <= N) }
(self as &mut [T]).rsplit_array_mut::<M>().unwrap()
}
}

Expand Down
102 changes: 60 additions & 42 deletions library/core/src/slice/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1764,9 +1764,7 @@ impl<T> [T] {
/// the index `N` itself) and the slice will contain all
/// indices from `[N, len)` (excluding the index `len` itself).
///
/// # Panics
///
/// Panics if `N > len`.
/// Returns `None` if the slice has less than `N` elements.
///
/// # Examples
///
Expand All @@ -1776,31 +1774,38 @@ impl<T> [T] {
/// let v = &[1, 2, 3, 4, 5, 6][..];
///
/// {
/// let (left, right) = v.split_array_ref::<0>();
/// assert_eq!(left, &[]);
/// assert_eq!(right, [1, 2, 3, 4, 5, 6]);
/// let (left, right) = v.split_array_ref::<0>().unwrap();
/// assert_eq!(left, &[]);
/// assert_eq!(right, [1, 2, 3, 4, 5, 6]);
/// }
///
/// {
/// let (left, right) = v.split_array_ref::<2>();
/// let (left, right) = v.split_array_ref::<2>().unwrap();
/// assert_eq!(left, &[1, 2]);
/// assert_eq!(right, [3, 4, 5, 6]);
/// }
///
/// {
/// let (left, right) = v.split_array_ref::<6>();
/// let (left, right) = v.split_array_ref::<6>().unwrap();
/// assert_eq!(left, &[1, 2, 3, 4, 5, 6]);
/// assert_eq!(right, []);
/// }
///
/// assert!(v.split_array_ref::<7>().is_none());
/// ```
#[unstable(feature = "split_array", reason = "new API", issue = "90091")]
#[inline]
#[track_caller]
#[must_use]
pub fn split_array_ref<const N: usize>(&self) -> (&[T; N], &[T]) {
let (a, b) = self.split_at(N);
// SAFETY: a points to [T; N]? Yes it's [T] of length N (checked by split_at)
unsafe { (&*(a.as_ptr() as *const [T; N]), b) }
pub const fn split_array_ref<const N: usize>(&self) -> Option<(&[T; N], &[T])> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to instead have split_array_ref (panicking) and try_split_array_ref (fallible)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would blow up the method count to 8; I don't think adding unwrap()/except() is that big of a hurdle.

if N > self.len() {
None
} else {
// SAFETY: 0 <= N <= len
let (a, b) = unsafe { self.split_at_unchecked(N) };
// SAFETY: a points to [T; N]? Yes it's [T] of length N (checked by split_at)
Some(unsafe { (&*(a.as_ptr() as *const [T; N]), b) })
}
}

/// Divides one mutable slice into an array and a remainder slice at an index.
Expand All @@ -1809,31 +1814,36 @@ impl<T> [T] {
/// the index `N` itself) and the slice will contain all
/// indices from `[N, len)` (excluding the index `len` itself).
///
/// # Panics
///
/// Panics if `N > len`.
/// Returns `None` if the slice has less than `N` elements.
///
/// # Examples
///
/// ```
/// #![feature(split_array)]
///
/// let mut v = &mut [1, 0, 3, 0, 5, 6][..];
/// let (left, right) = v.split_array_mut::<2>();
/// let (left, right) = v.split_array_mut::<2>().unwrap();
/// assert_eq!(left, &mut [1, 0]);
/// assert_eq!(right, [3, 0, 5, 6]);
/// left[1] = 2;
/// right[1] = 4;
/// assert_eq!(v, [1, 2, 3, 4, 5, 6]);
///
/// assert!(v.split_array_mut::<7>().is_none());
/// ```
#[unstable(feature = "split_array", reason = "new API", issue = "90091")]
#[inline]
#[track_caller]
#[must_use]
pub fn split_array_mut<const N: usize>(&mut self) -> (&mut [T; N], &mut [T]) {
let (a, b) = self.split_at_mut(N);
// SAFETY: a points to [T; N]? Yes it's [T] of length N (checked by split_at_mut)
unsafe { (&mut *(a.as_mut_ptr() as *mut [T; N]), b) }
pub const fn split_array_mut<const N: usize>(&mut self) -> Option<(&mut [T; N], &mut [T])> {
if N > self.len() {
None
} else {
// SAFETY: 0 <= N <= len
let (a, b) = unsafe { self.split_at_mut_unchecked(N) };
// SAFETY: a points to [T; N]? Yes it's [T] of length N (checked by split_at)
Some(unsafe { (&mut *(a.as_mut_ptr() as *mut [T; N]), b) })
}
}

/// Divides one slice into an array and a remainder slice at an index from
Expand All @@ -1843,9 +1853,7 @@ impl<T> [T] {
/// the index `len - N` itself) and the array will contain all
/// indices from `[len - N, len)` (excluding the index `len` itself).
///
/// # Panics
///
/// Panics if `N > len`.
/// Returns `None` if the slice has less than `N` elements.
///
/// # Examples
///
Expand All @@ -1855,31 +1863,37 @@ impl<T> [T] {
/// let v = &[1, 2, 3, 4, 5, 6][..];
///
/// {
/// let (left, right) = v.rsplit_array_ref::<0>();
/// assert_eq!(left, [1, 2, 3, 4, 5, 6]);
/// assert_eq!(right, &[]);
/// let (left, right) = v.rsplit_array_ref::<0>().unwrap();
/// assert_eq!(left, [1, 2, 3, 4, 5, 6]);
/// assert_eq!(right, &[]);
/// }
///
/// {
/// let (left, right) = v.rsplit_array_ref::<2>();
/// let (left, right) = v.rsplit_array_ref::<2>().unwrap();
/// assert_eq!(left, [1, 2, 3, 4]);
/// assert_eq!(right, &[5, 6]);
/// }
///
/// {
/// let (left, right) = v.rsplit_array_ref::<6>();
/// let (left, right) = v.rsplit_array_ref::<6>().unwrap();
/// assert_eq!(left, []);
/// assert_eq!(right, &[1, 2, 3, 4, 5, 6]);
/// }
///
/// assert!(v.rsplit_array_ref::<7>().is_none());
/// ```
#[unstable(feature = "split_array", reason = "new API", issue = "90091")]
#[inline]
#[must_use]
pub fn rsplit_array_ref<const N: usize>(&self) -> (&[T], &[T; N]) {
assert!(N <= self.len());
let (a, b) = self.split_at(self.len() - N);
// SAFETY: b points to [T; N]? Yes it's [T] of length N (checked by split_at)
unsafe { (a, &*(b.as_ptr() as *const [T; N])) }
pub const fn rsplit_array_ref<const N: usize>(&self) -> Option<(&[T], &[T; N])> {
if N > self.len() {
None
} else {
// SAFETY: N <= len; thus 0 <= len - N <= len
let (a, b) = unsafe { self.split_at_unchecked(self.len() - N) };
// SAFETY: b points to [T; N]? Yes it's [T] of length N (checked by split_at)
Some(unsafe { (a, &*(b.as_ptr() as *const [T; N])) })
}
}

/// Divides one mutable slice into an array and a remainder slice at an
Expand All @@ -1889,31 +1903,35 @@ impl<T> [T] {
/// the index `N` itself) and the array will contain all
/// indices from `[len - N, len)` (excluding the index `len` itself).
///
/// # Panics
///
/// Panics if `N > len`.
/// Returns `None` if the slice has less than `N` elements.
///
/// # Examples
///
/// ```
/// #![feature(split_array)]
///
/// let mut v = &mut [1, 0, 3, 0, 5, 6][..];
/// let (left, right) = v.rsplit_array_mut::<4>();
/// let (left, right) = v.rsplit_array_mut::<4>().unwrap();
/// assert_eq!(left, [1, 0]);
/// assert_eq!(right, &mut [3, 0, 5, 6]);
/// left[1] = 2;
/// right[1] = 4;
/// assert_eq!(v, [1, 2, 3, 4, 5, 6]);
///
/// assert!(v.rsplit_array_mut::<7>().is_none());
/// ```
#[unstable(feature = "split_array", reason = "new API", issue = "90091")]
#[inline]
#[must_use]
pub fn rsplit_array_mut<const N: usize>(&mut self) -> (&mut [T], &mut [T; N]) {
assert!(N <= self.len());
let (a, b) = self.split_at_mut(self.len() - N);
// SAFETY: b points to [T; N]? Yes it's [T] of length N (checked by split_at_mut)
unsafe { (a, &mut *(b.as_mut_ptr() as *mut [T; N])) }
pub const fn rsplit_array_mut<const N: usize>(&mut self) -> Option<(&mut [T], &mut [T; N])> {
if N > self.len() {
None
} else {
// SAFETY: N <= len; thus 0 <= len - N <= len
let (a, b) = unsafe { self.split_at_mut_unchecked(self.len() - N) };
// SAFETY: b points to [T; N]? Yes it's [T] of length N (checked by split_at_mut)
Some(unsafe { (a, &mut *(b.as_mut_ptr() as *mut [T; N])) })
}
}

/// Returns an iterator over subslices separated by elements that match
Expand Down
32 changes: 0 additions & 32 deletions library/core/tests/array.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,38 +488,6 @@ fn array_rsplit_array_mut() {
}
}

#[should_panic]
#[test]
fn array_split_array_ref_out_of_bounds() {
let v = [1, 2, 3, 4, 5, 6];

v.split_array_ref::<7>();
}

#[should_panic]
#[test]
fn array_split_array_mut_out_of_bounds() {
let mut v = [1, 2, 3, 4, 5, 6];

v.split_array_mut::<7>();
}

#[should_panic]
#[test]
fn array_rsplit_array_ref_out_of_bounds() {
let v = [1, 2, 3, 4, 5, 6];

v.rsplit_array_ref::<7>();
}

#[should_panic]
#[test]
fn array_rsplit_array_mut_out_of_bounds() {
let mut v = [1, 2, 3, 4, 5, 6];

v.rsplit_array_mut::<7>();
}

#[test]
fn array_intoiter_advance_by() {
use std::cell::Cell;
Expand Down
Loading