From 1afe14ceedaad19bc3e10dda28ce3f52c69e287a Mon Sep 17 00:00:00 2001 From: ibraheemdev Date: Tue, 31 Aug 2021 12:59:44 -0400 Subject: [PATCH 1/6] add `slice::swap_unchecked` --- library/core/src/slice/mod.rs | 46 +++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 53b8b343238d5..2624df78a7612 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -560,15 +560,45 @@ impl [T] { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn swap(&mut self, a: usize, b: usize) { - // Can't take two mutable loans from one vector, so instead use raw pointers. - let pa = ptr::addr_of_mut!(self[a]); - let pb = ptr::addr_of_mut!(self[b]); - // SAFETY: `pa` and `pb` have been created from safe mutable references and refer - // to elements in the slice and therefore are guaranteed to be valid and aligned. - // Note that accessing the elements behind `a` and `b` is checked and will - // panic when out of bounds. + assert!(a < self.len()); + assert!(b < self.len()); + // SAFETY: we just checked that both `a` and `b` are in bounds + unsafe { self.swap_unchecked(a, b) } + } + + /// Swaps two elements in the slice, without doing bounds checking. + /// + /// For a safe alternative see [`swap`]. + /// + /// # Arguments + /// + /// * a - The index of the first element + /// * b - The index of the second element + /// + /// # Safety + /// + /// Calling this method with an out-of-bounds index is *[undefined behavior]*. + /// The caller has to ensure that `a < self.len()` and `b < self.len()`. + /// + /// # Examples + /// + /// ``` + /// let mut v = ["a", "b", "c", "d"]; + /// // SAFETY: we know that 1 and 3 are both indices of the slice + /// unsafe { v.swap_unchecked(1, 3) }; + /// assert!(v == ["a", "d", "c", "b"]); + /// ``` + /// + /// [`swap`]: slice::swap + /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html + #[unstable(feature = "slice_swap_unchecked", issue = "88539")] + pub unsafe fn swap_unchecked(&mut self, a: usize, b: usize) { + debug_assert!(a < self.len()); + debug_assert!(b < self.len()); + let ptr = self.as_mut_ptr(); + // SAFETY: caller has to guarantee that `a < self.len()` and `b < self.len()` unsafe { - ptr::swap(pa, pb); + ptr::swap(ptr.add(a), ptr.add(b)); } } From 14769ce96f53553c9f61b9d5ef950cfbe85d7e51 Mon Sep 17 00:00:00 2001 From: ibraheemdev Date: Tue, 31 Aug 2021 13:25:09 -0400 Subject: [PATCH 2/6] enable `slice_swap_unchecked` feature in doc test --- library/core/src/slice/mod.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 2624df78a7612..16e3c9e52fa9e 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -583,6 +583,8 @@ impl [T] { /// # Examples /// /// ``` + /// #![feature(slice_swap_unchecked)] + /// /// let mut v = ["a", "b", "c", "d"]; /// // SAFETY: we know that 1 and 3 are both indices of the slice /// unsafe { v.swap_unchecked(1, 3) }; From 33ecc33268c1cda372c90c203cead2721a822363 Mon Sep 17 00:00:00 2001 From: ibraheemdev Date: Tue, 31 Aug 2021 21:00:30 -0400 Subject: [PATCH 3/6] use `swap_unchecked` in `slice::reverse` --- library/core/src/slice/mod.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 16e3c9e52fa9e..496cc359c19f9 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -707,11 +707,7 @@ impl [T] { // The resulting pointers `pa` and `pb` are therefore valid and // aligned, and can be read from and written to. unsafe { - // Unsafe swap to avoid the bounds check in safe swap. - let ptr = self.as_mut_ptr(); - let pa = ptr.add(i); - let pb = ptr.add(ln - i - 1); - ptr::swap(pa, pb); + self.swap_unchecked(i, ln - i - 1); } i += 1; } From 2a8ff8df54d5effdfd0154009eea2d50bdd5e598 Mon Sep 17 00:00:00 2001 From: Ibraheem Ahmed Date: Mon, 11 Oct 2021 16:12:43 -0400 Subject: [PATCH 4/6] improve slice::swap panic message --- library/core/src/slice/mod.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 496cc359c19f9..8108d52071b26 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -560,8 +560,9 @@ impl [T] { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn swap(&mut self, a: usize, b: usize) { - assert!(a < self.len()); - assert!(b < self.len()); + assert_in_bounds(self.len(), a); + assert_in_bounds(self.len(), b); + // SAFETY: we just checked that both `a` and `b` are in bounds unsafe { self.swap_unchecked(a, b) } } @@ -595,8 +596,12 @@ impl [T] { /// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html #[unstable(feature = "slice_swap_unchecked", issue = "88539")] pub unsafe fn swap_unchecked(&mut self, a: usize, b: usize) { - debug_assert!(a < self.len()); - debug_assert!(b < self.len()); + #[cfg(debug_assertions)] + { + assert_in_bounds(self.len(), a); + assert_in_bounds(self.len(), b); + } + let ptr = self.as_mut_ptr(); // SAFETY: caller has to guarantee that `a < self.len()` and `b < self.len()` unsafe { @@ -3497,6 +3502,12 @@ impl [T] { } } +fn assert_in_bounds(len: usize, idx: usize) { + if idx >= len { + panic!("index out of bounds: the len is {} but the index is {}", len, idx); + } +} + trait CloneFromSpec { fn spec_clone_from(&mut self, src: &[T]); } From c517a0de3e8d7d6a85b6f47f0ae8c8988311e288 Mon Sep 17 00:00:00 2001 From: Ibraheem Ahmed Date: Mon, 11 Oct 2021 16:13:17 -0400 Subject: [PATCH 5/6] add slice::swap tests --- library/core/tests/slice.rs | 39 +++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/library/core/tests/slice.rs b/library/core/tests/slice.rs index c591dd3e1a6db..b6a326f3d7368 100644 --- a/library/core/tests/slice.rs +++ b/library/core/tests/slice.rs @@ -2152,3 +2152,42 @@ fn test_slice_fill_with_uninit() { let mut a = [MaybeUninit::::uninit(); 10]; a.fill(MaybeUninit::uninit()); } + +#[test] +fn test_swap() { + let mut x = ["a", "b", "c", "d"]; + x.swap(1, 3); + assert_eq!(x, ["a", "d", "c", "b"]); + x.swap(0, 3); + assert_eq!(x, ["b", "d", "c", "a"]); +} + +mod swap_panics { + #[test] + #[should_panic(expected = "index out of bounds: the len is 4 but the index is 4")] + fn index_a_equals_len() { + let mut x = ["a", "b", "c", "d"]; + x.swap(4, 2); + } + + #[test] + #[should_panic(expected = "index out of bounds: the len is 4 but the index is 4")] + fn index_b_equals_len() { + let mut x = ["a", "b", "c", "d"]; + x.swap(2, 4); + } + + #[test] + #[should_panic(expected = "index out of bounds: the len is 4 but the index is 5")] + fn index_a_greater_than_len() { + let mut x = ["a", "b", "c", "d"]; + x.swap(5, 2); + } + + #[test] + #[should_panic(expected = "index out of bounds: the len is 4 but the index is 5")] + fn index_b_greater_than_len() { + let mut x = ["a", "b", "c", "d"]; + x.swap(2, 5); + } +} From cf12732a38a2c038c8cbd5e0266cb38c43cb0c5d Mon Sep 17 00:00:00 2001 From: Ibraheem Ahmed Date: Thu, 14 Oct 2021 09:31:34 -0400 Subject: [PATCH 6/6] don't duplicate slice `panic_bounds_check` --- library/core/src/slice/mod.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/library/core/src/slice/mod.rs b/library/core/src/slice/mod.rs index 8108d52071b26..c0e0589d5edee 100644 --- a/library/core/src/slice/mod.rs +++ b/library/core/src/slice/mod.rs @@ -560,8 +560,8 @@ impl [T] { #[stable(feature = "rust1", since = "1.0.0")] #[inline] pub fn swap(&mut self, a: usize, b: usize) { - assert_in_bounds(self.len(), a); - assert_in_bounds(self.len(), b); + let _ = &self[a]; + let _ = &self[b]; // SAFETY: we just checked that both `a` and `b` are in bounds unsafe { self.swap_unchecked(a, b) } @@ -598,8 +598,8 @@ impl [T] { pub unsafe fn swap_unchecked(&mut self, a: usize, b: usize) { #[cfg(debug_assertions)] { - assert_in_bounds(self.len(), a); - assert_in_bounds(self.len(), b); + let _ = &self[a]; + let _ = &self[b]; } let ptr = self.as_mut_ptr(); @@ -3502,12 +3502,6 @@ impl [T] { } } -fn assert_in_bounds(len: usize, idx: usize) { - if idx >= len { - panic!("index out of bounds: the len is {} but the index is {}", len, idx); - } -} - trait CloneFromSpec { fn spec_clone_from(&mut self, src: &[T]); }