From 79b4ce06ae8be58fcf09aa96afea4bed6d487db5 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Mon, 22 Sep 2014 07:12:10 -0700
Subject: [PATCH 1/3] collections: Stabilize String

# Rationale

When dealing with strings, many functions deal with either a `char` (unicode
codepoint) or a byte (utf-8 encoding related). There is often an inconsistent
way in which methods are referred to as to whether they contain "byte", "char",
or nothing in their name.  There are also issues open to rename *all* methods to
reflect that they operate on utf8 encodings or bytes (e.g. utf8_len() or
byte_len()).

The current state of String seems to largely be what is desired, so this PR
proposes the following rationale for methods dealing with bytes or characters:

> When constructing a string, the input encoding *must* be mentioned (e.g.
> from_utf8). This makes it clear what exactly the input type is expected to be
> in terms of encoding.
>
> When a method operates on anything related to an *index* within the string
> such as length, capacity, position, etc, the method *implicitly* operates on
> bytes. It is an understood fact that String is a utf-8 encoded string, and
> burdening all methods with "bytes" would be redundant.
>
> When a method operates on the *contents* of a string, such as push() or pop(),
> then "char" is the default type. A String can loosely be thought of as being a
> collection of unicode codepoints, but not all collection-related operations
> make sense because some can be woefully inefficient.

# Method stabilization

The following methods have been marked #[stable]

* The String type itself
* String::new
* String::with_capacity
* String::from_utf16_lossy
* String::into_bytes
* String::as_bytes
* String::len
* String::clear
* String::as_slice

The following methods have been marked #[unstable]

* String::from_utf8 - The error type in the returned `Result` may change to
                      provide a nicer message when it's `unwrap()`'d
* String::from_utf8_lossy - The returned `MaybeOwned` type still needs
                            stabilization
* String::from_utf16 - The return type may change to become a `Result` which
                       includes more contextual information like where the error
                       occurred.
* String::from_chars - This is equivalent to iter().collect(), but currently not
                       as ergonomic.
* String::from_char - This method is the equivalent of Vec::from_elem, and has
                      been marked #[unstable] becuase it can be seen as a
                      duplicate of iterator-based functionality as well as
                      possibly being renamed.
* String::push_str - This *can* be emulated with .extend(foo.chars()), but is
                     less efficient because of decoding/encoding. Due to the
                     desire to minimize API surface this may be able to be
                     removed in the future for something possibly generic with
                     no loss in performance.
* String::grow - This is a duplicate of iterator-based functionality, which may
                 become more ergonomic in the future.
* String::capacity - This function was just added.
* String::push - This function was just added.
* String::pop - This function was just added.
* String::truncate - The failure conventions around String methods and byte
                     indices isn't totally clear at this time, so the failure
                     semantics and return value of this method are subject to
                     change.
* String::as_mut_vec - the naming of this method may change.
* string::raw::* - these functions are all waiting on [an RFC][2]

[2]: https://github.com/rust-lang/rfcs/pull/240

The following method have been marked #[experimental]

* String::from_str - This function only exists as it's more efficient than
                     to_string(), but having a less ergonomic function for
                     performance reasons isn't the greatest reason to keep it
                     around. Like Vec::push_all, this has been marked
                     experimental for now.

The following methods have been #[deprecated]

* String::append - This method has been deprecated to remain consistent with the
                   deprecation of Vec::append. While convenient, it is one of
                   the only functional-style apis on String, and requires more
                   though as to whether it belongs as a first-class method or
                   now (and how it relates to other collections).
* String::from_byte - This is fairly rare functionality and can be emulated with
                      str::from_utf8 plus an assert plus a call to to_string().
                      Additionally, String::from_char could possibly be used.
* String::byte_capacity - Renamed to String::capacity due to the rationale
                          above.
* String::push_char - Renamed to String::push due to the rationale above.
* String::pop_char - Renamed to String::pop due to the rationale above.
* String::push_bytes - There are a number of `unsafe` functions on the `String`
                       type which allow bypassing utf-8 checks. These have all
                       been deprecated in favor of calling `.as_mut_vec()` and
                       then operating directly on the vector returned. These
                       methods were deprecated because naming them with relation
                       to other methods was difficult to rationalize and it's
                       arguably more composable to call .as_mut_vec().
* String::as_mut_bytes - See push_bytes
* String::push_byte - See push_bytes
* String::pop_byte - See push_bytes
* String::shift_byte - See push_bytes

# Reservation methods

This commit does not yet touch the methods for reserving bytes. The methods on
Vec have also not yet been modified. These methods are discussed in the upcoming
[Collections reform RFC][1]

[1]: https://github.com/aturon/rfcs/blob/collections-conventions/active/0000-collections-conventions.md#implicit-growth
---
 src/libcollections/string.rs | 86 ++++++++++++++++++++++++++++++++----
 1 file changed, 77 insertions(+), 9 deletions(-)

diff --git a/src/libcollections/string.rs b/src/libcollections/string.rs
index bb66d271ee4d4..7f29645407c57 100644
--- a/src/libcollections/string.rs
+++ b/src/libcollections/string.rs
@@ -31,6 +31,7 @@ use vec::Vec;
 
 /// A growable string stored as a UTF-8 encoded buffer.
 #[deriving(Clone, PartialEq, PartialOrd, Eq, Ord)]
+#[stable]
 pub struct String {
     vec: Vec<u8>,
 }
@@ -44,6 +45,7 @@ impl String {
     /// let mut s = String::new();
     /// ```
     #[inline]
+    #[stable]
     pub fn new() -> String {
         String {
             vec: Vec::new(),
@@ -60,6 +62,7 @@ impl String {
     /// let mut s = String::with_capacity(10);
     /// ```
     #[inline]
+    #[stable]
     pub fn with_capacity(capacity: uint) -> String {
         String {
             vec: Vec::with_capacity(capacity),
@@ -75,6 +78,7 @@ impl String {
     /// assert_eq!(s.as_slice(), "hello");
     /// ```
     #[inline]
+    #[experimental = "needs investigation to see if to_string() can match perf"]
     pub fn from_str(string: &str) -> String {
         String { vec: string.as_bytes().to_vec() }
     }
@@ -111,6 +115,7 @@ impl String {
     /// assert_eq!(s, Err(vec![240, 144, 128]));
     /// ```
     #[inline]
+    #[unstable = "error type may change"]
     pub fn from_utf8(vec: Vec<u8>) -> Result<String, Vec<u8>> {
         if str::is_utf8(vec.as_slice()) {
             Ok(String { vec: vec })
@@ -129,6 +134,7 @@ impl String {
     /// let output = String::from_utf8_lossy(input);
     /// assert_eq!(output.as_slice(), "Hello \uFFFDWorld");
     /// ```
+    #[unstable = "return type may change"]
     pub fn from_utf8_lossy<'a>(v: &'a [u8]) -> MaybeOwned<'a> {
         if str::is_utf8(v) {
             return MaybeOwnedSlice(unsafe { mem::transmute(v) })
@@ -260,6 +266,7 @@ impl String {
     /// v[4] = 0xD800;
     /// assert_eq!(String::from_utf16(v), None);
     /// ```
+    #[unstable = "error value in return may change"]
     pub fn from_utf16(v: &[u16]) -> Option<String> {
         let mut s = String::with_capacity(v.len() / 2);
         for c in str::utf16_items(v) {
@@ -284,6 +291,7 @@ impl String {
     /// assert_eq!(String::from_utf16_lossy(v),
     ///            "𝄞mus\uFFFDic\uFFFD".to_string());
     /// ```
+    #[stable]
     pub fn from_utf16_lossy(v: &[u16]) -> String {
         str::utf16_items(v).map(|c| c.to_char_lossy()).collect()
     }
@@ -298,6 +306,7 @@ impl String {
     /// assert_eq!(s.as_slice(), "hello");
     /// ```
     #[inline]
+    #[unstable = "may be removed in favor of .collect()"]
     pub fn from_chars(chs: &[char]) -> String {
         chs.iter().map(|c| *c).collect()
     }
@@ -312,6 +321,7 @@ impl String {
     /// assert_eq!(bytes, vec![104, 101, 108, 108, 111]);
     /// ```
     #[inline]
+    #[stable]
     pub fn into_bytes(self) -> Vec<u8> {
         self.vec
     }
@@ -329,6 +339,7 @@ impl String {
     /// assert_eq!(big.as_slice(), "hello world!");
     /// ```
     #[inline]
+    #[deprecated = "use .push_str() instead"]
     pub fn append(mut self, second: &str) -> String {
         self.push_str(second);
         self
@@ -343,6 +354,8 @@ impl String {
     /// assert_eq!(s.as_slice(), "aaaaa");
     /// ```
     #[inline]
+    #[unstable = "may be replaced with iterators, questionable usability, and \
+                  the name may change"]
     pub fn from_char(length: uint, ch: char) -> String {
         if length == 0 {
             return String::new()
@@ -370,6 +383,7 @@ impl String {
     /// let s = String::from_byte(104);
     /// assert_eq!(s.as_slice(), "h");
     /// ```
+    #[deprecated = "use str::from_utf8 with a slice of one byte instead"]
     pub fn from_byte(b: u8) -> String {
         assert!(b < 128u8);
         String::from_char(1, b as char)
@@ -385,6 +399,7 @@ impl String {
     /// assert_eq!(s.as_slice(), "foobar");
     /// ```
     #[inline]
+    #[unstable = "extra variants of `push`, could possibly be based on iterators"]
     pub fn push_str(&mut self, string: &str) {
         self.vec.push_all(string.as_bytes())
     }
@@ -399,6 +414,7 @@ impl String {
     /// assert_eq!(s.as_slice(), "fooZZZZZ");
     /// ```
     #[inline]
+    #[unstable = "duplicate of iterator-based functionality"]
     pub fn grow(&mut self, count: uint, ch: char) {
         for _ in range(0, count) {
             self.push_char(ch)
@@ -414,10 +430,25 @@ impl String {
     /// assert!(s.byte_capacity() >= 10);
     /// ```
     #[inline]
+    #[deprecated = "renamed to .capacity()"]
     pub fn byte_capacity(&self) -> uint {
         self.vec.capacity()
     }
 
+    /// Returns the number of bytes that this string buffer can hold without reallocating.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// let s = String::with_capacity(10);
+    /// assert!(s.byte_capacity() >= 10);
+    /// ```
+    #[inline]
+    #[unstable = "just implemented, needs to prove itself"]
+    pub fn capacity(&self) -> uint {
+        self.vec.capacity()
+    }
+
     /// Reserves capacity for at least `extra` additional bytes in this string buffer.
     ///
     /// # Example
@@ -477,19 +508,27 @@ impl String {
         self.vec.shrink_to_fit()
     }
 
+    /// Deprecated, use .push() instead.
+    #[inline]
+    #[deprecated = "renamed to .push()"]
+    pub fn push_char(&mut self, ch: char) {
+        self.push(ch)
+    }
+
     /// Adds the given character to the end of the string.
     ///
     /// # Example
     ///
     /// ```
     /// let mut s = String::from_str("abc");
-    /// s.push_char('1');
-    /// s.push_char('2');
-    /// s.push_char('3');
+    /// s.push('1');
+    /// s.push('2');
+    /// s.push('3');
     /// assert_eq!(s.as_slice(), "abc123");
     /// ```
     #[inline]
-    pub fn push_char(&mut self, ch: char) {
+    #[stable = "function just renamed from push_char"]
+    pub fn push(&mut self, ch: char) {
         let cur_len = self.len();
         // This may use up to 4 bytes.
         self.vec.reserve_additional(4);
@@ -520,6 +559,7 @@ impl String {
     /// assert_eq!(s.as_slice(), "hello");
     /// ```
     #[inline]
+    #[deprecated = "call .as_mut_vec() and push onto that"]
     pub unsafe fn push_bytes(&mut self, bytes: &[u8]) {
         self.vec.push_all(bytes)
     }
@@ -534,6 +574,7 @@ impl String {
     /// assert_eq!(s.as_bytes(), b);
     /// ```
     #[inline]
+    #[stable]
     pub fn as_bytes<'a>(&'a self) -> &'a [u8] {
         self.vec.as_slice()
     }
@@ -557,6 +598,7 @@ impl String {
     /// assert_eq!(s.as_slice(), "h3ll0")
     /// ```
     #[inline]
+    #[deprecated = "call .as_mut_vec().as_slice() instead"]
     pub unsafe fn as_mut_bytes<'a>(&'a mut self) -> &'a mut [u8] {
         self.vec.as_mut_slice()
     }
@@ -575,6 +617,7 @@ impl String {
     /// assert_eq!(s.as_slice(), "he");
     /// ```
     #[inline]
+    #[unstable = "the failure conventions for strings are under development"]
     pub fn truncate(&mut self, len: uint) {
         assert!(self.as_slice().is_char_boundary(len));
         self.vec.truncate(len)
@@ -595,6 +638,7 @@ impl String {
     /// assert_eq!(s.as_slice(), "hello");
     /// ```
     #[inline]
+    #[deprecated = "call .as_mut_vec().push() instead"]
     pub unsafe fn push_byte(&mut self, byte: u8) {
         self.vec.push(byte)
     }
@@ -617,6 +661,7 @@ impl String {
     /// }
     /// ```
     #[inline]
+    #[deprecated = "call .as_mut_vec().pop() instead"]
     pub unsafe fn pop_byte(&mut self) -> Option<u8> {
         let len = self.len();
         if len == 0 {
@@ -628,6 +673,11 @@ impl String {
         Some(byte)
     }
 
+    /// Deprecated. Renamed to `pop`.
+    #[inline]
+    #[deprecated = "renamed to .pop()"]
+    pub fn pop_char(&mut self) -> Option<char> { self.pop() }
+
     /// Removes the last character from the string buffer and returns it.
     /// Returns `None` if this string buffer is empty.
     ///
@@ -635,13 +685,14 @@ impl String {
     ///
     /// ```
     /// let mut s = String::from_str("foo");
-    /// assert_eq!(s.pop_char(), Some('o'));
-    /// assert_eq!(s.pop_char(), Some('o'));
-    /// assert_eq!(s.pop_char(), Some('f'));
-    /// assert_eq!(s.pop_char(), None);
+    /// assert_eq!(s.pop(), Some('o'));
+    /// assert_eq!(s.pop(), Some('o'));
+    /// assert_eq!(s.pop(), Some('f'));
+    /// assert_eq!(s.pop(), None);
     /// ```
     #[inline]
-    pub fn pop_char(&mut self) -> Option<char> {
+    #[unstable = "this function was just renamed from pop_char"]
+    pub fn pop(&mut self) -> Option<char> {
         let len = self.len();
         if len == 0 {
             return None
@@ -671,6 +722,7 @@ impl String {
     ///     assert_eq!(s.shift_byte(), None);
     /// }
     /// ```
+    #[deprecated = "call .as_mut_rev().remove(0)"]
     pub unsafe fn shift_byte(&mut self) -> Option<u8> {
         self.vec.remove(0)
     }
@@ -722,25 +774,31 @@ impl String {
     /// }
     /// assert_eq!(s.as_slice(), "olleh");
     /// ```
+    #[unstable = "the name of this method may be changed"]
     pub unsafe fn as_mut_vec<'a>(&'a mut self) -> &'a mut Vec<u8> {
         &mut self.vec
     }
 }
 
+#[experimental = "collection traits will probably be removed"]
 impl Collection for String {
     #[inline]
+    #[stable]
     fn len(&self) -> uint {
         self.vec.len()
     }
 }
 
+#[experimental = "collection traits will probably be removed"]
 impl Mutable for String {
     #[inline]
+    #[stable]
     fn clear(&mut self) {
         self.vec.clear()
     }
 }
 
+#[experimental = "waiting on FromIterator stabilization"]
 impl FromIterator<char> for String {
     fn from_iter<I:Iterator<char>>(iterator: I) -> String {
         let mut buf = String::new();
@@ -749,6 +807,7 @@ impl FromIterator<char> for String {
     }
 }
 
+#[experimental = "waiting on Extendable stabilization"]
 impl Extendable<char> for String {
     fn extend<I:Iterator<char>>(&mut self, mut iterator: I) {
         for ch in iterator {
@@ -757,8 +816,10 @@ impl Extendable<char> for String {
     }
 }
 
+#[experimental = "waiting on Str stabilization"]
 impl Str for String {
     #[inline]
+    #[stable]
     fn as_slice<'a>(&'a self) -> &'a str {
         unsafe {
             mem::transmute(self.vec.as_slice())
@@ -766,6 +827,7 @@ impl Str for String {
     }
 }
 
+#[experimental = "waiting on StrAllocating stabilization"]
 impl StrAllocating for String {
     #[inline]
     fn into_string(self) -> String {
@@ -773,18 +835,21 @@ impl StrAllocating for String {
     }
 }
 
+#[stable]
 impl Default for String {
     fn default() -> String {
         String::new()
     }
 }
 
+#[experimental = "waiting on Show stabilization"]
 impl fmt::Show for String {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         self.as_slice().fmt(f)
     }
 }
 
+#[experimental = "waiting on Hash stabilization"]
 impl<H: hash::Writer> hash::Hash<H> for String {
     #[inline]
     fn hash(&self, hasher: &mut H) {
@@ -792,6 +857,7 @@ impl<H: hash::Writer> hash::Hash<H> for String {
     }
 }
 
+#[experimental = "waiting on Equiv stabilization"]
 impl<'a, S: Str> Equiv<S> for String {
     #[inline]
     fn equiv(&self, other: &S) -> bool {
@@ -799,6 +865,7 @@ impl<'a, S: Str> Equiv<S> for String {
     }
 }
 
+#[experimental = "waiting on Add stabilization"]
 impl<S: Str> Add<S, String> for String {
     fn add(&self, other: &S) -> String {
         let mut s = String::from_str(self.as_slice());
@@ -808,6 +875,7 @@ impl<S: Str> Add<S, String> for String {
 }
 
 /// Unsafe operations
+#[unstable = "waiting on raw module conventions"]
 pub mod raw {
     use core::mem;
     use core::ptr::RawPtr;

From 31be3319bf5ef1a74ef1044f5bd52dd95947c959 Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Mon, 22 Sep 2014 08:24:14 -0700
Subject: [PATCH 2/3] collections: Deprecate shift_char for insert/remove

This commit deprecates the String::shift_char() function in favor of the
addition of an insert()/remove() pair of functions. This aligns the API with Vec
in that characters can be inserted at arbitrary positions.  Additionaly, there
is no `_char` suffix due to the rationaled laid out in the previous commit.

These functions are both introduced as unstable as their failure semantics,
while in line with slices/vectors, are uncertain about whether they should
remain the same.
---
 src/libcollections/string.rs | 102 +++++++++++++++++++++++++++++------
 1 file changed, 87 insertions(+), 15 deletions(-)

diff --git a/src/libcollections/string.rs b/src/libcollections/string.rs
index 7f29645407c57..cdb6ba1bcd705 100644
--- a/src/libcollections/string.rs
+++ b/src/libcollections/string.rs
@@ -727,37 +727,80 @@ impl String {
         self.vec.remove(0)
     }
 
-    /// Removes the first character from the string buffer and returns it.
-    /// Returns `None` if this string buffer is empty.
+    /// Deprecated, call `remove(0)` instead
+    #[deprecated = "call .remove(0) instead"]
+    pub fn shift_char(&mut self) -> Option<char> {
+        self.remove(0)
+    }
+
+    /// Removes the character from the string buffer at byte position `idx` and
+    /// returns it. Returns `None` if `idx` is out of bounds.
     ///
     /// # Warning
     ///
-    /// This is a O(n) operation as it requires copying every element in the buffer.
+    /// This is a O(n) operation as it requires copying every element in the
+    /// buffer.
+    ///
+    /// # Failure
+    ///
+    /// If `idx` does not lie on a character boundary, then this function will
+    /// fail.
     ///
     /// # Example
     ///
     /// ```
     /// let mut s = String::from_str("foo");
-    /// assert_eq!(s.shift_char(), Some('f'));
-    /// assert_eq!(s.shift_char(), Some('o'));
-    /// assert_eq!(s.shift_char(), Some('o'));
-    /// assert_eq!(s.shift_char(), None);
+    /// assert_eq!(s.remove(0), Some('f'));
+    /// assert_eq!(s.remove(1), Some('o'));
+    /// assert_eq!(s.remove(0), Some('o'));
+    /// assert_eq!(s.remove(0), None);
     /// ```
-    pub fn shift_char(&mut self) -> Option<char> {
+    #[unstable = "the failure semantics of this function and return type \
+                  may change"]
+    pub fn remove(&mut self, idx: uint) -> Option<char> {
         let len = self.len();
-        if len == 0 {
-            return None
-        }
+        if idx >= len { return None }
 
-        let CharRange {ch, next} = self.as_slice().char_range_at(0);
-        let new_len = len - next;
+        let CharRange { ch, next } = self.as_slice().char_range_at(idx);
         unsafe {
-            ptr::copy_memory(self.vec.as_mut_ptr(), self.vec.as_ptr().offset(next as int), new_len);
-            self.vec.set_len(new_len);
+            ptr::copy_memory(self.vec.as_mut_ptr().offset(idx as int),
+                             self.vec.as_ptr().offset(next as int),
+                             len - next);
+            self.vec.set_len(len - (next - idx));
         }
         Some(ch)
     }
 
+    /// Insert a character into the string buffer at byte position `idx`.
+    ///
+    /// # Warning
+    ///
+    /// This is a O(n) operation as it requires copying every element in the
+    /// buffer.
+    ///
+    /// # Failure
+    ///
+    /// If `idx` does not lie on a character boundary or is out of bounds, then
+    /// this function will fail.
+    pub fn insert(&mut self, idx: uint, ch: char) {
+        let len = self.len();
+        assert!(idx <= len);
+        assert!(self.as_slice().is_char_boundary(idx));
+        self.vec.reserve_additional(4);
+        let mut bits = [0, ..4];
+        let amt = ch.encode_utf8(bits).unwrap();
+
+        unsafe {
+            ptr::copy_memory(self.vec.as_mut_ptr().offset((idx + amt) as int),
+                             self.vec.as_ptr().offset(idx as int),
+                             len - idx);
+            ptr::copy_memory(self.vec.as_mut_ptr().offset(idx as int),
+                             bits.as_ptr(),
+                             amt);
+            self.vec.set_len(len + amt);
+        }
+    }
+
     /// Views the string buffer as a mutable sequence of bytes.
     ///
     /// This is unsafe because it does not check
@@ -1209,6 +1252,35 @@ mod tests {
         assert_eq!(b.as_slice(), "1234522");
     }
 
+    #[test]
+    fn remove() {
+        let mut s = "ศไทย中华Việt Nam; foobar".to_string();;
+        assert_eq!(s.remove(0), Some('ศ'));
+        assert_eq!(s.len(), 33);
+        assert_eq!(s.as_slice(), "ไทย中华Việt Nam; foobar");
+        assert_eq!(s.remove(33), None);
+        assert_eq!(s.remove(300), None);
+        assert_eq!(s.remove(17), Some('ệ'));
+        assert_eq!(s.as_slice(), "ไทย中华Vit Nam; foobar");
+    }
+
+    #[test] #[should_fail]
+    fn remove_bad() {
+        "ศ".to_string().remove(1);
+    }
+
+    #[test]
+    fn insert() {
+        let mut s = "foobar".to_string();
+        s.insert(0, 'ệ');
+        assert_eq!(s.as_slice(), "ệfoobar");
+        s.insert(6, 'ย');
+        assert_eq!(s.as_slice(), "ệfooยbar");
+    }
+
+    #[test] #[should_fail] fn insert_bad1() { "".to_string().insert(1, 't'); }
+    #[test] #[should_fail] fn insert_bad2() { "ệ".to_string().insert(1, 't'); }
+
     #[bench]
     fn bench_with_capacity(b: &mut Bencher) {
         b.iter(|| {

From 50375139e2bc69920786411f7b1e05866898ed7a Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Mon, 22 Sep 2014 08:28:35 -0700
Subject: [PATCH 3/3] Deal with the fallout of string stabilization

---
 src/compiletest/runtest.rs      |  3 +-
 src/libcollections/str.rs       |  4 +--
 src/libcollections/string.rs    | 59 +++++++++++++++++++--------------
 src/libgetopts/lib.rs           | 26 +++++++--------
 src/libgraphviz/lib.rs          |  8 +++--
 src/libregex/compile.rs         |  2 +-
 src/libregex/re.rs              |  7 ++--
 src/librustdoc/clean/mod.rs     |  4 +--
 src/librustdoc/externalfiles.rs |  2 +-
 src/librustdoc/html/render.rs   |  3 +-
 src/librustdoc/passes.rs        |  2 +-
 src/libserialize/json.rs        | 22 ++++++------
 src/libstd/path/windows.rs      | 20 +++++------
 13 files changed, 88 insertions(+), 74 deletions(-)

diff --git a/src/compiletest/runtest.rs b/src/compiletest/runtest.rs
index f3e9177fc6e11..268a6020a10a5 100644
--- a/src/compiletest/runtest.rs
+++ b/src/compiletest/runtest.rs
@@ -444,7 +444,8 @@ fn run_debuginfo_gdb_test(config: &Config, props: &TestProps, testfile: &Path) {
                      "-nx".to_string(),
                      format!("-command={}", debugger_script.as_str().unwrap()));
 
-            let gdb_path = tool_path.append("/bin/arm-linux-androideabi-gdb");
+            let mut gdb_path = tool_path;
+            gdb_path.push_str("/bin/arm-linux-androideabi-gdb");
             let procsrv::Result {
                 out,
                 err,
diff --git a/src/libcollections/str.rs b/src/libcollections/str.rs
index f677b170bb367..d198e948ac838 100644
--- a/src/libcollections/str.rs
+++ b/src/libcollections/str.rs
@@ -698,7 +698,7 @@ pub trait StrAllocating: Str {
         let me = self.as_slice();
         let mut out = String::with_capacity(me.len());
         for c in me.chars() {
-            c.escape_default(|c| out.push_char(c));
+            c.escape_default(|c| out.push(c));
         }
         out
     }
@@ -708,7 +708,7 @@ pub trait StrAllocating: Str {
         let me = self.as_slice();
         let mut out = String::with_capacity(me.len());
         for c in me.chars() {
-            c.escape_unicode(|c| out.push_char(c));
+            c.escape_unicode(|c| out.push(c));
         }
         out
     }
diff --git a/src/libcollections/string.rs b/src/libcollections/string.rs
index cdb6ba1bcd705..6843996a9e145 100644
--- a/src/libcollections/string.rs
+++ b/src/libcollections/string.rs
@@ -159,7 +159,7 @@ impl String {
 
         if i > 0 {
             unsafe {
-                res.push_bytes(v.slice_to(i))
+                res.as_mut_vec().push_all(v.slice_to(i))
             };
         }
 
@@ -176,10 +176,10 @@ impl String {
             macro_rules! error(() => ({
                 unsafe {
                     if subseqidx != i_ {
-                        res.push_bytes(v.slice(subseqidx, i_));
+                        res.as_mut_vec().push_all(v.slice(subseqidx, i_));
                     }
                     subseqidx = i;
-                    res.push_bytes(REPLACEMENT);
+                    res.as_mut_vec().push_all(REPLACEMENT);
                 }
             }))
 
@@ -245,7 +245,7 @@ impl String {
         }
         if subseqidx < total {
             unsafe {
-                res.push_bytes(v.slice(subseqidx, total))
+                res.as_mut_vec().push_all(v.slice(subseqidx, total))
             };
         }
         Owned(res.into_string())
@@ -271,7 +271,7 @@ impl String {
         let mut s = String::with_capacity(v.len() / 2);
         for c in str::utf16_items(v) {
             match c {
-                str::ScalarValue(c) => s.push_char(c),
+                str::ScalarValue(c) => s.push(c),
                 str::LoneSurrogate(_) => return None
             }
         }
@@ -332,6 +332,7 @@ impl String {
     /// # Example
     ///
     /// ```
+    /// # #![allow(deprecated)]
     /// let s = String::from_str("hello");
     /// let big = s.append(" ").append("world").append("!");
     /// // s has now been moved and cannot be used
@@ -362,11 +363,11 @@ impl String {
         }
 
         let mut buf = String::new();
-        buf.push_char(ch);
+        buf.push(ch);
         let size = buf.len() * length;
         buf.reserve(size);
         for _ in range(1, length) {
-            buf.push_char(ch)
+            buf.push(ch)
         }
         buf
     }
@@ -380,6 +381,7 @@ impl String {
     /// # Example
     ///
     /// ```rust
+    /// # #![allow(deprecated)]
     /// let s = String::from_byte(104);
     /// assert_eq!(s.as_slice(), "h");
     /// ```
@@ -417,7 +419,7 @@ impl String {
     #[unstable = "duplicate of iterator-based functionality"]
     pub fn grow(&mut self, count: uint, ch: char) {
         for _ in range(0, count) {
-            self.push_char(ch)
+            self.push(ch)
         }
     }
 
@@ -426,6 +428,7 @@ impl String {
     /// # Example
     ///
     /// ```
+    /// # #![allow(deprecated)]
     /// let s = String::with_capacity(10);
     /// assert!(s.byte_capacity() >= 10);
     /// ```
@@ -441,7 +444,7 @@ impl String {
     ///
     /// ```
     /// let s = String::with_capacity(10);
-    /// assert!(s.byte_capacity() >= 10);
+    /// assert!(s.capacity() >= 10);
     /// ```
     #[inline]
     #[unstable = "just implemented, needs to prove itself"]
@@ -455,9 +458,9 @@ impl String {
     ///
     /// ```
     /// let mut s = String::with_capacity(10);
-    /// let before = s.byte_capacity();
+    /// let before = s.capacity();
     /// s.reserve_additional(100);
-    /// assert!(s.byte_capacity() - before >= 100);
+    /// assert!(s.capacity() - before >= 100);
     /// ```
     #[inline]
     pub fn reserve_additional(&mut self, extra: uint) {
@@ -471,7 +474,7 @@ impl String {
     /// ```
     /// let mut s = String::new();
     /// s.reserve(10);
-    /// assert!(s.byte_capacity() >= 10);
+    /// assert!(s.capacity() >= 10);
     /// ```
     #[inline]
     pub fn reserve(&mut self, capacity: uint) {
@@ -485,7 +488,7 @@ impl String {
     /// ```
     /// let mut s = String::new();
     /// s.reserve_exact(10);
-    /// assert_eq!(s.byte_capacity(), 10);
+    /// assert_eq!(s.capacity(), 10);
     /// ```
     #[inline]
     pub fn reserve_exact(&mut self, capacity: uint) {
@@ -499,9 +502,9 @@ impl String {
     /// ```
     /// let mut s = String::from_str("foo");
     /// s.reserve(100);
-    /// assert!(s.byte_capacity() >= 100);
+    /// assert!(s.capacity() >= 100);
     /// s.shrink_to_fit();
-    /// assert_eq!(s.byte_capacity(), 3);
+    /// assert_eq!(s.capacity(), 3);
     /// ```
     #[inline]
     pub fn shrink_to_fit(&mut self) {
@@ -527,7 +530,7 @@ impl String {
     /// assert_eq!(s.as_slice(), "abc123");
     /// ```
     #[inline]
-    #[stable = "function just renamed from push_char"]
+    #[stable = "function just renamed from push"]
     pub fn push(&mut self, ch: char) {
         let cur_len = self.len();
         // This may use up to 4 bytes.
@@ -552,6 +555,7 @@ impl String {
     /// # Example
     ///
     /// ```
+    /// # #![allow(deprecated)]
     /// let mut s = String::new();
     /// unsafe {
     ///     s.push_bytes([104, 101, 108, 108, 111]);
@@ -587,6 +591,7 @@ impl String {
     /// # Example
     ///
     /// ```
+    /// # #![allow(deprecated)]
     /// let mut s = String::from_str("hello");
     /// unsafe {
     ///     let bytes = s.as_mut_bytes();
@@ -598,7 +603,7 @@ impl String {
     /// assert_eq!(s.as_slice(), "h3ll0")
     /// ```
     #[inline]
-    #[deprecated = "call .as_mut_vec().as_slice() instead"]
+    #[deprecated = "call .as_mut_vec().as_mut_slice() instead"]
     pub unsafe fn as_mut_bytes<'a>(&'a mut self) -> &'a mut [u8] {
         self.vec.as_mut_slice()
     }
@@ -631,6 +636,7 @@ impl String {
     /// # Example
     ///
     /// ```
+    /// # #![allow(deprecated)]
     /// let mut s = String::from_str("hell");
     /// unsafe {
     ///     s.push_byte(111);
@@ -652,6 +658,7 @@ impl String {
     /// # Example
     ///
     /// ```
+    /// # #![allow(deprecated)]
     /// let mut s = String::from_str("foo");
     /// unsafe {
     ///     assert_eq!(s.pop_byte(), Some(111));
@@ -714,6 +721,7 @@ impl String {
     /// # Example
     ///
     /// ```
+    /// # #![allow(deprecated)]
     /// let mut s = String::from_str("foo");
     /// unsafe {
     ///     assert_eq!(s.shift_byte(), Some(102));
@@ -722,7 +730,7 @@ impl String {
     ///     assert_eq!(s.shift_byte(), None);
     /// }
     /// ```
-    #[deprecated = "call .as_mut_rev().remove(0)"]
+    #[deprecated = "call .as_mut_vec().remove(0)"]
     pub unsafe fn shift_byte(&mut self) -> Option<u8> {
         self.vec.remove(0)
     }
@@ -782,6 +790,7 @@ impl String {
     ///
     /// If `idx` does not lie on a character boundary or is out of bounds, then
     /// this function will fail.
+    #[unstable = "the failure semantics of this function are uncertain"]
     pub fn insert(&mut self, idx: uint, ch: char) {
         let len = self.len();
         assert!(idx <= len);
@@ -854,7 +863,7 @@ impl FromIterator<char> for String {
 impl Extendable<char> for String {
     fn extend<I:Iterator<char>>(&mut self, mut iterator: I) {
         for ch in iterator {
-            self.push_char(ch)
+            self.push(ch)
         }
     }
 }
@@ -1171,13 +1180,13 @@ mod tests {
     }
 
     #[test]
-    fn test_push_char() {
+    fn test_push() {
         let mut data = String::from_str("ประเทศไทย中");
-        data.push_char('华');
-        data.push_char('b'); // 1 byte
-        data.push_char('¢'); // 2 byte
-        data.push_char('€'); // 3 byte
-        data.push_char('𤭢'); // 4 byte
+        data.push('华');
+        data.push('b'); // 1 byte
+        data.push('¢'); // 2 byte
+        data.push('€'); // 3 byte
+        data.push('𤭢'); // 4 byte
         assert_eq!(data.as_slice(), "ประเทศไทย中华b¢€𤭢");
     }
 
diff --git a/src/libgetopts/lib.rs b/src/libgetopts/lib.rs
index f95ecb412d177..cac67ef404149 100644
--- a/src/libgetopts/lib.rs
+++ b/src/libgetopts/lib.rs
@@ -685,9 +685,9 @@ pub fn usage(brief: &str, opts: &[OptGroup]) -> String {
         match short_name.len() {
             0 => {}
             1 => {
-                row.push_char('-');
+                row.push('-');
                 row.push_str(short_name.as_slice());
-                row.push_char(' ');
+                row.push(' ');
             }
             _ => fail!("the short name should only be 1 ascii char long"),
         }
@@ -698,7 +698,7 @@ pub fn usage(brief: &str, opts: &[OptGroup]) -> String {
             _ => {
                 row.push_str("--");
                 row.push_str(long_name.as_slice());
-                row.push_char(' ');
+                row.push(' ');
             }
         }
 
@@ -707,9 +707,9 @@ pub fn usage(brief: &str, opts: &[OptGroup]) -> String {
             No => {}
             Yes => row.push_str(hint.as_slice()),
             Maybe => {
-                row.push_char('[');
+                row.push('[');
                 row.push_str(hint.as_slice());
-                row.push_char(']');
+                row.push(']');
             }
         }
 
@@ -718,7 +718,7 @@ pub fn usage(brief: &str, opts: &[OptGroup]) -> String {
         let rowlen = row.as_slice().char_len();
         if rowlen < 24 {
             for _ in range(0, 24 - rowlen) {
-                row.push_char(' ');
+                row.push(' ');
             }
         } else {
             row.push_str(desc_sep.as_slice())
@@ -728,7 +728,7 @@ pub fn usage(brief: &str, opts: &[OptGroup]) -> String {
         let mut desc_normalized_whitespace = String::new();
         for word in desc.as_slice().words() {
             desc_normalized_whitespace.push_str(word);
-            desc_normalized_whitespace.push_char(' ');
+            desc_normalized_whitespace.push(' ');
         }
 
         // FIXME: #5516 should be graphemes not codepoints
@@ -755,12 +755,12 @@ fn format_option(opt: &OptGroup) -> String {
     let mut line = String::new();
 
     if opt.occur != Req {
-        line.push_char('[');
+        line.push('[');
     }
 
     // Use short_name is possible, but fallback to long_name.
     if opt.short_name.len() > 0 {
-        line.push_char('-');
+        line.push('-');
         line.push_str(opt.short_name.as_slice());
     } else {
         line.push_str("--");
@@ -768,18 +768,18 @@ fn format_option(opt: &OptGroup) -> String {
     }
 
     if opt.hasarg != No {
-        line.push_char(' ');
+        line.push(' ');
         if opt.hasarg == Maybe {
-            line.push_char('[');
+            line.push('[');
         }
         line.push_str(opt.hint.as_slice());
         if opt.hasarg == Maybe {
-            line.push_char(']');
+            line.push(']');
         }
     }
 
     if opt.occur != Req {
-        line.push_char(']');
+        line.push(']');
     }
     if opt.occur == Multi {
         line.push_str("..");
diff --git a/src/libgraphviz/lib.rs b/src/libgraphviz/lib.rs
index 19f58a210d00b..4ae7f882212f0 100644
--- a/src/libgraphviz/lib.rs
+++ b/src/libgraphviz/lib.rs
@@ -426,7 +426,7 @@ impl<'a> LabelText<'a> {
     fn escape_str(s: &str) -> String {
         let mut out = String::with_capacity(s.len());
         for c in s.chars() {
-            LabelText::escape_char(c, |c| out.push_char(c));
+            LabelText::escape_char(c, |c| out.push(c));
         }
         out
     }
@@ -461,9 +461,11 @@ impl<'a> LabelText<'a> {
 
     /// Puts `suffix` on a line below this label, with a blank line separator.
     pub fn suffix_line(self, suffix: LabelText) -> LabelText<'static> {
-        let prefix = self.pre_escaped_content().into_string();
+        let mut prefix = self.pre_escaped_content().into_string();
         let suffix = suffix.pre_escaped_content();
-        EscStr(str::Owned(prefix.append(r"\n\n").append(suffix.as_slice())))
+        prefix.push_str(r"\n\n");
+        prefix.push_str(suffix.as_slice());
+        EscStr(str::Owned(prefix))
     }
 }
 
diff --git a/src/libregex/compile.rs b/src/libregex/compile.rs
index 91c3da001628e..a0fefd2421427 100644
--- a/src/libregex/compile.rs
+++ b/src/libregex/compile.rs
@@ -105,7 +105,7 @@ impl Program {
         let mut pre = String::with_capacity(5);
         for inst in c.insts.slice_from(1).iter() {
             match *inst {
-                OneChar(c, FLAG_EMPTY) => pre.push_char(c),
+                OneChar(c, FLAG_EMPTY) => pre.push(c),
                 _ => break
             }
         }
diff --git a/src/libregex/re.rs b/src/libregex/re.rs
index c2578d227ee38..0c9b3a79bdf65 100644
--- a/src/libregex/re.rs
+++ b/src/libregex/re.rs
@@ -26,9 +26,9 @@ pub fn quote(text: &str) -> String {
     let mut quoted = String::with_capacity(text.len());
     for c in text.chars() {
         if parse::is_punct(c) {
-            quoted.push_char('\\')
+            quoted.push('\\')
         }
-        quoted.push_char(c);
+        quoted.push(c);
     }
     quoted
 }
@@ -504,7 +504,8 @@ impl Regex {
             new.push_str(rep.reg_replace(&cap).as_slice());
             last_match = e;
         }
-        new.append(text.slice(last_match, text.len()))
+        new.push_str(text.slice(last_match, text.len()));
+        return new;
     }
 
     /// Returns the original string of this regex.
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index 660b6c7ade2b2..5ac7baaa6d10a 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -1961,9 +1961,9 @@ fn lit_to_string(lit: &ast::Lit) -> String {
         ast::LitByte(b) => {
             let mut res = String::from_str("b'");
             (b as char).escape_default(|c| {
-                res.push_char(c);
+                res.push(c);
             });
-            res.push_char('\'');
+            res.push('\'');
             res
         },
         ast::LitChar(c) => format!("'{}'", c),
diff --git a/src/librustdoc/externalfiles.rs b/src/librustdoc/externalfiles.rs
index 0931f132c0214..08fb94a801c3e 100644
--- a/src/librustdoc/externalfiles.rs
+++ b/src/librustdoc/externalfiles.rs
@@ -64,7 +64,7 @@ pub fn load_external_files(names: &[String]) -> Option<String> {
     let mut out = String::new();
     for name in names.iter() {
         out.push_str(load_or_return!(name.as_slice(), None, None).as_slice());
-        out.push_char('\n');
+        out.push('\n');
     }
     Some(out)
 }
diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs
index c831015e53936..fe7b1be3eca68 100644
--- a/src/librustdoc/html/render.rs
+++ b/src/librustdoc/html/render.rs
@@ -1080,7 +1080,8 @@ impl Context {
             let mut json_out = BufferedWriter::new(try!(File::create(json_dst)));
             try!(stability.encode(&mut json::Encoder::new(&mut json_out)));
 
-            let title = stability.name.clone().append(" - Stability dashboard");
+            let mut title = stability.name.clone();
+            title.push_str(" - Stability dashboard");
             let desc = format!("API stability overview for the Rust `{}` crate.",
                                this.layout.krate);
             let page = layout::Page {
diff --git a/src/librustdoc/passes.rs b/src/librustdoc/passes.rs
index 33337594b4c67..3c66a7c785094 100644
--- a/src/librustdoc/passes.rs
+++ b/src/librustdoc/passes.rs
@@ -276,7 +276,7 @@ pub fn collapse_docs(krate: clean::Crate) -> plugins::PluginResult {
                     clean::NameValue(ref x, ref s)
                             if "doc" == x.as_slice() => {
                         docstr.push_str(s.as_slice());
-                        docstr.push_char('\n');
+                        docstr.push('\n');
                     },
                     _ => ()
                 }
diff --git a/src/libserialize/json.rs b/src/libserialize/json.rs
index 14274ef9f9b04..4d9bb87d133a4 100644
--- a/src/libserialize/json.rs
+++ b/src/libserialize/json.rs
@@ -1514,14 +1514,14 @@ impl<T: Iterator<char>> Parser<T> {
 
             if escape {
                 match self.ch_or_null() {
-                    '"' => res.push_char('"'),
-                    '\\' => res.push_char('\\'),
-                    '/' => res.push_char('/'),
-                    'b' => res.push_char('\x08'),
-                    'f' => res.push_char('\x0c'),
-                    'n' => res.push_char('\n'),
-                    'r' => res.push_char('\r'),
-                    't' => res.push_char('\t'),
+                    '"' => res.push('"'),
+                    '\\' => res.push('\\'),
+                    '/' => res.push('/'),
+                    'b' => res.push('\x08'),
+                    'f' => res.push('\x0c'),
+                    'n' => res.push('\n'),
+                    'r' => res.push('\r'),
+                    't' => res.push('\t'),
                     'u' => match try!(self.decode_hex_escape()) {
                         0xDC00 .. 0xDFFF => return self.error(LoneLeadingSurrogateInHexEscape),
 
@@ -1535,13 +1535,13 @@ impl<T: Iterator<char>> Parser<T> {
 
                             let buf = [n1, try!(self.decode_hex_escape())];
                             match str::utf16_items(buf.as_slice()).next() {
-                                Some(ScalarValue(c)) => res.push_char(c),
+                                Some(ScalarValue(c)) => res.push(c),
                                 _ => return self.error(LoneLeadingSurrogateInHexEscape),
                             }
                         }
 
                         n => match char::from_u32(n as u32) {
-                            Some(c) => res.push_char(c),
+                            Some(c) => res.push(c),
                             None => return self.error(InvalidUnicodeCodePoint),
                         },
                     },
@@ -1556,7 +1556,7 @@ impl<T: Iterator<char>> Parser<T> {
                         self.bump();
                         return Ok(res);
                     },
-                    Some(c) => res.push_char(c),
+                    Some(c) => res.push(c),
                     None => unreachable!()
                 }
             }
diff --git a/src/libstd/path/windows.rs b/src/libstd/path/windows.rs
index 3f598e5280624..803db4848ada5 100644
--- a/src/libstd/path/windows.rs
+++ b/src/libstd/path/windows.rs
@@ -212,7 +212,7 @@ impl GenericPathUnsafe for Path {
             None if ".." == self.repr.as_slice() => {
                 let mut s = String::with_capacity(3 + filename.len());
                 s.push_str("..");
-                s.push_char(SEP);
+                s.push(SEP);
                 s.push_str(filename);
                 self.update_normalized(s);
             }
@@ -222,7 +222,7 @@ impl GenericPathUnsafe for Path {
             Some((_,idxa,end)) if self.repr.as_slice().slice(idxa,end) == ".." => {
                 let mut s = String::with_capacity(end + 1 + filename.len());
                 s.push_str(self.repr.as_slice().slice_to(end));
-                s.push_char(SEP);
+                s.push(SEP);
                 s.push_str(filename);
                 self.update_normalized(s);
             }
@@ -235,7 +235,7 @@ impl GenericPathUnsafe for Path {
             Some((idxb,_,_)) => {
                 let mut s = String::with_capacity(idxb + 1 + filename.len());
                 s.push_str(self.repr.as_slice().slice_to(idxb));
-                s.push_char(SEP);
+                s.push(SEP);
                 s.push_str(filename);
                 self.update_normalized(s);
             }
@@ -299,7 +299,7 @@ impl GenericPathUnsafe for Path {
             match me.prefix {
                 Some(DiskPrefix) if me.repr.len() == plen => (),
                 _ if !(me.repr.len() > plen && me.repr.as_bytes()[me.repr.len()-1] == SEP_BYTE) => {
-                    s.push_char(SEP);
+                    s.push(SEP);
                 }
                 _ => ()
             }
@@ -745,7 +745,7 @@ impl Path {
                 Some(VerbatimUNCPrefix(x, 0)) if s.len() == 8 + x => {
                     // the server component has no trailing '\'
                     let mut s = String::from_str(s);
-                    s.push_char(SEP);
+                    s.push(SEP);
                     Some(s)
                 }
                 _ => None
@@ -815,20 +815,20 @@ impl Path {
                         let mut s = String::with_capacity(n);
                         match prefix {
                             Some(DiskPrefix) => {
-                                s.push_char(prefix_.as_bytes()[0].to_ascii()
+                                s.push(prefix_.as_bytes()[0].to_ascii()
                                                    .to_uppercase().to_char());
-                                s.push_char(':');
+                                s.push(':');
                             }
                             Some(VerbatimDiskPrefix) => {
                                 s.push_str(prefix_.slice_to(4));
-                                s.push_char(prefix_.as_bytes()[4].to_ascii()
+                                s.push(prefix_.as_bytes()[4].to_ascii()
                                                    .to_uppercase().to_char());
                                 s.push_str(prefix_.slice_from(5));
                             }
                             Some(UNCPrefix(a,b)) => {
                                 s.push_str("\\\\");
                                 s.push_str(prefix_.slice(2, a+2));
-                                s.push_char(SEP);
+                                s.push(SEP);
                                 s.push_str(prefix_.slice(3+a, 3+a+b));
                             }
                             Some(_) => s.push_str(prefix_),
@@ -842,7 +842,7 @@ impl Path {
                             }
                         }
                         for comp in it {
-                            s.push_char(SEP);
+                            s.push(SEP);
                             s.push_str(comp);
                         }
                         Some(s)