Skip to content

Commit

Permalink
Remove Splice struct return value from String::splice
Browse files Browse the repository at this point in the history
  • Loading branch information
mattico committed Aug 26, 2017
1 parent a24e0f2 commit 0c3c43c
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 109 deletions.
3 changes: 1 addition & 2 deletions src/doc/unstable-book/src/library-features/splice.md
Expand Up @@ -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");
```
109 changes: 19 additions & 90 deletions src/liballoc/string.rs
Expand Up @@ -1391,19 +1391,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 doesnt 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
///
Expand All @@ -1415,45 +1415,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`]`>`.
Expand Down Expand Up @@ -2240,61 +2227,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()
}
}
22 changes: 5 additions & 17 deletions src/liballoc/tests/string.rs
Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand Down

0 comments on commit 0c3c43c

Please sign in to comment.