Fix issues with the Add/AddAssign impls for Cow<str>

* Correct the stability attributes.
* Make Add and AddAssign actually behave the same.
* Use String::with_capacity when allocating a new string.
* Fix the tests.

Author: ollie27

src/libcollections/borrow.rs

@@ -17,6 +17,7 @@ use core::hash::{Hash, Hasher};
 use core::ops::{Add, AddAssign, Deref};
 
 use fmt;
+use string::String;
 
 use self::Cow::*;
 
@@ -284,48 +285,60 @@ impl<'a, T: ?Sized + ToOwned> AsRef<T> for Cow<'a, T> {
     }
 }
 
-#[stable(feature = "cow_add", since = "1.13.0")]
+#[stable(feature = "cow_add", since = "1.14.0")]
 impl<'a> Add<&'a str> for Cow<'a, str> {
     type Output = Cow<'a, str>;
 
-    fn add(self, rhs: &'a str) -> Self {
-        if self == "" {
-            Cow::Borrowed(rhs)
-        } else if rhs == "" {
-            self
-        } else {
-            Cow::Owned(self.into_owned() + rhs)
-        }
+    #[inline]
+    fn add(mut self, rhs: &'a str) -> Self::Output {
+        self += rhs;
+        self
     }
 }
 
-#[stable(feature = "cow_add", since = "1.13.0")]
+#[stable(feature = "cow_add", since = "1.14.0")]
 impl<'a> Add<Cow<'a, str>> for Cow<'a, str> {
     type Output = Cow<'a, str>;
 
-    fn add(self, rhs: Cow<'a, str>) -> Self {
-        if self == "" {
-            rhs
-        } else if rhs == "" {
-            self
-        } else {
-            Cow::Owned(self.into_owned() + rhs.borrow())
-        }
+    #[inline]
+    fn add(mut self, rhs: Cow<'a, str>) -> Self::Output {
+        self += rhs;
+        self
     }
 }
 
-#[stable(feature = "cow_add", since = "1.13.0")]
+#[stable(feature = "cow_add", since = "1.14.0")]
 impl<'a> AddAssign<&'a str> for Cow<'a, str> {
     fn add_assign(&mut self, rhs: &'a str) {
-        if rhs == "" { return; }
-        self.to_mut().push_str(rhs);
+        if self.is_empty() {
+            *self = Cow::Borrowed(rhs)
+        } else if rhs.is_empty() {
+            return;
+        } else {
+            if let Cow::Borrowed(lhs) = *self {
+                let mut s = String::with_capacity(lhs.len() + rhs.len());
+                s.push_str(lhs);
+                *self = Cow::Owned(s);
+            }
+            self.to_mut().push_str(rhs);
+        }
     }
 }
 
-#[stable(feature = "cow_add", since = "1.13.0")]
+#[stable(feature = "cow_add", since = "1.14.0")]
 impl<'a> AddAssign<Cow<'a, str>> for Cow<'a, str> {
     fn add_assign(&mut self, rhs: Cow<'a, str>) {
-        if rhs == "" { return; }
-        self.to_mut().push_str(rhs.borrow());
+        if self.is_empty() {
+            *self = rhs
+        } else if rhs.is_empty() {
+            return;
+        } else {
+            if let Cow::Borrowed(lhs) = *self {
+                let mut s = String::with_capacity(lhs.len() + rhs.len());
+                s.push_str(lhs);
+                *self = Cow::Owned(s);
+            }
+            self.to_mut().push_str(&rhs);
+        }
     }
 }

src/libcollectionstest/cow_str.rs

@@ -1,4 +1,4 @@
-// Copyright 2012-2013-2014 The Rust Project Developers. See the COPYRIGHT
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
 // file at the top-level directory of this distribution and at
 // http://rust-lang.org/COPYRIGHT.
 //
@@ -12,54 +12,130 @@ use std::borrow::Cow;
 
 // check that Cow<'a, str> implements addition
 #[test]
-fn check_cow_add() {
-    borrowed1 = Cow::Borrowed("Hello, ");
-    borrowed2 = Cow::Borrowed("World!");
-    borrow_empty = Cow::Borrowed("");
+fn check_cow_add_cow() {
+    let borrowed1 = Cow::Borrowed("Hello, ");
+    let borrowed2 = Cow::Borrowed("World!");
+    let borrow_empty = Cow::Borrowed("");
 
-    owned1 = Cow::Owned("Hi, ".into());
-    owned2 = Cow::Owned("Rustaceans!".into());
-    owned_empty = Cow::Owned("".into());
+    let owned1: Cow<str> = Cow::Owned(String::from("Hi, "));
+    let owned2: Cow<str> = Cow::Owned(String::from("Rustaceans!"));
+    let owned_empty: Cow<str> = Cow::Owned(String::new());
 
-    assert_eq!("Hello, World!", borrowed1 + borrowed2);
-    assert_eq!("Hello, Rustaceans!", borrowed1 + owned2);
+    assert_eq!("Hello, World!", borrowed1.clone() + borrowed2.clone());
+    assert_eq!("Hello, Rustaceans!", borrowed1.clone() + owned2.clone());
 
-    assert_eq!("Hello, World!", owned1 + borrowed2);
-    assert_eq!("Hello, Rustaceans!", owned1 + owned2);
+    assert_eq!("Hi, World!", owned1.clone() + borrowed2.clone());
+    assert_eq!("Hi, Rustaceans!", owned1.clone() + owned2.clone());
 
-    if let Cow::Owned(_) = borrowed1 + borrow_empty {
+    if let Cow::Owned(_) = borrowed1.clone() + borrow_empty.clone() {
         panic!("Adding empty strings to a borrow should note allocate");
     }
-    if let Cow::Owned(_) = borrow_empty + borrowed1 {
+    if let Cow::Owned(_) = borrow_empty.clone() + borrowed1.clone() {
         panic!("Adding empty strings to a borrow should note allocate");
     }
-    if let Cow::Owned(_) = borrowed1 + owned_empty {
+    if let Cow::Owned(_) = borrowed1.clone() + owned_empty.clone() {
         panic!("Adding empty strings to a borrow should note allocate");
     }
-    if let Cow::Owned(_) = owned_empty + borrowed1 {
+    if let Cow::Owned(_) = owned_empty.clone() + borrowed1.clone() {
         panic!("Adding empty strings to a borrow should note allocate");
     }
 }
 
-fn check_cow_add_assign() {
-    borrowed1 = Cow::Borrowed("Hello, ");
-    borrowed2 = Cow::Borrowed("World!");
-    borrow_empty = Cow::Borrowed("");
+#[test]
+fn check_cow_add_str() {
+    let borrowed = Cow::Borrowed("Hello, ");
+    let borrow_empty = Cow::Borrowed("");
+
+    let owned: Cow<str> = Cow::Owned(String::from("Hi, "));
+    let owned_empty: Cow<str> = Cow::Owned(String::new());
 
-    owned1 = Cow::Owned("Hi, ".into());
-    owned2 = Cow::Owned("Rustaceans!".into());
-    owned_empty = Cow::Owned("".into());
+    assert_eq!("Hello, World!", borrowed.clone() + "World!");
 
-    let borrowed1clone = borrowed1.clone();
-    borrowed1clone += borrow_empty;
-    assert_eq!((&borrowed1clone).as_ptr(), (&borrowed1).as_ptr());
+    assert_eq!("Hi, World!", owned.clone() + "World!");
 
-    borrowed1clone += owned_empty;
-    assert_eq!((&borrowed1clone).as_ptr(), (&borrowed1).as_ptr());
+    if let Cow::Owned(_) = borrowed.clone() + "" {
+        panic!("Adding empty strings to a borrow should note allocate");
+    }
+    if let Cow::Owned(_) = borrow_empty.clone() + "Hello, " {
+        panic!("Adding empty strings to a borrow should note allocate");
+    }
+    if let Cow::Owned(_) = owned_empty.clone() + "Hello, " {
+        panic!("Adding empty strings to a borrow should note allocate");
+    }
+}
+
+#[test]
+fn check_cow_add_assign_cow() {
+    let mut borrowed1 = Cow::Borrowed("Hello, ");
+    let borrowed2 = Cow::Borrowed("World!");
+    let borrow_empty = Cow::Borrowed("");
+
+    let mut owned1: Cow<str> = Cow::Owned(String::from("Hi, "));
+    let owned2: Cow<str> = Cow::Owned(String::from("Rustaceans!"));
+    let owned_empty: Cow<str> = Cow::Owned(String::new());
+
+    let mut s = borrowed1.clone();
+    s += borrow_empty.clone();
+    assert_eq!("Hello, ", s);
+    if let Cow::Owned(_) = s {
+        panic!("Adding empty strings to a borrow should note allocate");
+    }
+    let mut s = borrow_empty.clone();
+    s += borrowed1.clone();
+    assert_eq!("Hello, ", s);
+    if let Cow::Owned(_) = s {
+        panic!("Adding empty strings to a borrow should note allocate");
+    }
+    let mut s = borrowed1.clone();
+    s += owned_empty.clone();
+    assert_eq!("Hello, ", s);
+    if let Cow::Owned(_) = s {
+        panic!("Adding empty strings to a borrow should note allocate");
+    }
+    let mut s = owned_empty.clone();
+    s += borrowed1.clone();
+    assert_eq!("Hello, ", s);
+    if let Cow::Owned(_) = s {
+        panic!("Adding empty strings to a borrow should note allocate");
+    }
 
     owned1 += borrowed2;
     borrowed1 += owned2;
 
-    assert_eq!("Hello, World!", owned1);
+    assert_eq!("Hi, World!", owned1);
     assert_eq!("Hello, Rustaceans!", borrowed1);
 }
+
+#[test]
+fn check_cow_add_assign_str() {
+    let mut borrowed = Cow::Borrowed("Hello, ");
+    let borrow_empty = Cow::Borrowed("");
+
+    let mut owned: Cow<str> = Cow::Owned(String::from("Hi, "));
+    let owned_empty: Cow<str> = Cow::Owned(String::new());
+
+    let mut s = borrowed.clone();
+    s += "";
+    assert_eq!("Hello, ", s);
+    if let Cow::Owned(_) = s {
+        panic!("Adding empty strings to a borrow should note allocate");
+    }
+    let mut s = borrow_empty.clone();
+    s += "World!";
+    assert_eq!("World!", s);
+    if let Cow::Owned(_) = s {
+        panic!("Adding empty strings to a borrow should note allocate");
+    }
+    let mut s = owned_empty.clone();
+    s += "World!";
+    assert_eq!("World!", s);
+    if let Cow::Owned(_) = s {
+        panic!("Adding empty strings to a borrow should note allocate");
+    }
+
+    owned += "World!";
+    borrowed += "World!";
+
+    assert_eq!("Hi, World!", owned);
+    assert_eq!("Hello, World!", borrowed);
+}

src/libcollectionstest/lib.rs

@@ -42,6 +42,7 @@ mod bench;
 
 mod binary_heap;
 mod btree;
+mod cow_str;
 mod enum_set;
 mod fmt;
 mod linked_list;