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

Add impl AddAssign<char> for Cow<'_, str> #66215

Closed
Closed
Show file tree
Hide file tree
Changes from 8 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
15 changes: 15 additions & 0 deletions src/liballoc/borrow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,3 +479,18 @@ impl<'a> AddAssign<Cow<'a, str>> for Cow<'a, str> {
}
}
}

#[stable(feature = "cow_str_add_assign_char", since = "1.41.0")]
impl AddAssign<char> for Cow<'_, str> {
Ruster-a11y marked this conversation as resolved.
Show resolved Hide resolved
fn add_assign(&mut self, rhs: char) {
if let Cow::Borrowed(lhs) = *self {
let base_capacity = lhs.len() + rhs.len_utf8();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't quite right -- the amortized_new_size heuristic expects to receive the currently used capacity (how much is already used) and the requested extra capacity (how much is being added) as separate data points. In this AddAssign impl the currently used capacity is the length of the lhs Cow<str> and how much is being added is the size of the rhs char. Adding these together ahead of time would make the heuristic no longer applicable, or behave not as intended.

// 4 bytes more since we know `rhs.len_utf8() <= 4`
let new_optimal_size = lhs.vec.buf.amortized_new_size(base_capacity, 4).expect("capacity overflow");
let mut s = String::with_capacity(new_optimal_size);
s.push_str(lhs);
*self = Cow::Owned(s);
}
self.to_mut().push(rhs);
}
}
56 changes: 56 additions & 0 deletions src/liballoc/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1982,6 +1982,51 @@ impl Add<&str> for String {
}
}

/// Implements the `+` operator for concatenating a string and a char together.
///
/// This consumes the `String` on the left-hand side and re-uses its buffer (growing it if
/// necessary). This is done to avoid allocating a new `String` and copying the entire contents on
/// every operation, which would lead to `O(n^2)` running time when building an `n`-byte string by
/// repeated concatenation.
///
/// # Examples
///
/// Concatenating a `String` with a `char` takes the `String` by value and copies the `char`:
///
/// ```
/// let a = String::from("hello world! ");
/// let b = char::from('👋');
Ruster-a11y marked this conversation as resolved.
Show resolved Hide resolved
/// let c = a + b;
/// // `a` is moved and can no longer be used here.
/// ```
///
/// If you want to keep using the initial `String`, you can clone it and append to the clone instead:
///
/// ```
/// let a = String::from("hello world! ");
/// let b = char::from('👋');
Ruster-a11y marked this conversation as resolved.
Show resolved Hide resolved
/// let c = a.clone() + b;
/// // `a` is still valid here.
/// ```
///
/// Concatenating `char` to a `&str` slice can be done by converting the `&str` to a `String`:
///
/// ```
/// let a = "hello world! ";
/// let b = '👋';
/// let c = a.to_string() + b;
/// ```
#[stable(feature = "add_string_and_char", since = "1.41.0")]
impl Add<char> for String {
type Output = String;

#[inline]
fn add(mut self, other: char) -> String {
self.push(other);
self
}
}

/// Implements the `+=` operator for appending to a `String`.
///
/// This has the same behavior as the [`push_str`][String::push_str] method.
Expand All @@ -1993,6 +2038,17 @@ impl AddAssign<&str> for String {
}
}

/// Implements the `+=` operator for appending a `char` to a `String`.
///
/// This has the same behavior as the [`push`][String::push] method.
#[stable(feature = "stringaddassign_char", since = "1.41.0")]
impl AddAssign<char> for String {
#[inline]
fn add_assign(&mut self, other: char) {
self.push(other);
}
}

#[stable(feature = "rust1", since = "1.0.0")]
impl ops::Index<ops::Range<usize>> for String {
type Output = str;
Expand Down
30 changes: 30 additions & 0 deletions src/liballoc/tests/cow_str.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,33 @@ fn check_cow_clone_from() {
c1.clone_from(&c2);
assert!(c1.into_owned().capacity() >= 25);
}

#[test]
fn check_cow_add_assign_char() {
let test_char = '👋';

let mut borrowed = Cow::Borrowed("Hello, World! ");
let borrow_empty = Cow::Borrowed("");

let mut owned: Cow<'_, str> = Cow::Owned(String::from("Hi, World! "));
let owned_empty: Cow<'_, str> = Cow::Owned(String::new());

let mut s = borrow_empty.clone();
s += test_char;
assert_eq!(test_char.to_string(), s);
if let Cow::Owned(_) = s {
panic!("Adding empty strings to a borrow should note allocate");
}
let mut s = owned_empty.clone();
s += test_char;
assert_eq!(test_char.to_string(), s);
if let Cow::Owned(_) = s {
panic!("Adding empty strings to a borrow should note allocate");
}

owned += test_char;
borrowed += test_char;

assert_eq!(format!("Hi, World! {}", test_char), owned);
assert_eq!(format!("Hello, World! {}", test_char), borrowed);
}
13 changes: 8 additions & 5 deletions src/liballoc/tests/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,10 @@ fn test_add_assign() {
assert_eq!(s.as_str(), "");
s += "abc";
assert_eq!(s.as_str(), "abc");
s += "ประเทศไทย中华Việt Nam";
assert_eq!(s.as_str(), "abcประเทศไทย中华Việt Nam");
s += "ประเทศไทย中华Việt Nam ";
assert_eq!(s.as_str(), "abcประเทศไทย中华Việt Nam ");
s += '👋';
assert_eq!(s.as_str(), "abcประเทศไทย中华Việt Nam 👋")
}

#[test]
Expand Down Expand Up @@ -304,9 +306,10 @@ fn test_str_clear() {
fn test_str_add() {
let a = String::from("12345");
let b = a + "2";
let b = b + "2";
assert_eq!(b.len(), 7);
assert_eq!(b, "1234522");
let b = b + "2 ";
let b = b + '👋';
assert_eq!(b.len(), 12);
assert_eq!(b, "1234522 👋");
}

#[test]
Expand Down