Rollup merge of rust-lang#44044 - mattico:string-splice-return, r=dtolnay

Remove Splice struct return value from String::splice

The implementation is now almost identical to the one in the RFC.

Fixes rust-lang#44038
cc rust-lang#32310

Author: alexcrichton

src/doc/unstable-book/src/library-features/splice.md

@@ -18,7 +18,6 @@ let mut s = String::from("α is alpha, β is beta");
 let beta_offset = s.find('β').unwrap_or(s.len());
 
 // Replace the range up until the β from the string
-let t: String = s.splice(..beta_offset, "Α is capital alpha; ").collect();
-assert_eq!(t, "α is alpha, ");
+s.splice(..beta_offset, "Α is capital alpha; ");
 assert_eq!(s, "Α is capital alpha; β is beta");
 ```

src/liballoc/string.rs

@@ -1392,19 +1392,19 @@ impl String {
     }
 
     /// Creates a splicing iterator that removes the specified range in the string,
-    /// replaces with the given string, and yields the removed chars.
-    /// The given string doesn’t need to be the same length as the range.
+    /// and replaces it with the given string.
+    /// The given string doesn't need to be the same length as the range.
     ///
-    /// Note: The element range is removed when the [`Splice`] is dropped,
-    /// even if the iterator is not consumed until the end.
+    /// Note: Unlike [`Vec::splice`], the replacement happens eagerly, and this
+    /// method does not return the removed chars.
     ///
     /// # Panics
     ///
     /// Panics if the starting point or end point do not lie on a [`char`]
     /// boundary, or if they're out of bounds.
     ///
     /// [`char`]: ../../std/primitive.char.html
-    /// [`Splice`]: ../../std/string/struct.Splice.html
+    /// [`Vec::splice`]: ../../std/vec/struct.Vec.html#method.splice
     ///
     /// # Examples
     ///
@@ -1416,45 +1416,32 @@ impl String {
     /// let beta_offset = s.find('β').unwrap_or(s.len());
     ///
     /// // Replace the range up until the β from the string
-    /// let t: String = s.splice(..beta_offset, "Α is capital alpha; ").collect();
-    /// assert_eq!(t, "α is alpha, ");
+    /// s.splice(..beta_offset, "Α is capital alpha; ");
     /// assert_eq!(s, "Α is capital alpha; β is beta");
     /// ```
     #[unstable(feature = "splice", reason = "recently added", issue = "32310")]
-    pub fn splice<'a, 'b, R>(&'a mut self, range: R, replace_with: &'b str) -> Splice<'a, 'b>
+    pub fn splice<R>(&mut self, range: R, replace_with: &str)
         where R: RangeArgument<usize>
     {
         // Memory safety
         //
         // The String version of Splice does not have the memory safety issues
         // of the vector version. The data is just plain bytes.
-        // Because the range removal happens in Drop, if the Splice iterator is leaked,
-        // the removal will not happen.
-        let len = self.len();
-        let start = match range.start() {
-             Included(&n) => n,
-             Excluded(&n) => n + 1,
-             Unbounded => 0,
+
+        match range.start() {
+             Included(&n) => assert!(self.is_char_boundary(n)),
+             Excluded(&n) => assert!(self.is_char_boundary(n + 1)),
+             Unbounded => {},
         };
-        let end = match range.end() {
-             Included(&n) => n + 1,
-             Excluded(&n) => n,
-             Unbounded => len,
+        match range.end() {
+             Included(&n) => assert!(self.is_char_boundary(n + 1)),
+             Excluded(&n) => assert!(self.is_char_boundary(n)),
+             Unbounded => {},
         };
 
-        // Take out two simultaneous borrows. The &mut String won't be accessed
-        // until iteration is over, in Drop.
-        let self_ptr = self as *mut _;
-        // slicing does the appropriate bounds checks
-        let chars_iter = self[start..end].chars();
-
-        Splice {
-            start,
-            end,
-            iter: chars_iter,
-            string: self_ptr,
-            replace_with,
-        }
+        unsafe {
+            self.as_mut_vec()
+        }.splice(range, replace_with.bytes());
     }
 
     /// Converts this `String` into a [`Box`]`<`[`str`]`>`.
@@ -2241,61 +2228,3 @@ impl<'a> DoubleEndedIterator for Drain<'a> {
 
 #[unstable(feature = "fused", issue = "35602")]
 impl<'a> FusedIterator for Drain<'a> {}
-
-/// A splicing iterator for `String`.
-///
-/// This struct is created by the [`splice()`] method on [`String`]. See its
-/// documentation for more.
-///
-/// [`splice()`]: struct.String.html#method.splice
-/// [`String`]: struct.String.html
-#[derive(Debug)]
-#[unstable(feature = "splice", reason = "recently added", issue = "32310")]
-pub struct Splice<'a, 'b> {
-    /// Will be used as &'a mut String in the destructor
-    string: *mut String,
-    /// Start of part to remove
-    start: usize,
-    /// End of part to remove
-    end: usize,
-    /// Current remaining range to remove
-    iter: Chars<'a>,
-    replace_with: &'b str,
-}
-
-#[unstable(feature = "splice", reason = "recently added", issue = "32310")]
-unsafe impl<'a, 'b> Sync for Splice<'a, 'b> {}
-#[unstable(feature = "splice", reason = "recently added", issue = "32310")]
-unsafe impl<'a, 'b> Send for Splice<'a, 'b> {}
-
-#[unstable(feature = "splice", reason = "recently added", issue = "32310")]
-impl<'a, 'b> Drop for Splice<'a, 'b> {
-    fn drop(&mut self) {
-        unsafe {
-            let vec = (*self.string).as_mut_vec();
-            vec.splice(self.start..self.end, self.replace_with.bytes());
-        }
-    }
-}
-
-#[unstable(feature = "splice", reason = "recently added", issue = "32310")]
-impl<'a, 'b> Iterator for Splice<'a, 'b> {
-    type Item = char;
-
-    #[inline]
-    fn next(&mut self) -> Option<char> {
-        self.iter.next()
-    }
-
-    fn size_hint(&self) -> (usize, Option<usize>) {
-        self.iter.size_hint()
-    }
-}
-
-#[unstable(feature = "splice", reason = "recently added", issue = "32310")]
-impl<'a, 'b> DoubleEndedIterator for Splice<'a, 'b> {
-    #[inline]
-    fn next_back(&mut self) -> Option<char> {
-        self.iter.next_back()
-    }
-}

src/liballoc/tests/string.rs

@@ -442,9 +442,8 @@ fn test_drain() {
 #[test]
 fn test_splice() {
     let mut s = "Hello, world!".to_owned();
-    let t: String = s.splice(7..12, "世界").collect();
+    s.splice(7..12, "世界");
     assert_eq!(s, "Hello, 世界!");
-    assert_eq!(t, "world");
 }
 
 #[test]
@@ -457,12 +456,10 @@ fn test_splice_char_boundary() {
 #[test]
 fn test_splice_inclusive_range() {
     let mut v = String::from("12345");
-    let t: String = v.splice(2...3, "789").collect();
+    v.splice(2...3, "789");
     assert_eq!(v, "127895");
-    assert_eq!(t, "34");
-    let t2: String = v.splice(1...2, "A").collect();
+    v.splice(1...2, "A");
     assert_eq!(v, "1A895");
-    assert_eq!(t2, "27");
 }
 
 #[test]
@@ -482,24 +479,15 @@ fn test_splice_inclusive_out_of_bounds() {
 #[test]
 fn test_splice_empty() {
     let mut s = String::from("12345");
-    let t: String = s.splice(1..2, "").collect();
+    s.splice(1..2, "");
     assert_eq!(s, "1345");
-    assert_eq!(t, "2");
 }
 
 #[test]
 fn test_splice_unbounded() {
     let mut s = String::from("12345");
-    let t: String = s.splice(.., "").collect();
+    s.splice(.., "");
     assert_eq!(s, "");
-    assert_eq!(t, "12345");
-}
-
-#[test]
-fn test_splice_forget() {
-    let mut s = String::from("12345");
-    ::std::mem::forget(s.splice(2..4, "789"));
-    assert_eq!(s, "12345");
 }
 
 #[test]