Skip to content
This repository was archived by the owner on Dec 22, 2021. It is now read-only.

Retrofit BuildFrom into the new design #97

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
172 changes: 158 additions & 14 deletions src/main/scala/strawman/collection/Factories.scala
Original file line number Diff line number Diff line change
@@ -1,20 +1,106 @@
package strawman
package collection

import strawman.collection.mutable.Builder
import scala.language.implicitConversions

import strawman.collection.mutable.{ArrayBuffer, Builder}

import scala.{Any, Int, Nothing, Ordering}
import scala.annotation.unchecked.uncheckedVariance


/** Builds a collection of type `C` from elements of type `A` when a source collection of type `From` is available.
* Implicit instances of `BuildFrom` are available for all collection types.
*
* @tparam From Type of source collection
* @tparam A Type of elements (e.g. `Int`, `Boolean`, etc.)
* @tparam C Type of collection (e.g. `List[Int]`, `TreeMap[Int, String]`, etc.)
*/
trait BuildFrom[-From, -A, +C] extends Any {
def fromSpecificIterable(from: From)(it: Iterable[A]): C
}

object BuildFrom extends BuildFromLowPriority {
/** Build the source collection type from a MapOps */
implicit def buildFromMapOps[CC[K, V] <: Map[K, V] with MapOps[K, V, CC, _], A, B, E, F]: BuildFrom[CC[A, B], (E, F), CC[E, F]] = new BuildFrom[CC[A, B], (E, F), CC[E, F]] {
//TODO: Reuse a prototype instance
def fromSpecificIterable(from: CC[A, B])(it: Iterable[(E, F)]): CC[E, F] = from.mapFactory.fromIterable(it)
}

/** Build the source collection type from a SortedMapOps */
implicit def buildFromSortedMapOps[CC[K, V] <: SortedMap[K, V] with SortedMapOps[K, V, CC, _], A, B, E : Ordering, F]: BuildFrom[CC[A, B], (E, F), CC[E, F]] = new BuildFrom[CC[A, B], (E, F), CC[E, F]] {
//TODO: Reuse a prototype instance
def fromSpecificIterable(from: CC[A, B])(it: Iterable[(E, F)]): CC[E, F] = from.sortedMapFactory.fromSpecificIterable(it)
}
}

trait BuildFromLowPriority {
/** Build the source collection type from an IterableOps */
implicit def buildFromIterableOps[CC[X] <: IterableOps[X, CC, _], A, E]: BuildFrom[CC[A], E, CC[E]] = new BuildFrom[CC[A], E, CC[E]] {
//TODO: Reuse a prototype instance
def fromSpecificIterable(from: CC[A])(it: Iterable[E]): CC[E] = from.iterableFactory.fromIterable(it)
}

/** Build the source collection type from an Iterable with SortedOps */
implicit def buildFromSortedOps[CC[X] <: Iterable[X] with SortedOps[X, CC[X], CC], A, E : Ordering]: BuildFrom[CC[A], E, CC[E]] = new BuildFrom[CC[A], E, CC[E]] {
def fromSpecificIterable(from: CC[A])(it: Iterable[E]): CC[E] = from.sortedIterableFactory.fromSpecificIterable(it)
}
}

/** A more specific `BuildFrom` for strict target collection types which can provide a `Builder`.
* Note that a `Builder` can be obtained for any `BuildFrom` via `Builder.from`.
*/
trait StrictBuildFrom[-From, -A, +C] extends Any with BuildFrom[From, A, C] {
def newBuilder(from: From): Builder[A, C]
}

object StrictBuildFrom extends StrictBuildFromLowPriority {
/** Build the source collection type from a strict MapOps */
implicit def strictBuildFromMapOps[CC[K, V] <: Map[K, V] with MapOps[K, V, CC, _], A, B, E, F]: StrictBuildFrom[CC[A, B], (E, F), CC[E, F]] = new StrictBuildFrom[CC[A, B], (E, F), CC[E, F]] {
//TODO: Reuse a prototype instance
def newBuilder(from: CC[A, B]): Builder[(E, F), CC[E, F]] = from.mapFactory.asInstanceOf[MapFactoryWithBuilder[CC]].newBuilder[E, F]()
def fromSpecificIterable(from: CC[A, B])(it: Iterable[(E, F)]): CC[E, F] = from.mapFactory.fromIterable(it)
}

/** Build the source collection type from a strict SortedMapOps */
implicit def strictBuildFromSortedMapOps[CC[K, V] <: SortedMap[K, V] with SortedMapOps[K, V, CC, _], A, B, E : Ordering, F]: StrictBuildFrom[CC[A, B], (E, F), CC[E, F]] = new StrictBuildFrom[CC[A, B], (E, F), CC[E, F]] {
//TODO: Reuse a prototype instance
def newBuilder(from: CC[A, B]): Builder[(E, F), CC[E, F]] = from.sortedMapFactory.asInstanceOf[SortedMapFactoryWithBuilder[CC]].newBuilder[E, F]()
def fromSpecificIterable(from: CC[A, B])(it: Iterable[(E, F)]): CC[E, F] = from.sortedMapFactory.fromSpecificIterable(it)
}
}

trait StrictBuildFromLowPriority {
/** Build the source collection type from a strict IterableOps */
implicit def strictBuildFromIterableOps[CC[X] <: IterableOps[X, CC, _] with Buildable[X, CC[X]], A, E]: StrictBuildFrom[CC[A], E, CC[E]] = new StrictBuildFrom[CC[A], E, CC[E]] {
//TODO: Reuse a prototype instance
def newBuilder(from: CC[A]): Builder[E, CC[E]] = from.iterableFactory.asInstanceOf[IterableFactoryWithBuilder[CC]].newBuilder[E]()
def fromSpecificIterable(from: CC[A])(it: Iterable[E]): CC[E] = from.iterableFactory.fromIterable(it)
}

/** Build the source collection type from a strict Iterable with SortedOps */
implicit def strictBuildFromSortedOps[CC[X] <: Iterable[X] with SortedOps[X, CC[X], CC] with Buildable[X, CC[X]], A, E : Ordering]: StrictBuildFrom[CC[A], E, CC[E]] = new StrictBuildFrom[CC[A], E, CC[E]] {
def newBuilder(from: CC[A]): Builder[E, CC[E]] = from.sortedIterableFactory.asInstanceOf[SortedIterableFactoryWithBuilder[CC]].newBuilder[E]()
def fromSpecificIterable(from: CC[A])(it: Iterable[E]): CC[E] = from.sortedIterableFactory.fromSpecificIterable(it)
}
}

/**
* Builds a collection of type `C` from elements of type `A`
* @tparam A Type of elements (e.g. `Int`, `Boolean`, etc.)
* @tparam C Type of collection (e.g. `List[Int]`, `TreeMap[Int, String]`, etc.)
*/
trait FromSpecificIterable[-A, +C] extends Any {
trait FromSpecificIterable[-A, +C] extends Any with BuildFrom[Any, A, C] {
def fromSpecificIterable(from: Any)(it: Iterable[A]): C = fromSpecificIterable(it)
def fromSpecificIterable(it: Iterable[A]): C
}

/** A more specific `FromSpecificIterable` for strict collection types which can provide a `Builder`. */
trait FromSpecificIterableWithBuilder[-A, +C] extends Any with FromSpecificIterable[A, C] with StrictBuildFrom[Any, A, C] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need this type? It does not seem to be consumed anywhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not per so but we do need FromSpecificIterable with StrictBuildFrom

def newBuilder(from: Any): Builder[A, C] = newBuilder
def newBuilder: Builder[A, C]
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer that we add parens to emphasize the fact that each created builder has its own identity: def newBuilder(): Builder[A, C].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have many methods without a parameter list that can return objects with different identities, for example Iterable.tail or IterableFactory.empty. The name newBuilder in particular makes it clear that you get a new object. I would prefer to omit empty parameter lists wherever possible.

}

/** Base trait for companion objects of unconstrained collection types */
trait IterableFactory[+CC[_]] {

Expand All @@ -28,13 +114,7 @@ trait IterableFactory[+CC[_]] {

}

trait IterableFactoryWithBuilder[+CC[_]] extends IterableFactory[CC] {
def newBuilder[A](): Builder[A, CC[A]]
}

object IterableFactory {
import scala.language.implicitConversions

implicit def toSpecific[A, CC[_]](factory: IterableFactory[CC]): FromSpecificIterable[A, CC[A]] =
new FromSpecificIterable[A, CC[A]] {
def fromSpecificIterable(it: Iterable[A]): CC[A] = factory.fromIterable[A](it)
Expand All @@ -44,7 +124,24 @@ object IterableFactory {
def empty[A]: CC[A] = delegate.empty
def fromIterable[E](it: Iterable[E]): CC[E] = delegate.fromIterable(it)
}
}

trait IterableFactoryWithBuilder[+CC[_]] extends IterableFactory[CC] {
def newBuilder[A](): Builder[A, CC[A]]
}

object IterableFactoryWithBuilder {
implicit def toSpecific[A, CC[_]](factory: IterableFactoryWithBuilder[CC]): FromSpecificIterableWithBuilder[A, CC[A]] =
new FromSpecificIterableWithBuilder[A, CC[A]] {
def fromSpecificIterable(it: Iterable[A]): CC[A] = factory.fromIterable[A](it)
def newBuilder: Builder[A, CC[A]] = factory.newBuilder[A]()
}

class Delegate[CC[_]](delegate: IterableFactoryWithBuilder[CC]) extends IterableFactoryWithBuilder[CC] {
def empty[A]: CC[A] = delegate.empty
def fromIterable[E](it: Iterable[E]): CC[E] = delegate.fromIterable(it)
def newBuilder[A](): Builder[A, CC[A]] = delegate.newBuilder[A]()
}
}

trait SpecificIterableFactory[-A, +C] extends FromSpecificIterable[A, C] {
Expand All @@ -55,6 +152,8 @@ trait SpecificIterableFactory[-A, +C] extends FromSpecificIterable[A, C] {
def fill(n: Int)(elem: => A): C = fromSpecificIterable(View.Fill(n)(elem))
}

trait SpecificIterableFactoryWithBuilder[-A, +C] extends SpecificIterableFactory[A, C] with FromSpecificIterableWithBuilder[A, C]

/** Factory methods for collections of kind `* −> * -> *` */
trait MapFactory[+CC[X, Y]] {

Expand All @@ -65,8 +164,6 @@ trait MapFactory[+CC[X, Y]] {
}

object MapFactory {
import scala.language.implicitConversions

implicit def toSpecific[K, V, CC[X, Y]](factory: MapFactory[CC]): FromSpecificIterable[(K, V), CC[K, V]] =
new FromSpecificIterable[(K, V), CC[K, V]] {
def fromSpecificIterable(it: Iterable[(K, V)]): CC[K, V] = factory.fromIterable[K, V](it)
Expand All @@ -76,7 +173,24 @@ object MapFactory {
def fromIterable[K, V](it: Iterable[(K, V)]): C[K, V] = delegate.fromIterable(it)
def empty[K, V]: C[K, V] = delegate.empty
}
}

trait MapFactoryWithBuilder[+CC[X, Y]] extends MapFactory[CC] {
def newBuilder[K, V](): Builder[(K, V), CC[K, V]]
}

object MapFactoryWithBuilder {
implicit def toSpecific[K, V, CC[X, Y]](factory: MapFactoryWithBuilder[CC]): FromSpecificIterableWithBuilder[(K, V), CC[K, V]] =
new FromSpecificIterableWithBuilder[(K, V), CC[K, V]] {
def fromSpecificIterable(it: Iterable[(K, V)]): CC[K, V] = factory.fromIterable[K, V](it)
def newBuilder: Builder[(K, V), CC[K, V]] = factory.newBuilder[K, V]()
}

class Delegate[C[X, Y]](delegate: MapFactoryWithBuilder[C]) extends MapFactoryWithBuilder[C] {
def fromIterable[K, V](it: Iterable[(K, V)]): C[K, V] = delegate.fromIterable(it)
def empty[K, V]: C[K, V] = delegate.empty
def newBuilder[K, V](): Builder[(K, V), C[K, V]] = delegate.newBuilder()
}
}

/** Base trait for companion objects of collections that require an implicit evidence */
Expand All @@ -92,8 +206,6 @@ trait SortedIterableFactory[+CC[_]] {
}

object SortedIterableFactory {
import scala.language.implicitConversions

implicit def toSpecific[A: Ordering, CC[_]](factory: SortedIterableFactory[CC]): FromSpecificIterable[A, CC[A]] =
new FromSpecificIterable[A, CC[A]] {
def fromSpecificIterable(it: Iterable[A]): CC[A] = factory.sortedFromIterable[A](it)
Expand All @@ -103,7 +215,24 @@ object SortedIterableFactory {
def empty[A : Ordering]: CC[A] = delegate.empty
def sortedFromIterable[E : Ordering](it: Iterable[E]): CC[E] = delegate.sortedFromIterable(it)
}
}

trait SortedIterableFactoryWithBuilder[+CC[_]] extends SortedIterableFactory[CC] {
def newBuilder[A : Ordering](): Builder[A, CC[A]]
}

object SortedIterableFactoryWithBuilder {
implicit def toSpecific[A: Ordering, CC[_]](factory: SortedIterableFactoryWithBuilder[CC]): FromSpecificIterableWithBuilder[A, CC[A]] =
new FromSpecificIterableWithBuilder[A, CC[A]] {
def fromSpecificIterable(it: Iterable[A]): CC[A] = factory.sortedFromIterable[A](it)
def newBuilder: Builder[A, CC[A]] = factory.newBuilder[A]()
}

class Delegate[CC[_]](delegate: SortedIterableFactoryWithBuilder[CC]) extends SortedIterableFactoryWithBuilder[CC] {
def empty[A : Ordering]: CC[A] = delegate.empty
def sortedFromIterable[E : Ordering](it: Iterable[E]): CC[E] = delegate.sortedFromIterable(it)
def newBuilder[A : Ordering](): Builder[A, CC[A]] = delegate.newBuilder[A]()
}
}

/** Factory methods for collections of kind `* −> * -> *` which require an implicit evidence value for the key type */
Expand All @@ -118,8 +247,6 @@ trait SortedMapFactory[+CC[X, Y]] {
}

object SortedMapFactory {
import scala.language.implicitConversions

implicit def toSpecific[K : Ordering, V, CC[_, _]](factory: SortedMapFactory[CC]): FromSpecificIterable[(K, V), CC[K, V]] =
new FromSpecificIterable[(K, V), CC[K, V]] {
def fromSpecificIterable(it: Iterable[(K, V)]): CC[K, V] = factory.sortedFromIterable(it)
Expand All @@ -129,5 +256,22 @@ object SortedMapFactory {
def empty[K: Ordering, V]: CC[K, V] = delegate.empty[K, V]
def sortedFromIterable[K: Ordering, V](it: Iterable[(K, V)]): CC[K, V] = delegate.sortedFromIterable(it)
}
}

trait SortedMapFactoryWithBuilder[+CC[X, Y]] extends SortedMapFactory[CC] {
def newBuilder[K : Ordering, V](): Builder[(K, V), CC[K, V]]
}

object SortedMapFactoryWithBuilder {
implicit def toSpecific[K : Ordering, V, CC[X, Y]](factory: SortedMapFactoryWithBuilder[CC]): FromSpecificIterableWithBuilder[(K, V), CC[K, V]] =
new FromSpecificIterableWithBuilder[(K, V), CC[K, V]] {
def fromSpecificIterable(it: Iterable[(K, V)]): CC[K, V] = factory.fromSpecificIterable(it)
def newBuilder: Builder[(K, V), CC[K, V]] = factory.newBuilder[K, V]()
}

class Delegate[C[X, Y]](delegate: SortedMapFactoryWithBuilder[C]) extends SortedMapFactoryWithBuilder[C] {
def sortedFromIterable[K : Ordering, V](it: Iterable[(K, V)]): C[K, V] = delegate.sortedFromIterable(it)
def empty[K : Ordering, V]: C[K, V] = delegate.empty
def newBuilder[K : Ordering, V](): Builder[(K, V), C[K, V]] = delegate.newBuilder()
}
}
4 changes: 4 additions & 0 deletions src/main/scala/strawman/collection/Iterable.scala
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,10 @@ trait IterableOps[+A, +CC[X], +C] extends Any {
}

/** Base trait for strict collections that can be built using a builder.
*
* All factories provided by such a collection must be of the `*WithBuilder` variant
* even though this is not formally enforced through types.
*
Copy link
Contributor

Choose a reason for hiding this comment

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

This is unfortunate that we can not enforce this constraint :(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can relax it from must to should though. Some strict collections may not be able to provide a more efficient builder anyway so it would be OK to fall back to a an ArrayBuffer. Implementing a WithBuilder factory would be an optimization, just like overriding a default implementation of some collection operation.

* @tparam A the element type of the collection
* @tparam C the type of the underlying collection
*/
Expand Down
2 changes: 2 additions & 0 deletions src/main/scala/strawman/collection/Map.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ trait MapOps[K, +V, +CC[X, Y] <: Map[X, Y], +C <: Map[K, V]]
/** Similar to fromIterable, but returns a Map collection type */
protected[this] def mapFromIterable[K2, V2](it: Iterable[(K2, V2)]): CC[K2, V2]

def mapFactory: MapFactory[CC]

/** Optionally returns the value associated with a key.
*
* @param key the key value
Expand Down
6 changes: 5 additions & 1 deletion src/main/scala/strawman/collection/SortedMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ trait SortedMap[K, +V]

trait SortedMapOps[K, +V, +CC[X, Y] <: SortedMap[X, Y] with SortedMapOps[X, Y, CC, _], +C <: SortedMap[K, V]]
extends MapOps[K, V, Map, C]
with SortedOps[K, C] {
with SortedOps[K, C, SortedSet] {

def sortedIterableFactory = SortedSet
Copy link
Contributor

Choose a reason for hiding this comment

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

Does that mean that when I navigate through the sortedIterableFactory of a SortedMap then I get back a SortedSet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. This is a side-effect of SortedOps being shared by sorted sets and maps. I was also baffled by this at first but it does make sense. A sorted map is sorted by key. If you assume a standard Ordering on tuples, it degrades nicely to a sorted set. The ordering for (K,V) with unique K values is the same and with K already being unique, so is (K,V).


def sortedMapFactory: SortedMapFactory[CC]

protected[this] def sortedMapFromIterable[K2, V2](it: collection.Iterable[(K2, V2)])(implicit ordering: Ordering[K2]): CC[K2, V2]

Expand Down
4 changes: 3 additions & 1 deletion src/main/scala/strawman/collection/SortedOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package strawman.collection
import scala.{Ordering, Option, Some}

/** Base trait for sorted collections */
trait SortedOps[A, +C] {
trait SortedOps[A, +C, +CC[_]] {
Copy link
Contributor

Choose a reason for hiding this comment

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

So far we usually order the type parameters such that CC is before C.


implicit def ordering: Ordering[A]

def sortedIterableFactory: SortedIterableFactory[CC]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR also adds the another factor kinds. So far every collection exposes an IterableFactory. Now in addition to that every sorted collection has a SortedIterableFactory. We'll probably want to add MapFactory and SortedMapFactory types to the appropriate collection types as well.


/** Returns the first key of the collection. */
def firstKey: A

Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/strawman/collection/SortedSet.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ trait SortedSet[A] extends Set[A] with SortedSetOps[A, SortedSet, SortedSet[A]]

trait SortedSetOps[A, +CC[X], +C <: SortedSet[A]]
extends SetOps[A, Set, C]
with SortedOps[A, C] {
with SortedOps[A, C, CC] {

protected[this] def sortedFromIterable[B: Ordering](it: Iterable[B]): CC[B]

Expand Down
1 change: 1 addition & 0 deletions src/main/scala/strawman/collection/immutable/BitSet.scala
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ sealed abstract class BitSet
def empty: BitSet = BitSet.empty

def iterableFactory = Set
def sortedIterableFactory = SortedSet
Copy link
Contributor

Choose a reason for hiding this comment

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

Why can’t we tell that the factory of a BitSet collection is the BitSet object?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the SortedIterableFactory can build a collection for every element type with an Ordering.


protected[this] def fromSpecificIterable(coll: collection.Iterable[Int]): BitSet = BitSet.fromSpecificIterable(coll)
protected[this] def sortedFromIterable[B : Ordering](it: collection.Iterable[B]): SortedSet[B] = SortedSet.sortedFromIterable(it)
Expand Down
1 change: 1 addition & 0 deletions src/main/scala/strawman/collection/immutable/HashMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ sealed trait HashMap[K, +V]
import HashMap.{bufferSize, liftMerger, Merger, MergeFunction, nullToEmpty}

def iterableFactory = List
def mapFactory = HashMap

protected[this] def fromSpecificIterable(coll: collection.Iterable[(K, V)]): HashMap[K, V] = HashMap.fromIterable(coll)

Expand Down
1 change: 1 addition & 0 deletions src/main/scala/strawman/collection/immutable/ListMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ sealed class ListMap[K, +V]
with Serializable {

def iterableFactory = List
def mapFactory = ListMap

protected[this] def mapFromIterable[K2, V2](it: collection.Iterable[(K2, V2)]): ListMap[K2,V2] = ListMap.fromIterable(it)

Expand Down
2 changes: 2 additions & 0 deletions src/main/scala/strawman/collection/immutable/TreeMap.scala
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ final class TreeMap[K, +V] private (tree: RB.Tree[K, V])(implicit val ordering:
def this()(implicit ordering: Ordering[K]) = this(null)(ordering)

def iterableFactory = List
def mapFactory = Map
def sortedMapFactory = TreeMap

protected[this] def fromSpecificIterable(coll: collection.Iterable[(K, V)]): TreeMap[K, V] =
TreeMap.sortedFromIterable(coll)
Expand Down
7 changes: 5 additions & 2 deletions src/main/scala/strawman/collection/immutable/TreeSet.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package strawman
package collection
package immutable

import mutable.Builder
import mutable.{ArrayBuffer, Builder}
import immutable.{RedBlackTree => RB}

import scala.{Boolean, Int, NullPointerException, Option, Ordering, Some, Unit}
Expand Down Expand Up @@ -63,6 +63,8 @@ final class TreeSet[A] private (tree: RB.Tree[A, Unit])(implicit val ordering: O

def iterableFactory = Set

def sortedIterableFactory = TreeSet

protected[this] def fromSpecificIterable(coll: strawman.collection.Iterable[A]): TreeSet[A] =
TreeSet.sortedFromIterable(coll)

Expand Down Expand Up @@ -103,7 +105,7 @@ final class TreeSet[A] private (tree: RB.Tree[A, Unit])(implicit val ordering: O
else newSet(RB.delete(tree, elem))
}

object TreeSet extends SortedIterableFactory[TreeSet] {
object TreeSet extends SortedIterableFactoryWithBuilder[TreeSet] {

def empty[A: Ordering]: TreeSet[A] = new TreeSet[A]

Expand All @@ -113,4 +115,5 @@ object TreeSet extends SortedIterableFactory[TreeSet] {
case _ => empty[E] ++ it
}

def newBuilder[A : Ordering](): Builder[A, TreeSet[A]] = new ArrayBuffer[A].mapResult(sortedFromIterable[A] _)
Copy link
Contributor

Choose a reason for hiding this comment

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

What’s the advantage of using an intermediate ArrayBuffer over directly building a TreeSet?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, right, the current collections append directly to the immutable TreeSet. We should do the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll address those issues in the Set PR. Doing it the old way requires a SetBuilder, which in turn requires a + operation on Sets that we do not have yet...

}
2 changes: 2 additions & 0 deletions src/main/scala/strawman/collection/mutable/BitSet.scala
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ class BitSet(protected[collection] final var elems: Array[Long])

def iterableFactory = Set

def sortedIterableFactory = SortedSet

protected[this] def sortedFromIterable[B : Ordering](it: collection.Iterable[B]): collection.mutable.SortedSet[B] =
collection.mutable.SortedSet.sortedFromIterable(it)

Expand Down
Loading