From b0a46e97fa0cd0385c889122b1ab153df06725d4 Mon Sep 17 00:00:00 2001 From: Julien Richard-Foy Date: Wed, 3 May 2017 14:45:03 +0200 Subject: [PATCH 1/3] Tune hierarchy of mutable collections - mutable.Map is a mutable.Iterable. - mutable.Set is a mutable.Iterable. --- src/main/scala/strawman/collection/mutable/Iterable.scala | 5 ++++- src/main/scala/strawman/collection/mutable/Map.scala | 7 ++++--- src/main/scala/strawman/collection/mutable/Set.scala | 3 ++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/main/scala/strawman/collection/mutable/Iterable.scala b/src/main/scala/strawman/collection/mutable/Iterable.scala index 096f0a79c6d9..b14dfadf4dc0 100644 --- a/src/main/scala/strawman/collection/mutable/Iterable.scala +++ b/src/main/scala/strawman/collection/mutable/Iterable.scala @@ -4,7 +4,10 @@ import strawman.collection import strawman.collection.IterableFactory trait Iterable[A] extends collection.Iterable[A] - with collection.IterableOps[A, Iterable, Iterable[A]] + with IterableOps[A, Iterable, Iterable[A]] + +trait IterableOps[A, +CC[X], +C] + extends collection.IterableOps[A, CC, C] object Iterable extends IterableFactory.Delegate[Iterable](ArrayBuffer) diff --git a/src/main/scala/strawman/collection/mutable/Map.scala b/src/main/scala/strawman/collection/mutable/Map.scala index 3e7e62126298..b46587b98256 100644 --- a/src/main/scala/strawman/collection/mutable/Map.scala +++ b/src/main/scala/strawman/collection/mutable/Map.scala @@ -5,9 +5,10 @@ package mutable import scala.{Option, `inline`, Unit} /** Base type of mutable Maps */ -trait Map[K, V] extends Iterable[(K, V)] - with collection.Map[K, V] - with MapOps[K, V, Map, Map[K, V]] +trait Map[K, V] + extends Iterable[(K, V)] + with collection.Map[K, V] + with MapOps[K, V, Map, Map[K, V]] /** Base trait of mutable Maps implementations */ trait MapOps[K, V, +CC[X, Y] <: Map[X, Y], +C <: Map[K, V]] diff --git a/src/main/scala/strawman/collection/mutable/Set.scala b/src/main/scala/strawman/collection/mutable/Set.scala index ef01ec683443..6e642c9063da 100644 --- a/src/main/scala/strawman/collection/mutable/Set.scala +++ b/src/main/scala/strawman/collection/mutable/Set.scala @@ -11,7 +11,8 @@ trait Set[A] extends Iterable[A] with SetOps[A, Set, Set[A]] trait SetOps[A, +CC[X], +C <: Set[A]] - extends collection.SetOps[A, CC, C] + extends IterableOps[A, CC, C] + with collection.SetOps[A, CC, C] with Growable[A] { /** Removes a single element from this $coll. From 5305957f0fa1444b79094c9574630a5f2cfe7ccb Mon Sep 17 00:00:00 2001 From: Julien Richard-Foy Date: Wed, 3 May 2017 15:29:49 +0200 Subject: [PATCH 2/3] Factor out the subtract method in a Shrinkable trait. Move method implementations in XxxOps traits. --- .../collection/mutable/Growable.scala | 9 +-- .../strawman/collection/mutable/HashMap.scala | 2 +- .../strawman/collection/mutable/Map.scala | 18 ++--- .../strawman/collection/mutable/Set.scala | 40 ++++++----- .../collection/mutable/Shrinkable.scala | 68 +++++++++++++++++++ .../strawman/collection/mutable/TreeMap.scala | 2 +- 6 files changed, 100 insertions(+), 39 deletions(-) create mode 100644 src/main/scala/strawman/collection/mutable/Shrinkable.scala diff --git a/src/main/scala/strawman/collection/mutable/Growable.scala b/src/main/scala/strawman/collection/mutable/Growable.scala index 278954ee7367..daea934469cc 100644 --- a/src/main/scala/strawman/collection/mutable/Growable.scala +++ b/src/main/scala/strawman/collection/mutable/Growable.scala @@ -1,4 +1,5 @@ -package strawman.collection.mutable +package strawman +package collection.mutable import strawman.collection.IterableOnce import scala.{`inline`, Unit} @@ -28,7 +29,7 @@ trait Growable[-A] { * @param elems the remaining elements to $add. * @return the $coll itself */ - @`inline` final def +=(elem1: A, elem2: A, elems: A*): this.type = this += elem1 += elem2 ++= (elems.toStrawman: IterableOnce[A]) + @`inline` final def += (elem1: A, elem2: A, elems: A*): this.type = this += elem1 += elem2 ++= (elems.toStrawman: IterableOnce[A]) /** ${Add}s all elements produced by a TraversableOnce to this $coll. * @@ -36,14 +37,14 @@ trait Growable[-A] { * @return the $coll itself. */ def addAll(xs: IterableOnce[A]): this.type = { - @tailrec def loop(xs: scala.collection.LinearSeq[A]): Unit = { + @tailrec def loop(xs: collection.LinearSeq[A]): Unit = { if (xs.nonEmpty) { this += xs.head loop(xs.tail) } } xs match { - case xs: scala.collection.LinearSeq[_] => loop(xs.asInstanceOf[scala.collection.LinearSeq[A]]) + case xs: collection.LinearSeq[A] => loop(xs) case xs => xs.iterator() foreach += // Deviation: IterableOnce does not define `foreach`. } // @ichoran writes: Right now, this actually isn't any faster than using an iterator diff --git a/src/main/scala/strawman/collection/mutable/HashMap.scala b/src/main/scala/strawman/collection/mutable/HashMap.scala index f94716a1a035..7acf177e47c3 100644 --- a/src/main/scala/strawman/collection/mutable/HashMap.scala +++ b/src/main/scala/strawman/collection/mutable/HashMap.scala @@ -57,7 +57,7 @@ final class HashMap[K, V] private[collection] (contents: HashTable.Contents[K, D def clear(): Unit = table.clearTable() - def remove(key: K): this.type = { table.removeEntry(key); this } + def subtract(key: K): this.type = { table.removeEntry(key); this } override def size: Int = table.size diff --git a/src/main/scala/strawman/collection/mutable/Map.scala b/src/main/scala/strawman/collection/mutable/Map.scala index b46587b98256..35adac80a768 100644 --- a/src/main/scala/strawman/collection/mutable/Map.scala +++ b/src/main/scala/strawman/collection/mutable/Map.scala @@ -9,23 +9,17 @@ trait Map[K, V] extends Iterable[(K, V)] with collection.Map[K, V] with MapOps[K, V, Map, Map[K, V]] + with Growable[(K, V)] + with Shrinkable[K] /** Base trait of mutable Maps implementations */ trait MapOps[K, V, +CC[X, Y] <: Map[X, Y], +C <: Map[K, V]] extends IterableOps[(K, V), Iterable, C] - with collection.MapOps[K, V, CC, C] - with Growable[(K, V)] { + with collection.MapOps[K, V, CC, C] { - def fromIterable[B](coll: collection.Iterable[B]): Iterable[B] = Iterable.fromIterable(coll) + protected def coll: Map[K, V] - /** Removes a single element from this $coll. - * - * @param key the key of the element to remove. - * @return the $coll itself - */ - def remove(key: K): this.type - /** Alias for `remove` */ - @`inline` final def -= (key: K): this.type = remove(key) + def fromIterable[B](coll: collection.Iterable[B]): Iterable[B] = Iterable.fromIterable(coll) /** Adds a new key/value pair to this map and optionally returns previously bound value. * If the map already contains a @@ -50,7 +44,7 @@ trait MapOps[K, V, +CC[X, Y] <: Map[X, Y], +C <: Map[K, V]] * @param key The key to update * @param value The new value */ - def update(key: K, value: V): Unit = { this += ((key, value)) } + def update(key: K, value: V): Unit = { coll += ((key, value)) } override def clone(): C = empty ++= coll diff --git a/src/main/scala/strawman/collection/mutable/Set.scala b/src/main/scala/strawman/collection/mutable/Set.scala index 6e642c9063da..8588d4426cc2 100644 --- a/src/main/scala/strawman/collection/mutable/Set.scala +++ b/src/main/scala/strawman/collection/mutable/Set.scala @@ -6,33 +6,31 @@ import strawman.collection.{IterableFactory, IterableOnce} import scala.{Boolean, Int, None, Option, Some, Unit, `inline`} /** Base trait for mutable sets */ -trait Set[A] extends Iterable[A] - with collection.Set[A] - with SetOps[A, Set, Set[A]] +trait Set[A] + extends Iterable[A] + with collection.Set[A] + with SetOps[A, Set, Set[A]] + with Growable[A] + with Shrinkable[A] trait SetOps[A, +CC[X], +C <: Set[A]] extends IterableOps[A, CC, C] - with collection.SetOps[A, CC, C] - with Growable[A] { + with collection.SetOps[A, CC, C] { - /** Removes a single element from this $coll. - * - * @param elem the element to remove. - * @return the $coll itself + /** + * @return The reference of the contained element that is equal to `elem`, if found, otherwise `None` + * @param elem The element to get. */ - def subtract(elem: A): this.type - /** Alias for `subtract` */ - @`inline` final def -= (elem: A): this.type = subtract(elem) - - def contains(elem: A): Boolean def get(elem: A): Option[A] def insert(elem: A): Boolean = - !contains(elem) && { +=(elem); true } + !contains(elem) && { + coll += elem; true + } def remove(elem: A): Option[A] = { val res = get(elem) - -=(elem) + coll -= elem res } @@ -46,8 +44,8 @@ trait SetOps[A, +CC[X], +C <: Set[A]] toRemove -= elem } } - for (elem <- toRemove) +=(elem) - for (elem <- toAdd) -=(elem) + for (elem <- toRemove) coll -= elem + for (elem <- toAdd) coll += elem } def flatMapInPlace(f: A => IterableOnce[A]): Unit = { @@ -59,8 +57,8 @@ trait SetOps[A, +CC[X], +C <: Set[A]] toAdd += mapped toRemove -= elem } - for (elem <- toRemove) -=(elem) - for (elem <- toAdd) +=(elem) + for (elem <- toRemove) coll -= elem + for (elem <- toAdd) coll += elem } def filterInPlace(p: A => Boolean): Unit = { @@ -68,7 +66,7 @@ trait SetOps[A, +CC[X], +C <: Set[A]] for (elem <- this) if (!p(elem)) toRemove += elem for (elem <- toRemove) - -=(elem) + coll -= elem } override def clone(): C = empty ++= coll diff --git a/src/main/scala/strawman/collection/mutable/Shrinkable.scala b/src/main/scala/strawman/collection/mutable/Shrinkable.scala new file mode 100644 index 000000000000..f72a960332f7 --- /dev/null +++ b/src/main/scala/strawman/collection/mutable/Shrinkable.scala @@ -0,0 +1,68 @@ +package strawman +package collection.mutable + +import scala.annotation.tailrec + +import collection.toNewSeq +import scala.{`inline`, Unit} + +/** This trait forms part of collections that can be reduced + * using a `-=` operator. + * + * @author Martin Odersky + * @version 2.8 + * @since 2.8 + * @define coll shrinkable collection + * @define Coll `Shrinkable` + */ +trait Shrinkable[-A] { + + /** Removes a single element from this $coll. + * + * @param elem the element to remove. + * @return the $coll itself + */ + def subtract(elem: A): this.type + + /** Alias for `subtract` */ + @`inline` final def -= (elem: A): this.type = subtract(elem) + + /** Removes two or more elements from this $coll. + * + * @param elem1 the first element to remove. + * @param elem2 the second element to remove. + * @param elems the remaining elements to remove. + * @return the $coll itself + */ + def subtract(elem1: A, elem2: A, elems: A*): this.type = { + this -= elem1 + this -= elem2 + this --= elems.toStrawman + } + + /** Alias for `subtract` */ + @`inline` final def -= (elem1: A, elem2: A, elems: A*): this.type = subtract(elem1, elem2, elems: _*) + + /** Removes all elements produced by an iterator from this $coll. + * + * @param xs the iterator producing the elements to remove. + * @return the $coll itself + */ + def subtractAll(xs: collection.IterableOnce[A]): this.type = { + @tailrec def loop(xs: collection.LinearSeq[A]): Unit = { + if (xs.nonEmpty) { + subtract(xs.head) + loop(xs.tail) + } + } + xs match { + case xs: collection.LinearSeq[A] => loop(xs) + case xs => xs.iterator().foreach(subtract) + } + this + } + + /** Alias for `subtractAll` */ + @`inline` final def --= (xs: collection.IterableOnce[A]): this.type = subtractAll(xs) + +} \ No newline at end of file diff --git a/src/main/scala/strawman/collection/mutable/TreeMap.scala b/src/main/scala/strawman/collection/mutable/TreeMap.scala index 32bbbb4d7f7c..9691848355fb 100644 --- a/src/main/scala/strawman/collection/mutable/TreeMap.scala +++ b/src/main/scala/strawman/collection/mutable/TreeMap.scala @@ -46,7 +46,7 @@ sealed class TreeMap[K, V] private (tree: RB.Tree[K, V])(implicit val ordering: def add(elem: (K, V)): this.type = { RB.insert(tree, elem._1, elem._2); this } - def remove(elem: K): this.type = { RB.delete(tree, elem); this } + def subtract(elem: K): this.type = { RB.delete(tree, elem); this } def clear(): Unit = RB.clear(tree) From 18ac93496da99396849b8249f0ea46b970c6ed49 Mon Sep 17 00:00:00 2001 From: Julien Richard-Foy Date: Thu, 4 May 2017 11:56:20 +0200 Subject: [PATCH 3/3] Pull up mapInPlace to mutable.Iterable, so that it is consistently inherited by all mutable collections. Pull up flatMapInPlace and filterInPlace to mutable.GrowableIterable, which is a new trait extending both Iterable and Growable. Make mutable.Map extend GrowableIterable (and implement its newly inherited methods). Make GrowableSeq extend GrowableIterable (and remove inherited method definitions). Make mutable.Set extend GrowableIterable (and adapt its previous mapInPlace methods to match exactly their new signature). --- .../collection/mutable/Iterable.scala | 23 ++++++++-- .../strawman/collection/mutable/Map.scala | 45 +++++++++++++++++-- .../strawman/collection/mutable/Seq.scala | 17 ++++--- .../strawman/collection/mutable/Set.scala | 38 ++++++++-------- 4 files changed, 92 insertions(+), 31 deletions(-) diff --git a/src/main/scala/strawman/collection/mutable/Iterable.scala b/src/main/scala/strawman/collection/mutable/Iterable.scala index b14dfadf4dc0..f55ce6734b29 100644 --- a/src/main/scala/strawman/collection/mutable/Iterable.scala +++ b/src/main/scala/strawman/collection/mutable/Iterable.scala @@ -1,13 +1,30 @@ package strawman.collection.mutable import strawman.collection -import strawman.collection.IterableFactory +import strawman.collection.{IterableFactory, IterableOnce} -trait Iterable[A] extends collection.Iterable[A] - with IterableOps[A, Iterable, Iterable[A]] +import scala.Boolean + +trait Iterable[A] + extends collection.Iterable[A] + with IterableOps[A, Iterable, Iterable[A]] { + + def mapInPlace(f: A => A): this.type + +} trait IterableOps[A, +CC[X], +C] extends collection.IterableOps[A, CC, C] object Iterable extends IterableFactory.Delegate[Iterable](ArrayBuffer) + +trait GrowableIterable[A] + extends Iterable[A] + with Growable[A] { + + def flatMapInPlace(f: A => IterableOnce[A]): this.type + + def filterInPlace(p: A => Boolean): this.type + +} diff --git a/src/main/scala/strawman/collection/mutable/Map.scala b/src/main/scala/strawman/collection/mutable/Map.scala index 35adac80a768..9ef43c684515 100644 --- a/src/main/scala/strawman/collection/mutable/Map.scala +++ b/src/main/scala/strawman/collection/mutable/Map.scala @@ -2,14 +2,15 @@ package strawman package collection package mutable -import scala.{Option, `inline`, Unit} +import strawman.collection.{IterableOnce, MapFactory} + +import scala.{Boolean, Option, Unit, `inline`} /** Base type of mutable Maps */ trait Map[K, V] - extends Iterable[(K, V)] + extends GrowableIterable[(K, V)] with collection.Map[K, V] with MapOps[K, V, Map, Map[K, V]] - with Growable[(K, V)] with Shrinkable[K] /** Base trait of mutable Maps implementations */ @@ -48,6 +49,44 @@ trait MapOps[K, V, +CC[X, Y] <: Map[X, Y], +C <: Map[K, V]] override def clone(): C = empty ++= coll + def mapInPlace(f: ((K, V)) => (K, V)): this.type = { + val toAdd = Map[K, V]() + val toRemove = Set[K]() + for (elem <- this) { + val mapped = f(elem) + if (!contains(mapped._1)) { + toAdd += mapped + toRemove -= elem._1 + } + } + for (elem <- toRemove) coll -= elem + for (elem <- toAdd) coll += elem + this + } + + def flatMapInPlace(f: ((K, V)) => IterableOnce[(K, V)]): this.type = { + val toAdd = Map[K, V]() + val toRemove = Set[K]() + for (elem <- this) + for (mapped <- f(elem).iterator()) + if (!contains(mapped._1)) { + toAdd += mapped + toRemove -= elem._1 + } + for (elem <- toRemove) coll -= elem + for (elem <- toAdd) coll += elem + this + } + + def filterInPlace(p: ((K, V)) => Boolean): this.type = { + val toRemove = Set[K]() + for (elem <- this) + if (!p(elem)) toRemove += elem._1 + for (elem <- toRemove) + coll -= elem + this + } + } object Map extends MapFactory.Delegate[Map](HashMap) \ No newline at end of file diff --git a/src/main/scala/strawman/collection/mutable/Seq.scala b/src/main/scala/strawman/collection/mutable/Seq.scala index 01c5bbc5616c..5c0d7fd4791f 100644 --- a/src/main/scala/strawman/collection/mutable/Seq.scala +++ b/src/main/scala/strawman/collection/mutable/Seq.scala @@ -5,22 +5,25 @@ import strawman.collection import strawman.collection.{IterableOnce, toNewSeq, toOldSeq} import scala.Predef.intWrapper -trait Seq[A] extends Iterable[A] - with collection.Seq[A] - with SeqOps[A, Seq, Seq[A]] +trait Seq[A] + extends Iterable[A] + with collection.Seq[A] + with SeqOps[A, Seq, Seq[A]] trait SeqOps[A, +CC[A] <: Seq[A], +C] extends collection.SeqOps[A, CC, C] { + def update(idx: Int, elem: A): Unit - def mapInPlace(f: A => A): this.type + } -trait GrowableSeq[A] extends Seq[A] with Growable[A] { +trait GrowableSeq[A] + extends GrowableIterable[A] + with Seq[A] { + def insert(idx: Int, elem: A): Unit def insertAll(idx: Int, elems: IterableOnce[A]): Unit def remove(idx: Int): A def remove(from: Int, n: Int): Unit - def flatMapInPlace(f: A => IterableOnce[A]): this.type - def filterInPlace(p: A => Boolean): this.type def patchInPlace(from: Int, patch: collection.Seq[A], replaced: Int): this.type // +=, ++=, clear inherited from Growable diff --git a/src/main/scala/strawman/collection/mutable/Set.scala b/src/main/scala/strawman/collection/mutable/Set.scala index 8588d4426cc2..12d0befbfbe3 100644 --- a/src/main/scala/strawman/collection/mutable/Set.scala +++ b/src/main/scala/strawman/collection/mutable/Set.scala @@ -7,16 +7,30 @@ import scala.{Boolean, Int, None, Option, Some, Unit, `inline`} /** Base trait for mutable sets */ trait Set[A] - extends Iterable[A] + extends GrowableIterable[A] with collection.Set[A] with SetOps[A, Set, Set[A]] - with Growable[A] with Shrinkable[A] trait SetOps[A, +CC[X], +C <: Set[A]] extends IterableOps[A, CC, C] with collection.SetOps[A, CC, C] { + def mapInPlace(f: A => A): this.type = { + val toAdd = Set[A]() + val toRemove = Set[A]() + for (elem <- this) { + val mapped = f(elem) + if (!contains(mapped)) { + toAdd += mapped + toRemove -= elem + } + } + for (elem <- toRemove) coll -= elem + for (elem <- toAdd) coll += elem + this + } + /** * @return The reference of the contained element that is equal to `elem`, if found, otherwise `None` * @param elem The element to get. @@ -34,21 +48,7 @@ trait SetOps[A, +CC[X], +C <: Set[A]] res } - def mapInPlace(f: A => A): Unit = { - val toAdd = Set[A]() - val toRemove = Set[A]() - for (elem <- this) { - val mapped = f(elem) - if (!contains(mapped)) { - toAdd += mapped - toRemove -= elem - } - } - for (elem <- toRemove) coll -= elem - for (elem <- toAdd) coll += elem - } - - def flatMapInPlace(f: A => IterableOnce[A]): Unit = { + def flatMapInPlace(f: A => IterableOnce[A]): this.type = { val toAdd = Set[A]() val toRemove = Set[A]() for (elem <- this) @@ -59,14 +59,16 @@ trait SetOps[A, +CC[X], +C <: Set[A]] } for (elem <- toRemove) coll -= elem for (elem <- toAdd) coll += elem + this } - def filterInPlace(p: A => Boolean): Unit = { + def filterInPlace(p: A => Boolean): this.type = { val toRemove = Set[A]() for (elem <- this) if (!p(elem)) toRemove += elem for (elem <- toRemove) coll -= elem + this } override def clone(): C = empty ++= coll