From 1e40c80cf527c3b18baaf443f0fc553f35424499 Mon Sep 17 00:00:00 2001 From: Oliver Middleton Date: Fri, 4 Nov 2016 01:07:00 +0000 Subject: [PATCH] Fix issues with the Add/AddAssign impls for Cow * Correct the stability attributes. * Make Add and AddAssign actually behave the same. * Use String::with_capacity when allocating a new string. * Fix the tests. --- src/libcollections/borrow.rs | 67 +++++++++------ src/libcollectionstest/cow_str.rs | 134 +++++++++++++++++++++++------- src/libcollectionstest/lib.rs | 1 + 3 files changed, 146 insertions(+), 56 deletions(-) diff --git a/src/libcollections/borrow.rs b/src/libcollections/borrow.rs index 8f9c3578337..3bc2a3f8f68 100644 --- a/src/libcollections/borrow.rs +++ b/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 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> for Cow<'a, str> { type Output = Cow<'a, str>; - fn add(self, rhs: Cow<'a, str>) -> Self { - if self == "" { - rhs - } else if rhs == "" { - self + #[inline] + fn add(mut self, rhs: Cow<'a, str>) -> Self::Output { + self += rhs; + self + } +} + +#[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 self.is_empty() { + *self = Cow::Borrowed(rhs) + } else if rhs.is_empty() { + return; } else { - Cow::Owned(self.into_owned() + rhs.borrow()) + 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")] -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); - } -} - -#[stable(feature = "cow_add", since = "1.13.0")] +#[stable(feature = "cow_add", since = "1.14.0")] impl<'a> AddAssign> 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); + } } } diff --git a/src/libcollectionstest/cow_str.rs b/src/libcollectionstest/cow_str.rs index 82533ba0775..b29245121da 100644 --- a/src/libcollectionstest/cow_str.rs +++ b/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 = Cow::Owned(String::from("Hi, ")); + let owned2: Cow = Cow::Owned(String::from("Rustaceans!")); + let owned_empty: Cow = 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(""); - owned1 = Cow::Owned("Hi, ".into()); - owned2 = Cow::Owned("Rustaceans!".into()); - owned_empty = Cow::Owned("".into()); + let owned: Cow = Cow::Owned(String::from("Hi, ")); + let owned_empty: Cow = Cow::Owned(String::new()); - let borrowed1clone = borrowed1.clone(); - borrowed1clone += borrow_empty; - assert_eq!((&borrowed1clone).as_ptr(), (&borrowed1).as_ptr()); + assert_eq!("Hello, World!", borrowed.clone() + "World!"); - borrowed1clone += owned_empty; - assert_eq!((&borrowed1clone).as_ptr(), (&borrowed1).as_ptr()); + assert_eq!("Hi, World!", owned.clone() + "World!"); + + 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 = Cow::Owned(String::from("Hi, ")); + let owned2: Cow = Cow::Owned(String::from("Rustaceans!")); + let owned_empty: Cow = 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 = Cow::Owned(String::from("Hi, ")); + let owned_empty: Cow = 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); +} diff --git a/src/libcollectionstest/lib.rs b/src/libcollectionstest/lib.rs index 5d3e03c2dee..14ec8d58bef 100644 --- a/src/libcollectionstest/lib.rs +++ b/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;