Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove Splice struct return value from String::splice #44044

Merged
merged 1 commit into from
Aug 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/doc/unstable-book/src/library-features/splice.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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