From 30dc32b10eb53e4a92c61a42062983db58838217 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Fri, 28 Aug 2020 08:55:41 +0300 Subject: [PATCH 1/3] Add (non-public) slice::split_at_unchecked() and split_at_mut_unchecked() These are unsafe variants of the non-unchecked functions and don't do any bounds checking. For the time being these are not public and only a preparation for the following commit. Making it public and stabilization can follow later and be discussed in https://github.com/rust-lang/rust/issues/76014 . --- library/core/src/slice/mod.rs | 102 +++++++++++++++++++++++++++++++--- 1 file changed, 94 insertions(+), 8 deletions(-) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 0d97ddb29af..d5f32ccc49b 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -1148,7 +1148,10 @@ impl [T] { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn split_at(&self, mid: usize) -> (&[T], &[T]) { - (&self[..mid], &self[mid..]) + assert!(mid <= self.len()); + // SAFETY: `[ptr; mid]` and `[mid; len]` are inside `self`, which + // fulfills the requirements of `from_raw_parts_mut`. + unsafe { self.split_at_unchecked(mid) } } /// Divides one mutable slice into two at an index. @@ -1178,16 +1181,99 @@ impl [T] { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn split_at_mut(&mut self, mid: usize) -> (&mut [T], &mut [T]) { + assert!(mid <= self.len()); + // SAFETY: `[ptr; mid]` and `[mid; len]` are inside `self`, which + // fulfills the requirements of `from_raw_parts_mut`. + unsafe { self.split_at_mut_unchecked(mid) } + } + + /// Divides one slice into two at an index, without doing bounds checking. + /// + /// The first will contain all indices from `[0, mid)` (excluding + /// the index `mid` itself) and the second will contain all + /// indices from `[mid, len)` (excluding the index `len` itself). + /// + /// This is generally not recommended, use with caution! + /// Calling this method with an out-of-bounds index is *[undefined behavior]* + /// even if the resulting reference is not used. + /// For a safe alternative see [`split_at`]. + /// + /// [`split_at`]: #method.split_at + /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html + /// + /// # Examples + /// + /// ```compile_fail + /// #![feature(slice_split_at_unchecked)] + /// + /// let v = [1, 2, 3, 4, 5, 6]; + /// + /// unsafe { + /// let (left, right) = v.split_at_unchecked(0); + /// assert!(left == []); + /// assert!(right == [1, 2, 3, 4, 5, 6]); + /// } + /// + /// unsafe { + /// let (left, right) = v.split_at_unchecked(2); + /// assert!(left == [1, 2]); + /// assert!(right == [3, 4, 5, 6]); + /// } + /// + /// unsafe { + /// let (left, right) = v.split_at_unchecked(6); + /// assert!(left == [1, 2, 3, 4, 5, 6]); + /// assert!(right == []); + /// } + /// ``` + #[unstable(feature = "slice_split_at_unchecked", reason = "new API", issue = "76014")] + #[inline] + unsafe fn split_at_unchecked(&self, mid: usize) -> (&[T], &[T]) { + // SAFETY: Caller has to check that 0 <= mid < self.len() + unsafe { (self.get_unchecked(..mid), self.get_unchecked(mid..)) } + } + + /// Divides one mutable slice into two at an index, without doing bounds checking. + /// + /// The first will contain all indices from `[0, mid)` (excluding + /// the index `mid` itself) and the second will contain all + /// indices from `[mid, len)` (excluding the index `len` itself). + /// + /// This is generally not recommended, use with caution! + /// Calling this method with an out-of-bounds index is *[undefined behavior]* + /// even if the resulting reference is not used. + /// For a safe alternative see [`split_at_mut`]. + /// + /// [`split_at_mut`]: #method.split_at_mut + /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html + /// + /// # Examples + /// + /// ```compile_fail + /// #![feature(slice_split_at_unchecked)] + /// + /// let mut v = [1, 0, 3, 0, 5, 6]; + /// // scoped to restrict the lifetime of the borrows + /// unsafe { + /// let (left, right) = v.split_at_mut_unchecked(2); + /// assert!(left == [1, 0]); + /// assert!(right == [3, 0, 5, 6]); + /// left[1] = 2; + /// right[1] = 4; + /// } + /// assert!(v == [1, 2, 3, 4, 5, 6]); + /// ``` + #[unstable(feature = "slice_split_at_unchecked", reason = "new API", issue = "76014")] + #[inline] + unsafe fn split_at_mut_unchecked(&mut self, mid: usize) -> (&mut [T], &mut [T]) { let len = self.len(); let ptr = self.as_mut_ptr(); - // SAFETY: `[ptr; mid]` and `[mid; len]` are inside `self`, which - // fulfills the requirements of `from_raw_parts_mut`. - unsafe { - assert!(mid <= len); - - (from_raw_parts_mut(ptr, mid), from_raw_parts_mut(ptr.add(mid), len - mid)) - } + // SAFETY: Caller has to check that 0 <= mid < self.len(). + // + // [ptr; mid] and [mid; len] are not overlapping, so returning a mutable reference + // is fine. + unsafe { (from_raw_parts_mut(ptr, mid), from_raw_parts_mut(ptr.add(mid), len - mid)) } } /// Returns an iterator over subslices separated by elements that match From d08996ac543b4d330bef790ff9f727e99c7a539c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Wed, 26 Aug 2020 09:59:04 +0300 Subject: [PATCH 2/3] Get rid of bounds check in slice::chunks_exact() and related functions during construction LLVM can't figure out in let rem = self.len() % chunk_size; let len = self.len() - rem; let (fst, snd) = self.split_at(len); and let rem = self.len() % chunk_size; let (fst, snd) = self.split_at(rem); that the index passed to split_at() is smaller than the slice length and adds a bounds check plus panic for it. Apart from removing the overhead of the bounds check this also allows LLVM to optimize code around the ChunksExact iterator better. --- library/core/src/slice/mod.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index d5f32ccc49b..1f207198ded 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -865,8 +865,9 @@ impl [T] { pub fn chunks_exact(&self, chunk_size: usize) -> ChunksExact<'_, T> { assert_ne!(chunk_size, 0); let rem = self.len() % chunk_size; - let len = self.len() - rem; - let (fst, snd) = self.split_at(len); + let fst_len = self.len() - rem; + // SAFETY: 0 <= fst_len <= self.len() by construction above + let (fst, snd) = unsafe { self.split_at_unchecked(fst_len) }; ChunksExact { v: fst, rem: snd, chunk_size } } @@ -910,8 +911,9 @@ impl [T] { pub fn chunks_exact_mut(&mut self, chunk_size: usize) -> ChunksExactMut<'_, T> { assert_ne!(chunk_size, 0); let rem = self.len() % chunk_size; - let len = self.len() - rem; - let (fst, snd) = self.split_at_mut(len); + let fst_len = self.len() - rem; + // SAFETY: 0 <= fst_len <= self.len() by construction above + let (fst, snd) = unsafe { self.split_at_mut_unchecked(fst_len) }; ChunksExactMut { v: fst, rem: snd, chunk_size } } @@ -1063,7 +1065,8 @@ impl [T] { pub fn rchunks_exact(&self, chunk_size: usize) -> RChunksExact<'_, T> { assert!(chunk_size != 0); let rem = self.len() % chunk_size; - let (fst, snd) = self.split_at(rem); + // SAFETY: 0 <= rem <= self.len() by construction above + let (fst, snd) = unsafe { self.split_at_unchecked(rem) }; RChunksExact { v: snd, rem: fst, chunk_size } } @@ -1108,7 +1111,8 @@ impl [T] { pub fn rchunks_exact_mut(&mut self, chunk_size: usize) -> RChunksExactMut<'_, T> { assert!(chunk_size != 0); let rem = self.len() % chunk_size; - let (fst, snd) = self.split_at_mut(rem); + // SAFETY: 0 <= rem <= self.len() by construction above + let (fst, snd) = unsafe { self.split_at_mut_unchecked(rem) }; RChunksExactMut { v: snd, rem: fst, chunk_size } } From 8d3cf9237dcc9d4f2a7c0bde42dc133e60f51c7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Sun, 30 Aug 2020 22:47:05 +0300 Subject: [PATCH 3/3] Improve documentation of slice::get_unchecked() / split_at_unchecked() Thanks to Ivan Tham, who gave the majority of these suggestions during their review. --- library/core/src/slice/mod.rs | 72 ++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 31 deletions(-) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 1f207198ded..76d54ad9305 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -288,10 +288,12 @@ impl [T] { /// Returns a reference to an element or subslice, without doing bounds /// checking. /// - /// This is generally not recommended, use with caution! + /// For a safe alternative see [`get`]. + /// + /// # Safety + /// /// Calling this method with an out-of-bounds index is *[undefined behavior]* /// even if the resulting reference is not used. - /// For a safe alternative see [`get`]. /// /// [`get`]: #method.get /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html @@ -320,10 +322,12 @@ impl [T] { /// Returns a mutable reference to an element or subslice, without doing /// bounds checking. /// - /// This is generally not recommended, use with caution! + /// For a safe alternative see [`get_mut`]. + /// + /// # Safety + /// /// Calling this method with an out-of-bounds index is *[undefined behavior]* /// even if the resulting reference is not used. - /// For a safe alternative see [`get_mut`]. /// /// [`get_mut`]: #method.get_mut /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html @@ -1133,20 +1137,20 @@ impl [T] { /// /// { /// let (left, right) = v.split_at(0); - /// assert!(left == []); - /// assert!(right == [1, 2, 3, 4, 5, 6]); + /// assert_eq!(left, []); + /// assert_eq!(right, [1, 2, 3, 4, 5, 6]); /// } /// /// { /// let (left, right) = v.split_at(2); - /// assert!(left == [1, 2]); - /// assert!(right == [3, 4, 5, 6]); + /// assert_eq!(left, [1, 2]); + /// assert_eq!(right, [3, 4, 5, 6]); /// } /// /// { /// let (left, right) = v.split_at(6); - /// assert!(left == [1, 2, 3, 4, 5, 6]); - /// assert!(right == []); + /// assert_eq!(left, [1, 2, 3, 4, 5, 6]); + /// assert_eq!(right, []); /// } /// ``` #[stable(feature = "rust1", since = "1.0.0")] @@ -1175,12 +1179,12 @@ impl [T] { /// // scoped to restrict the lifetime of the borrows /// { /// let (left, right) = v.split_at_mut(2); - /// assert!(left == [1, 0]); - /// assert!(right == [3, 0, 5, 6]); + /// assert_eq!(left, [1, 0]); + /// assert_eq!(right, [3, 0, 5, 6]); /// left[1] = 2; /// right[1] = 4; /// } - /// assert!(v == [1, 2, 3, 4, 5, 6]); + /// assert_eq!(v, [1, 2, 3, 4, 5, 6]); /// ``` #[stable(feature = "rust1", since = "1.0.0")] #[inline] @@ -1197,11 +1201,14 @@ impl [T] { /// the index `mid` itself) and the second will contain all /// indices from `[mid, len)` (excluding the index `len` itself). /// - /// This is generally not recommended, use with caution! - /// Calling this method with an out-of-bounds index is *[undefined behavior]* - /// even if the resulting reference is not used. /// For a safe alternative see [`split_at`]. /// + /// # Safety + /// + /// Calling this method with an out-of-bounds index is *[undefined behavior]* + /// even if the resulting reference is not used. The caller has to ensure that + /// `0 <= mid <= self.len()`. + /// /// [`split_at`]: #method.split_at /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html /// @@ -1214,26 +1221,26 @@ impl [T] { /// /// unsafe { /// let (left, right) = v.split_at_unchecked(0); - /// assert!(left == []); - /// assert!(right == [1, 2, 3, 4, 5, 6]); + /// assert_eq!(left, []); + /// assert_eq!(right, [1, 2, 3, 4, 5, 6]); /// } /// /// unsafe { /// let (left, right) = v.split_at_unchecked(2); - /// assert!(left == [1, 2]); - /// assert!(right == [3, 4, 5, 6]); + /// assert_eq!(left, [1, 2]); + /// assert_eq!(right, [3, 4, 5, 6]); /// } /// /// unsafe { /// let (left, right) = v.split_at_unchecked(6); - /// assert!(left == [1, 2, 3, 4, 5, 6]); - /// assert!(right == []); + /// assert_eq!(left, [1, 2, 3, 4, 5, 6]); + /// assert_eq!(right, []); /// } /// ``` #[unstable(feature = "slice_split_at_unchecked", reason = "new API", issue = "76014")] #[inline] unsafe fn split_at_unchecked(&self, mid: usize) -> (&[T], &[T]) { - // SAFETY: Caller has to check that 0 <= mid < self.len() + // SAFETY: Caller has to check that `0 <= mid <= self.len()` unsafe { (self.get_unchecked(..mid), self.get_unchecked(mid..)) } } @@ -1243,11 +1250,14 @@ impl [T] { /// the index `mid` itself) and the second will contain all /// indices from `[mid, len)` (excluding the index `len` itself). /// - /// This is generally not recommended, use with caution! - /// Calling this method with an out-of-bounds index is *[undefined behavior]* - /// even if the resulting reference is not used. /// For a safe alternative see [`split_at_mut`]. /// + /// # Safety + /// + /// Calling this method with an out-of-bounds index is *[undefined behavior]* + /// even if the resulting reference is not used. The caller has to ensure that + /// `0 <= mid <= self.len()`. + /// /// [`split_at_mut`]: #method.split_at_mut /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html /// @@ -1260,12 +1270,12 @@ impl [T] { /// // scoped to restrict the lifetime of the borrows /// unsafe { /// let (left, right) = v.split_at_mut_unchecked(2); - /// assert!(left == [1, 0]); - /// assert!(right == [3, 0, 5, 6]); + /// assert_eq!(left, [1, 0]); + /// assert_eq!(right, [3, 0, 5, 6]); /// left[1] = 2; /// right[1] = 4; /// } - /// assert!(v == [1, 2, 3, 4, 5, 6]); + /// assert_eq!(v, [1, 2, 3, 4, 5, 6]); /// ``` #[unstable(feature = "slice_split_at_unchecked", reason = "new API", issue = "76014")] #[inline] @@ -1273,9 +1283,9 @@ impl [T] { let len = self.len(); let ptr = self.as_mut_ptr(); - // SAFETY: Caller has to check that 0 <= mid < self.len(). + // SAFETY: Caller has to check that `0 <= mid <= self.len()`. // - // [ptr; mid] and [mid; len] are not overlapping, so returning a mutable reference + // `[ptr; mid]` and `[mid; len]` are not overlapping, so returning a mutable reference // is fine. unsafe { (from_raw_parts_mut(ptr, mid), from_raw_parts_mut(ptr.add(mid), len - mid)) } }