Consistency work on Addable and Growable.

Seq-derived classes. Creating GrowingBuilder to complement AddingBuilder
on classes with += but not +. Fixed some inconsistencies I came across
in the process. No review.

10 files changed, 88 insertions, 67 deletions

diff --git a/src/library/scala/collection/generic/Addable.scala b/src/library/scala/collection/generic/Addable.scala
index 9686e96c09..5cbc7951cf 100644
--- a/src/library/scala/collection/generic/Addable.scala
+++ b/src/library/scala/collection/generic/Addable.scala
@@ -61,7 +61,3 @@ trait Addable[A, +Repr <: Addable[A, Repr]] { self =>
    */
   def ++ (iter: Iterator[A]): Repr = (repr /: iter) (_ + _)
 }
-
-
-
-
diff --git a/src/library/scala/collection/generic/Growable.scala b/src/library/scala/collection/generic/Growable.scala
index fc3712d81e..93fe1fcb59 100644
--- a/src/library/scala/collection/generic/Growable.scala
+++ b/src/library/scala/collection/generic/Growable.scala
@@ -60,7 +60,3 @@ trait Growable[-A] {
    */
   def clear()
 }
-
-
-
-
diff --git a/src/library/scala/collection/mutable/AddingBuilder.scala b/src/library/scala/collection/mutable/AddingBuilder.scala
index 06822e859b..d16a4a71f3 100644
--- a/src/library/scala/collection/mutable/AddingBuilder.scala
+++ b/src/library/scala/collection/mutable/AddingBuilder.scala
@@ -24,7 +24,7 @@ import generic._
  *  @version 2.8
  *  @since 2.8
  */
-class AddingBuilder[Elem, To <: Addable[Elem, To] with scala.collection.Iterable[Elem] with scala.collection.IterableLike[Elem, To]](empty: To)
+class AddingBuilder[Elem, To <: Addable[Elem, To] with collection.Iterable[Elem] with collection.IterableLike[Elem, To]](empty: To)
 extends Builder[Elem, To] {
   protected var elems: To = empty
   def +=(x: Elem): this.type = { elems = elems + x; this }
diff --git a/src/library/scala/collection/mutable/BufferLike.scala b/src/library/scala/collection/mutable/BufferLike.scala
index eeed35b3e7..5797bf0c55 100644
--- a/src/library/scala/collection/mutable/BufferLike.scala
+++ b/src/library/scala/collection/mutable/BufferLike.scala
@@ -60,12 +60,14 @@ trait BufferLike[A, +This <: BufferLike[A, This] with Buffer[A]]
                 extends Growable[A]
                    with Shrinkable[A]
                    with Scriptable[A]
-                   with Addable[A, This]
                    with Subtractable[A, This]
                    with Cloneable[This]
                    with SeqLike[A, This]
 { self : This =>
 
+  // Note this does not extend Addable because `+` is being phased out of
+  // all Seq-derived classes.
+
   import scala.collection.{Iterable, Traversable}
 
   // Abstract methods from IndexedSeq:
@@ -254,32 +256,24 @@ trait BufferLike[A, +This <: BufferLike[A, This] with Buffer[A]]
   @deprecated("use `+=:' instead")
   final def +:(elem: A): This = +=:(elem)
 
-  // !!! Note that the two + methods below presently still modify in place.  Arguably they
-  // should be changed to create new collections, and then have BOTH @migration and
-  // @deprecation annotations, because Seq-derived classes don't implement + since
-  // most of them are covariant and + interacts badly with +'s autostringification.
-  // However the compiler right now will not warn about both annotations (see the
-  // implementation comment) so I don't think that's an option without fixing that first.
-  //
-  // In addition, I don't think BufferLike should implement Addable since it
-  // deprecates all of it, and Addable has never shipped.
-  //
-  // Alternate implementations:
-  //
-  // clone() += elem
-  // clone() += elem1 += elem2 ++= elems
-
   /** Adds a single element to this collection and returns
-   *  the collection itself.
+   *  the collection itself.  Note that for backward compatibility
+   *  reasons, this method mutates the collection in place, unlike
+   *  similar but undeprecated methods throughout the collections
+   *  hierarchy.  You are strongly recommended to use '+=' instead.
    *
    *  @param elem  the element to add.
    */
   @deprecated("Use += instead if you intend to add by side effect to an existing collection.\n"+
               "Use `clone() +=' if you intend to create a new collection.")
-  override def + (elem: A): This = { +=(elem); repr }
+  def + (elem: A): This = { +=(elem); repr }
 
   /** Adds two or more elements to this collection and returns
-   *  the collection itself.
+   *  the collection itself.  Note that for backward compatibility
+   *  reasons, this method mutates the collection in place, unlike
+   *  all similar methods throughout the collections hierarchy.
+   *  similar but undeprecated methods throughout the collections
+   *  hierarchy.  You are strongly recommended to use '++=' instead.
    *
    *  @param elem1 the first element to add.
    *  @param elem2 the second element to add.
@@ -287,7 +281,7 @@ trait BufferLike[A, +This <: BufferLike[A, This] with Buffer[A]]
    */
   @deprecated("Use ++= instead if you intend to add by side effect to an existing collection.\n"+
               "Use `clone() ++=' if you intend to create a new collection.")
-  override def + (elem1: A, elem2: A, elems: A*): This = {
+  def + (elem1: A, elem2: A, elems: A*): This = {
     this += elem1 += elem2 ++= elems
     repr
   }
@@ -301,7 +295,7 @@ trait BufferLike[A, +This <: BufferLike[A, This] with Buffer[A]]
     "As of 2.8, ++ always creates a new collection, even on Buffers.\n"+
     "Use ++= instead if you intend to add by side effect to an existing collection.\n"
   )
-  override def ++(iter: Traversable[A]): This = clone() ++= iter
+  def ++(iter: Traversable[A]): This = clone() ++= iter
 
   /** Adds a number of elements provided by an iterator and returns
    *  the collection itself.
@@ -312,7 +306,7 @@ trait BufferLike[A, +This <: BufferLike[A, This] with Buffer[A]]
     "As of 2.8, ++ always creates a new collection, even on Buffers.\n"+
     "Use ++= instead if you intend to add by side effect to an existing collection.\n"
   )
-  override def ++ (iter: Iterator[A]): This = clone() ++= iter
+  def ++ (iter: Iterator[A]): This = clone() ++= iter
 
   /** Removes a single element from this collection and returns
    *  the collection itself.
diff --git a/src/library/scala/collection/mutable/GrowingBuilder.scala b/src/library/scala/collection/mutable/GrowingBuilder.scala
new file mode 100644
index 0000000000..259df9a434
--- /dev/null
+++ b/src/library/scala/collection/mutable/GrowingBuilder.scala
@@ -0,0 +1,30 @@
+/*                     __                                               *\
+**     ________ ___   / /  ___     Scala API                            **
+**    / __/ __// _ | / /  / _ |    (c) 2003-2010, LAMP/EPFL             **
+**  __\ \/ /__/ __ |/ /__/ __ |    http://scala-lang.org/               **
+** /____/\___/_/ |_/____/_/ | |                                         **
+**                          |/                                          **
+\*                                                                      */
+
+package scala.collection
+package mutable
+
+import generic._
+
+/** The canonical builder for collections that are growable, i.e. that support an
+ *  efficient `+=` method which adds an element to the collection.  It is
+ *  almost identical to AddingBuilder, but necessitated by the existence of
+ *  classes which are Growable but not Addable, which is a result of covariance
+ *  interacting surprisingly with any2stringadd thus driving '+' out of the Seq
+ *  hierachy.  The tendrils of original sin should never be underestimated.
+ *
+ *  @author Paul Phillips
+ *  @version 2.8
+ *  @since 2.8
+ */
+class GrowingBuilder[Elem, To <: Growable[Elem]](empty: To) extends Builder[Elem, To] {
+  protected var elems: To = empty
+  def +=(x: Elem): this.type = { elems += x; this }
+  def clear() { elems = empty }
+  def result: To = elems
+}
diff --git a/src/library/scala/collection/mutable/LinearSeq.scala b/src/library/scala/collection/mutable/LinearSeq.scala
index 25d7ef6be8..04191e30bc 100644
--- a/src/library/scala/collection/mutable/LinearSeq.scala
+++ b/src/library/scala/collection/mutable/LinearSeq.scala
@@ -14,8 +14,8 @@ package mutable
 
 import generic._
 
-/** A subtrait of <code>collection.Seq</code> which represents sequences
- *  that cannot be mutated.
+/** A subtrait of <code>collection.LinearSeq</code> which represents sequences
+ *  that can be mutated.
  *
  *  @since 2.8
  */
diff --git a/src/library/scala/collection/mutable/ListBuffer.scala b/src/library/scala/collection/mutable/ListBuffer.scala
index 92a69d671f..09ab074783 100644
--- a/src/library/scala/collection/mutable/ListBuffer.scala
+++ b/src/library/scala/collection/mutable/ListBuffer.scala
@@ -337,5 +337,5 @@ final class ListBuffer[A]
  */
 object ListBuffer extends SeqFactory[ListBuffer] {
   implicit def canBuildFrom[A]: CanBuildFrom[Coll, A, ListBuffer[A]] = new GenericCanBuildFrom[A]
-  def newBuilder[A]: Builder[A, ListBuffer[A]] = new AddingBuilder(new ListBuffer[A])
+  def newBuilder[A]: Builder[A, ListBuffer[A]] = new GrowingBuilder(new ListBuffer[A])
 }
diff --git a/src/library/scala/collection/mutable/PriorityQueue.scala b/src/library/scala/collection/mutable/PriorityQueue.scala
index c4dac9effb..8782d09c73 100644
--- a/src/library/scala/collection/mutable/PriorityQueue.scala
+++ b/src/library/scala/collection/mutable/PriorityQueue.scala
@@ -13,7 +13,7 @@ package scala.collection
 package mutable
 
 import generic._
-
+import annotation.migration
 
 /** This class implements priority queues using a heap.
  *  To prioritize elements of type T there must be an implicit
@@ -28,7 +28,6 @@ import generic._
 class PriorityQueue[A](implicit ord: Ordering[A])
       extends Seq[A]
       with SeqLike[A, PriorityQueue[A]]
-      with Addable[A, PriorityQueue[A]]
       with Growable[A]
       with Cloneable[PriorityQueue[A]]
       with Builder[A, PriorityQueue[A]]
@@ -47,8 +46,8 @@ class PriorityQueue[A](implicit ord: Ordering[A])
 
   private val resarr = new ResizableArrayAccess[A]
 
-  resarr.p_size0 += 1                                    // we do not use array(0)
-  override def length: Int = resarr.length - 1   // adjust length accordingly
+  resarr.p_size0 += 1                           // we do not use array(0)
+  override def length: Int = resarr.length - 1  // adjust length accordingly
   override def size: Int = length
   override def isEmpty: Boolean = resarr.p_size0 < 2
   override def repr = this
@@ -116,6 +115,23 @@ class PriorityQueue[A](implicit ord: Ordering[A])
     }
   }
 
+  @deprecated(
+    "Use += instead if you intend to add by side effect to an existing collection.\n"+
+    "Use `clone() +=' if you intend to create a new collection."
+  )
+  def +(elem: A): PriorityQueue[A] = { this.clone() += elem }
+
+  /** Add two or more elements to this set.
+   *  @param    elem1 the first element.
+   *  @param    kv2 the second element.
+   *  @param    kvs the remaining elements.
+   */
+  @deprecated(
+    "Use ++= instead if you intend to add by side effect to an existing collection.\n"+
+    "Use `clone() ++=' if you intend to create a new collection."
+  )
+  def +(elem1: A, elem2: A, elems: A*) = { this.clone().+=(elem1, elem2, elems : _*) }
+
   /** Inserts a single element into the priority queue.
    *
    *  @param  elem        the element to insert
@@ -128,27 +144,18 @@ class PriorityQueue[A](implicit ord: Ordering[A])
     this
   }
 
-  def +(elem: A): PriorityQueue[A] = { this.clone() += elem }
-
-  /** Add two or more elements to this set.
-   *  @param    elem1 the first element.
-   *  @param    kv2 the second element.
-   *  @param    kvs the remaining elements.
-   */
-  override def +(elem1: A, elem2: A, elems: A*) = { this.clone().+=(elem1, elem2, elems : _*) }
-
   /** Adds all elements provided by an <code>Iterable</code> object
    *  into the priority queue.
    *
    *  @param  iter        an iterable object
    */
-  override def ++(elems: scala.collection.Traversable[A]) = { this.clone() ++= elems }
+  def ++(elems: scala.collection.Traversable[A]) = { this.clone() ++= elems }
 
   /** Adds all elements provided by an iterator into the priority queue.
    *
    *  @param  it        an iterator
    */
-  override def ++(iter: Iterator[A]) = { this.clone() ++= iter } // ...whereas this doesn't?
+  def ++(iter: Iterator[A]) = { this.clone() ++= iter }
 
   /** Adds all elements to the queue.
    *
@@ -267,9 +274,9 @@ class PriorityQueue[A](implicit ord: Ordering[A])
   // }
 }
 
-
-
-
-
-
-
+// !!! TODO - but no SortedSeqFactory (yet?)
+// object PriorityQueue extends SeqFactory[PriorityQueue] {
+//   def empty[A](implicit ord: Ordering[A]): PriorityQueue[A] = new PriorityQueue[A](ord)
+//   implicit def canBuildFrom[A](implicit ord: Ordering[A]): CanBuildFrom[Coll, A, PriorityQueue] =
+// }
+//
diff --git a/src/library/scala/collection/mutable/Queue.scala b/src/library/scala/collection/mutable/Queue.scala
index 3b09ceba91..6b32f0f94e 100644
--- a/src/library/scala/collection/mutable/Queue.scala
+++ b/src/library/scala/collection/mutable/Queue.scala
@@ -24,7 +24,6 @@ import generic._
  */
 @serializable @cloneable
 class Queue[A] extends MutableList[A] with Cloneable[Queue[A]] {
-
   /** Adds all elements to the queue.
    *
    *  @param  elems       the elements to add.
@@ -144,3 +143,9 @@ class Queue[A] extends MutableList[A] with Cloneable[Queue[A]] {
    */
   def front: A = first0.elem
 }
+
+// !!! TODO
+// object Queue extends SeqFactory[Queue] {
+//   implicit def canBuildFrom[A]: CanBuildFrom[Coll, A, Queue[A]] = new GenericCanBuildFrom[A]
+//   def newBuilder[A]: Builder[A, Queue[A]] = new GrowingBuilder(new Queue[A])
+// }
diff --git a/src/library/scala/collection/mutable/SetBuilder.scala b/src/library/scala/collection/mutable/SetBuilder.scala
index 6286c46ac4..450d76463c 100644
--- a/src/library/scala/collection/mutable/SetBuilder.scala
+++ b/src/library/scala/collection/mutable/SetBuilder.scala
@@ -13,17 +13,10 @@ package mutable
 
 import generic._
 
-/** The canonical builder for collections that are addable, i.e. that support
- *  an efficient + method which adds an element to the collection.
- *  Collections are built from their empty element using this + method.
- *  @param empty   The empty element of the collection.
+/** The canonical builder for mutable Sets.
  *
+ *  @param empty   The empty element of the collection.
  *  @since 2.8
  */
-class SetBuilder[A, Coll <: Addable[A, Coll] with scala.collection.Iterable[A] with scala.collection.IterableLike[A, Coll]](empty: Coll)
-extends Builder[A, Coll] {
-  protected var elems: Coll = empty
-  def +=(x: A): this.type = { elems = elems + x; this }
-  def clear() { elems = empty }
-  def result: Coll = elems
-}
+class SetBuilder[A, Coll <: Addable[A, Coll] with collection.Iterable[A] with collection.IterableLike[A, Coll]](empty: Coll)
+extends AddingBuilder[A, Coll](empty) { }