From 1e27599bf5d7e300ff6181049a3e771f6a4825b9 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Sat, 3 Oct 2020 13:13:39 +0530 Subject: [PATCH 1/6] Add `try_remove` to `Vec` This provides a 'non-panicky' way of removing an element from a vector When the index exists, we return it; Otherwise, `None` is returned --- library/alloc/src/vec.rs | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index b20ccd388d1f1..9a889309b366a 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -1088,6 +1088,39 @@ impl Vec { } } + /// Tries to remove an element from the vector. If the element at `index` does not exist, + /// `None` is returned. Otherwise `Some(T)` is returned + /// + /// # Examples + /// ``` + /// let mut v = vec![1, 2, 3]; + /// assert_eq!(v.remove(0), Some(1)); + /// assert_eq!(v.remove(2), None); + /// ``` + pub fn try_remove(&mut self, index: usize) -> Option { + let len = self.len(); + if index >= len { + None + } else { + unsafe { + // infallible + let ret; + { + // the place we are taking from. + let ptr = self.as_mut_ptr().add(index); + // copy it out, unsafely having a copy of the value on + // the stack and in the vector at the same time. + ret = ptr::read(ptr); + + // Shift everything down to fill in that spot. + ptr::copy(ptr.offset(1), ptr, len - index - 1); + } + self.set_len(len - 1); + Some(ret) + } + } + } + /// Retains only the elements specified by the predicate. /// /// In other words, remove all elements `e` such that `f(&e)` returns `false`. From 6402b46f6f944eaafef27dc13eec5ce88a5ae641 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Sat, 3 Oct 2020 14:10:05 +0530 Subject: [PATCH 2/6] Keep `vec_try_remove` under unstable feature gate --- library/alloc/src/vec.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 9a889309b366a..54ea975554355 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -1097,6 +1097,7 @@ impl Vec { /// assert_eq!(v.remove(0), Some(1)); /// assert_eq!(v.remove(2), None); /// ``` + #![unstable(feature = "vec_try_remove", issue = "77481")] pub fn try_remove(&mut self, index: usize) -> Option { let len = self.len(); if index >= len { From 177040b7214fd7ea904ff932aac2d69df35f66d3 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Sat, 3 Oct 2020 15:55:56 +0530 Subject: [PATCH 3/6] Rewrite `Vec::remove` using `Vec::try_remove` --- library/alloc/src/vec.rs | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index 54ea975554355..e22cd8aa22700 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -1065,39 +1065,23 @@ impl Vec { fn assert_failed(index: usize, len: usize) -> ! { panic!("removal index (is {}) should be < len (is {})", index, len); } - - let len = self.len(); - if index >= len { - assert_failed(index, len); - } - unsafe { - // infallible - let ret; - { - // the place we are taking from. - let ptr = self.as_mut_ptr().add(index); - // copy it out, unsafely having a copy of the value on - // the stack and in the vector at the same time. - ret = ptr::read(ptr); - - // Shift everything down to fill in that spot. - ptr::copy(ptr.offset(1), ptr, len - index - 1); - } - self.set_len(len - 1); - ret + if let Some(retval) = self.try_remove(index) { + retval + } else { + assert_failed(index, self.len()); } } /// Tries to remove an element from the vector. If the element at `index` does not exist, /// `None` is returned. Otherwise `Some(T)` is returned - /// + /// /// # Examples /// ``` /// let mut v = vec![1, 2, 3]; - /// assert_eq!(v.remove(0), Some(1)); - /// assert_eq!(v.remove(2), None); + /// assert_eq!(v.try_remove(0), Some(1)); + /// assert_eq!(v.try_remove(2), None); /// ``` - #![unstable(feature = "vec_try_remove", issue = "77481")] + #[unstable(feature = "vec_try_remove", issue = "77481")] pub fn try_remove(&mut self, index: usize) -> Option { let len = self.len(); if index >= len { @@ -1112,7 +1096,6 @@ impl Vec { // copy it out, unsafely having a copy of the value on // the stack and in the vector at the same time. ret = ptr::read(ptr); - // Shift everything down to fill in that spot. ptr::copy(ptr.offset(1), ptr, len - index - 1); } From 26d1c224d67b6e3fc01be8c9fef10dde42c2be36 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Sat, 3 Oct 2020 17:04:47 +0530 Subject: [PATCH 4/6] Enable `vec_try_remove` feature gate for doctests --- library/alloc/src/vec.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/library/alloc/src/vec.rs b/library/alloc/src/vec.rs index e22cd8aa22700..f7d421ca0ccd4 100644 --- a/library/alloc/src/vec.rs +++ b/library/alloc/src/vec.rs @@ -1077,6 +1077,7 @@ impl Vec { /// /// # Examples /// ``` + /// #![feature(vec_try_remove)] /// let mut v = vec![1, 2, 3]; /// assert_eq!(v.try_remove(0), Some(1)); /// assert_eq!(v.try_remove(2), None); From 86dbce10eb157f36807ed622f7f7576996ac39f7 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Sat, 3 Oct 2020 18:56:35 +0530 Subject: [PATCH 5/6] Add tests for `Vec::try_remove` --- library/alloc/tests/vec.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index b7c7138db4f52..cb9cc900b73bc 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -1,3 +1,5 @@ +#![feature(vec_try_remove)] + use std::borrow::Cow; use std::cell::Cell; use std::collections::TryReserveError::*; @@ -518,6 +520,21 @@ fn test_swap_remove_empty() { vec.swap_remove(0); } +#[test] +fn test_try_remove() { + let mut vec = vec![1, 2, 3]; + // We are attempting to remove vec[0] which contains 1 + assert_eq!(vec.try_remove(0), Some(1)); + // Now `vec` looks like: [2, 3] + // We will now try to remove vec[2] which does not exist + // This should return `None` + assert_eq!(vec.try_remove(2), None); + + // We will try the same thing with an empty vector + let mut v = vec![]; + assert!(v.try_remove(0).is_none()); +} + #[test] fn test_move_items() { let vec = vec![1, 2, 3]; From aa3b7f84d5f7dd68d0fd6c2d2f6908225c2ddea3 Mon Sep 17 00:00:00 2001 From: Sayan Nandan Date: Sat, 3 Oct 2020 20:18:02 +0530 Subject: [PATCH 6/6] Add feature attr for `vec_try_remove` for tests --- library/alloc/tests/lib.rs | 1 + library/alloc/tests/vec.rs | 4 +--- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/library/alloc/tests/lib.rs b/library/alloc/tests/lib.rs index cff8ff9ac7ad9..4efeb0dc84c54 100644 --- a/library/alloc/tests/lib.rs +++ b/library/alloc/tests/lib.rs @@ -20,6 +20,7 @@ #![feature(inplace_iteration)] #![feature(iter_map_while)] #![feature(int_bits_const)] +#![feature(vec_try_remove)] use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; diff --git a/library/alloc/tests/vec.rs b/library/alloc/tests/vec.rs index cb9cc900b73bc..9d453fc32921e 100644 --- a/library/alloc/tests/vec.rs +++ b/library/alloc/tests/vec.rs @@ -1,5 +1,3 @@ -#![feature(vec_try_remove)] - use std::borrow::Cow; use std::cell::Cell; use std::collections::TryReserveError::*; @@ -531,7 +529,7 @@ fn test_try_remove() { assert_eq!(vec.try_remove(2), None); // We will try the same thing with an empty vector - let mut v = vec![]; + let mut v: Vec = vec![]; assert!(v.try_remove(0).is_none()); }