diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2013-11-25 09:23:06 +0100 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2013-11-26 15:01:11 +0100 |
commit | f12bb7bda4cd1e145ab5731ae6728f9e8dd2e54c (patch) | |
tree | ee95e631c21aebd0a9eafc6ceb9695808fea1cc3 | |
parent | 744425aab4f67805996388ea8620e81d1e4224b0 (diff) | |
download | scala-f12bb7bda4cd1e145ab5731ae6728f9e8dd2e54c.tar.gz scala-f12bb7bda4cd1e145ab5731ae6728f9e8dd2e54c.tar.bz2 scala-f12bb7bda4cd1e145ab5731ae6728f9e8dd2e54c.zip |
SI-4332 Plugs the gaps in views
These currently result in runtime errors:
scala> List(1).view.distinct.force
java.lang.UnsupportedOperationException: SeqView(...).newBuilder
scala> List(1).view.inits.toList
java.lang.UnsupportedOperationException: SeqView(...).newBuilder
Two tests are enclosed:
1. reflect on the views to make any method that returns
`Repr` is overriden in `*ViewLike`. This guards against
new additions to the collections API.
2. exercise the newly added overrides
Some care is needed with `tail`, we must re-override it
in `mutable.IndexedSeqView` to be explicit about the
end point of the slice, because the `IndexedSeqView#Sliced`
defines length in terms of that. (Higher up the chain, it
is just `iterator.size`, that's why `SeqView#tail` just sets
up a slice from `1 to Int.MaxValue`.)
Parallel collections views are not touched, as these are likely
to be deprecated or removed shortly.
Before this change, the test reported the following.
Not all of these methods were buggy. For instance, `sortBy`,
`sortWith` are implemented in terms of `sorted` which
was implemented in `SeqViewLike`. Even in those cases, I have
opted to override the methods in the `ViewLike` to guard
against changes in the base class implementation and to
simplify the rules in the test.
======================================================================
Checking scala.collection.TraversableView
======================================================================
trait TraversableLike
----------------------------------------------------------------------
def filterNot(p: A => Boolean): Repr
def inits: Iterator[Repr]
def tails: Iterator[Repr]
override def tail: Repr
======================================================================
Checking scala.collection.IterableView
======================================================================
trait IterableLike
----------------------------------------------------------------------
def dropRight(n: Int): Repr
def sliding(size: Int): Iterator[Repr]
def takeRight(n: Int): Repr
trait TraversableLike
----------------------------------------------------------------------
def filterNot(p: A => Boolean): Repr
def inits: Iterator[Repr]
def tails: Iterator[Repr]
override def tail: Repr
======================================================================
Checking scala.collection.SeqView
======================================================================
trait IterableLike
----------------------------------------------------------------------
def dropRight(n: Int): Repr
def sliding(size: Int): Iterator[Repr]
def takeRight(n: Int): Repr
trait SeqLike
----------------------------------------------------------------------
def combinations(n: Int): Iterator[Repr]
def distinct: Repr
def permutations: Iterator[Repr]
def sortBy[B](f: A => B)(implicit ord: scala.math.Ordering[B]): Repr
def sortWith(lt: (A, A) => Boolean): Repr
trait TraversableLike
----------------------------------------------------------------------
def filterNot(p: A => Boolean): Repr
def inits: Iterator[Repr]
def tails: Iterator[Repr]
override def tail: Repr
======================================================================
Checking scala.collection.mutable.IndexedSeqView
======================================================================
trait IndexedSeqOptimized
----------------------------------------------------------------------
override def dropRight(n: Int): Repr
override def tail: Repr
override def takeRight(n: Int): Repr
trait IterableLike
----------------------------------------------------------------------
def sliding(size: Int): Iterator[Repr]
trait SeqLike
----------------------------------------------------------------------
def combinations(n: Int): Iterator[Repr]
def distinct: Repr
def permutations: Iterator[Repr]
def sortBy[B](f: A => B)(implicit ord: scala.math.Ordering[B]): Repr
def sortWith(lt: (A, A) => Boolean): Repr
trait TraversableLike
----------------------------------------------------------------------
def filterNot(p: A => Boolean): Repr
def inits: Iterator[Repr]
def tails: Iterator[Repr]
======================================================================
Checking scala.collection.immutable.StreamView
======================================================================
trait IterableLike
----------------------------------------------------------------------
def dropRight(n: Int): Repr
def sliding(size: Int): Iterator[Repr]
def takeRight(n: Int): Repr
trait SeqLike
----------------------------------------------------------------------
def combinations(n: Int): Iterator[Repr]
def distinct: Repr
def permutations: Iterator[Repr]
def sortBy[B](f: A => B)(implicit ord: scala.math.Ordering[B]): Repr
def sortWith(lt: (A, A) => Boolean): Repr
trait TraversableLike
----------------------------------------------------------------------
def filterNot(p: A => Boolean): Repr
def inits: Iterator[Repr]
def tails: Iterator[Repr]
override def tail: Repr
-rw-r--r-- | src/library/scala/collection/IterableViewLike.scala | 9 | ||||
-rw-r--r-- | src/library/scala/collection/SeqViewLike.scala | 15 | ||||
-rw-r--r-- | src/library/scala/collection/TraversableViewLike.scala | 13 | ||||
-rw-r--r-- | src/library/scala/collection/mutable/IndexedSeqView.scala | 1 | ||||
-rw-r--r-- | test/files/run/t4332.check | 25 | ||||
-rw-r--r-- | test/files/run/t4332.scala | 44 | ||||
-rw-r--r-- | test/files/run/t4332b.scala | 35 |
7 files changed, 142 insertions, 0 deletions
diff --git a/src/library/scala/collection/IterableViewLike.scala b/src/library/scala/collection/IterableViewLike.scala index 236bfd154c..f24b8a075d 100644 --- a/src/library/scala/collection/IterableViewLike.scala +++ b/src/library/scala/collection/IterableViewLike.scala @@ -117,5 +117,14 @@ trait IterableViewLike[+A, override def sliding(size: Int, step: Int): Iterator[This] = self.iterator.sliding(size, step) map (x => newForced(x).asInstanceOf[This]) + override def sliding(size: Int): Iterator[This] = + sliding(size, 1) // we could inherit this, but that implies knowledge of the way the super class is implemented. + + override def dropRight(n: Int): This = + take(thisSeq.length - n) + + override def takeRight(n: Int): This = + drop(thisSeq.length - n) + override def stringPrefix = "IterableView" } diff --git a/src/library/scala/collection/SeqViewLike.scala b/src/library/scala/collection/SeqViewLike.scala index 1194cd7199..a949b56851 100644 --- a/src/library/scala/collection/SeqViewLike.scala +++ b/src/library/scala/collection/SeqViewLike.scala @@ -137,5 +137,20 @@ trait SeqViewLike[+A, override def sorted[B >: A](implicit ord: Ordering[B]): This = newForced(thisSeq sorted ord).asInstanceOf[This] + override def sortWith(lt: (A, A) => Boolean): This = + newForced(thisSeq sortWith lt).asInstanceOf[This] + + override def sortBy[B](f: (A) => B)(implicit ord: Ordering[B]): This = + newForced(thisSeq sortBy f).asInstanceOf[This] + + override def combinations(n: Int): Iterator[This] = + (thisSeq combinations n).map(as => newForced(as).asInstanceOf[This]) + + override def permutations: Iterator[This] = + thisSeq.permutations.map(as => newForced(as).asInstanceOf[This]) + + override def distinct: This = + newForced(thisSeq.distinct).asInstanceOf[This] + override def stringPrefix = "SeqView" } diff --git a/src/library/scala/collection/TraversableViewLike.scala b/src/library/scala/collection/TraversableViewLike.scala index ca1c450e3f..d56ecb16ed 100644 --- a/src/library/scala/collection/TraversableViewLike.scala +++ b/src/library/scala/collection/TraversableViewLike.scala @@ -209,5 +209,18 @@ trait TraversableViewLike[+A, override def unzip3[A1, A2, A3](implicit asTriple: A => (A1, A2, A3)) = (newMapped(x => asTriple(x)._1), newMapped(x => asTriple(x)._2), newMapped(x => asTriple(x)._3)) // TODO - Performance improvements. + override def filterNot(p: (A) => Boolean): This = + newFiltered(a => !(p(a))) + + override def inits: Iterator[This] = + thisSeq.inits.map(as => newForced(as).asInstanceOf[This]) + + override def tails: Iterator[This] = + thisSeq.tails.map(as => newForced(as).asInstanceOf[This]) + + override def tail: This = + // super.tail would also work as it is currently implemented in terms of drop(Int). + if (isEmpty) super.tail else newDropped(1) + override def toString = viewToString } diff --git a/src/library/scala/collection/mutable/IndexedSeqView.scala b/src/library/scala/collection/mutable/IndexedSeqView.scala index 36c1076b87..31a4749960 100644 --- a/src/library/scala/collection/mutable/IndexedSeqView.scala +++ b/src/library/scala/collection/mutable/IndexedSeqView.scala @@ -93,6 +93,7 @@ self => override def span(p: A => Boolean): (This, This) = (newTakenWhile(p), newDroppedWhile(p)) override def splitAt(n: Int): (This, This) = (take(n), drop(n)) // !!! override def reverse: This = newReversed + override def tail: IndexedSeqView[A, Coll] = if (isEmpty) super.tail else slice(1, length) } /** An object containing the necessary implicit definitions to make diff --git a/test/files/run/t4332.check b/test/files/run/t4332.check new file mode 100644 index 0000000000..ff9d9b8647 --- /dev/null +++ b/test/files/run/t4332.check @@ -0,0 +1,25 @@ + +====================================================================== +Checking scala.collection.TraversableView +====================================================================== + + +====================================================================== +Checking scala.collection.IterableView +====================================================================== + + +====================================================================== +Checking scala.collection.SeqView +====================================================================== + + +====================================================================== +Checking scala.collection.mutable.IndexedSeqView +====================================================================== + + +====================================================================== +Checking scala.collection.immutable.StreamView +====================================================================== + diff --git a/test/files/run/t4332.scala b/test/files/run/t4332.scala new file mode 100644 index 0000000000..5a67922911 --- /dev/null +++ b/test/files/run/t4332.scala @@ -0,0 +1,44 @@ +import scala.tools.partest._ + +object Test extends DirectTest { + override def code = "" + lazy val global = newCompiler("-usejavacp") + import global._, definitions._ + + override def show() { + new global.Run() + // Once we plug all of the view gaps, the output should be empty! + checkViews() + } + + def isExempt(sym: Symbol) = { + val exempt = Set("view", "repr", "sliceWithKnownDelta", "sliceWithKnownBound", "transform") + (exempt contains sym.name.decoded) + } + + def checkView(viewType: Type, viewLikeType: Type) { + val sep = "=" * 70 + println(s"\n$sep\nChecking ${viewType.typeSymbol.fullName}\n$sep") + val termMembers = viewType.nonPrivateMembers.toList filter (_.isTerm) map fullyInitializeSymbol + val inheritedFromGenericCollection + = termMembers filterNot (_.owner.name.decoded contains "ViewLike") filterNot (_.owner == viewType.typeSymbol) + def returnsView(sym: Symbol) = viewType.memberType(sym).finalResultType contains viewType.typeSymbol + val needOverride = inheritedFromGenericCollection filterNot isExempt filter returnsView + + val grouped = needOverride.groupBy(_.owner).toSeq.sortBy { case (owner, _) => viewType baseTypeIndex owner } + val report = grouped.map { + case (owner, syms) => s"\n$owner\n${"-" * 70}\n${syms.map(_.defString).sorted.mkString("\n")}" + }.mkString("\n") + println(report) + } + + def checkViews() { + import collection._ + checkView(typeOf[TraversableView[_, _]], typeOf[TraversableViewLike[_, _, _]]) + checkView(typeOf[IterableView[_, _]], typeOf[IterableViewLike[_, _, _]]) + checkView(typeOf[SeqView[_, _]], typeOf[SeqViewLike[_, _, _]]) + checkView(typeOf[mutable.IndexedSeqView[_, _]], typeOf[SeqViewLike[_, _, _]]) + checkView(typeOf[immutable.StreamView[_, _]], typeOf[immutable.StreamViewLike[_, _, _]]) + // Parallel views not checked, assuming we will drop them in 2.11 + } +} diff --git a/test/files/run/t4332b.scala b/test/files/run/t4332b.scala new file mode 100644 index 0000000000..8ee069ca2d --- /dev/null +++ b/test/files/run/t4332b.scala @@ -0,0 +1,35 @@ +object Test extends App { + def check(expected: Any, actual: Any, msg: String = "") = { + if (expected != actual) + sys.error(s"($actual != $expected) $msg") + } + val ls = List(1, 3, 2, 1) + for (N <- -1 to (ls.length + 1)) { + check(ls.takeRight(N), ls.view.takeRight(N).toList, s"takeRight($N)") + check(ls.dropRight(N), ls.view.dropRight(N).toList, s"dropRight($N)") + } + for (N <- 1 to (ls.length + 1)) { + check(ls.sliding(N).toList, ls.view.sliding(N).toList.map(_.toList), s"sliding($N)") + check(ls.sliding(N, 2).toList, ls.view.sliding(N, 2).toList.map(_.toList), s"sliding($N, 2)") + } + for (b <- List(true, false)) + check(ls.filterNot(x => true), ls.view.filterNot(x => true), s"filterNot($b)") + + check(ls.inits.toList, ls.view.inits.toList.map(_.toList), "inits") + check(ls.tails.toList, ls.view.tails.toList.map(_.toList), "tails") + + check(ls.combinations(2).toList.map(_.toList), ls.view.combinations(2).toList.map(_.toList), "combinations(2)") + check(ls.permutations.toList.map(_.toList), ls.view.permutations.toList.map(_.toList), "permutations") + + check(ls.sortBy(_ * -1), ls.view.sortBy(_ * -1).toList, "sortBy") + check(ls.sortWith((x, y) => y < x), ls.view.sortWith((x, y) => y < x).toList, "sortWith") + check(ls.sorted, ls.view.sorted.toList, "sorted") + + check(ls.distinct, ls.view.distinct.toList, "distinct") + + check(ls.tail, ls.view.tail.toList, "tail") + + import collection.mutable.Buffer + check(Buffer(1, 2, 3).tail, Buffer(1, 2, 3).view.tail.toList, "Buffer#tail") + check(Buffer(1, 2, 3).tail.length, Buffer(1, 2, 3).view.tail.length, "Buffer#tail#length") +} |