Skip to content

Commit f12bb7b

Browse files
committed
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
1 parent 744425a commit f12bb7b

File tree

7 files changed

+142
-0
lines changed

7 files changed

+142
-0
lines changed

src/library/scala/collection/IterableViewLike.scala

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,5 +117,14 @@ trait IterableViewLike[+A,
117117
override def sliding(size: Int, step: Int): Iterator[This] =
118118
self.iterator.sliding(size, step) map (x => newForced(x).asInstanceOf[This])
119119

120+
override def sliding(size: Int): Iterator[This] =
121+
sliding(size, 1) // we could inherit this, but that implies knowledge of the way the super class is implemented.
122+
123+
override def dropRight(n: Int): This =
124+
take(thisSeq.length - n)
125+
126+
override def takeRight(n: Int): This =
127+
drop(thisSeq.length - n)
128+
120129
override def stringPrefix = "IterableView"
121130
}

src/library/scala/collection/SeqViewLike.scala

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,5 +137,20 @@ trait SeqViewLike[+A,
137137
override def sorted[B >: A](implicit ord: Ordering[B]): This =
138138
newForced(thisSeq sorted ord).asInstanceOf[This]
139139

140+
override def sortWith(lt: (A, A) => Boolean): This =
141+
newForced(thisSeq sortWith lt).asInstanceOf[This]
142+
143+
override def sortBy[B](f: (A) => B)(implicit ord: Ordering[B]): This =
144+
newForced(thisSeq sortBy f).asInstanceOf[This]
145+
146+
override def combinations(n: Int): Iterator[This] =
147+
(thisSeq combinations n).map(as => newForced(as).asInstanceOf[This])
148+
149+
override def permutations: Iterator[This] =
150+
thisSeq.permutations.map(as => newForced(as).asInstanceOf[This])
151+
152+
override def distinct: This =
153+
newForced(thisSeq.distinct).asInstanceOf[This]
154+
140155
override def stringPrefix = "SeqView"
141156
}

src/library/scala/collection/TraversableViewLike.scala

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,5 +209,18 @@ trait TraversableViewLike[+A,
209209
override def unzip3[A1, A2, A3](implicit asTriple: A => (A1, A2, A3)) =
210210
(newMapped(x => asTriple(x)._1), newMapped(x => asTriple(x)._2), newMapped(x => asTriple(x)._3)) // TODO - Performance improvements.
211211

212+
override def filterNot(p: (A) => Boolean): This =
213+
newFiltered(a => !(p(a)))
214+
215+
override def inits: Iterator[This] =
216+
thisSeq.inits.map(as => newForced(as).asInstanceOf[This])
217+
218+
override def tails: Iterator[This] =
219+
thisSeq.tails.map(as => newForced(as).asInstanceOf[This])
220+
221+
override def tail: This =
222+
// super.tail would also work as it is currently implemented in terms of drop(Int).
223+
if (isEmpty) super.tail else newDropped(1)
224+
212225
override def toString = viewToString
213226
}

src/library/scala/collection/mutable/IndexedSeqView.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@ self =>
9393
override def span(p: A => Boolean): (This, This) = (newTakenWhile(p), newDroppedWhile(p))
9494
override def splitAt(n: Int): (This, This) = (take(n), drop(n)) // !!!
9595
override def reverse: This = newReversed
96+
override def tail: IndexedSeqView[A, Coll] = if (isEmpty) super.tail else slice(1, length)
9697
}
9798

9899
/** An object containing the necessary implicit definitions to make

test/files/run/t4332.check

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
2+
======================================================================
3+
Checking scala.collection.TraversableView
4+
======================================================================
5+
6+
7+
======================================================================
8+
Checking scala.collection.IterableView
9+
======================================================================
10+
11+
12+
======================================================================
13+
Checking scala.collection.SeqView
14+
======================================================================
15+
16+
17+
======================================================================
18+
Checking scala.collection.mutable.IndexedSeqView
19+
======================================================================
20+
21+
22+
======================================================================
23+
Checking scala.collection.immutable.StreamView
24+
======================================================================
25+

test/files/run/t4332.scala

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import scala.tools.partest._
2+
3+
object Test extends DirectTest {
4+
override def code = ""
5+
lazy val global = newCompiler("-usejavacp")
6+
import global._, definitions._
7+
8+
override def show() {
9+
new global.Run()
10+
// Once we plug all of the view gaps, the output should be empty!
11+
checkViews()
12+
}
13+
14+
def isExempt(sym: Symbol) = {
15+
val exempt = Set("view", "repr", "sliceWithKnownDelta", "sliceWithKnownBound", "transform")
16+
(exempt contains sym.name.decoded)
17+
}
18+
19+
def checkView(viewType: Type, viewLikeType: Type) {
20+
val sep = "=" * 70
21+
println(s"\n$sep\nChecking ${viewType.typeSymbol.fullName}\n$sep")
22+
val termMembers = viewType.nonPrivateMembers.toList filter (_.isTerm) map fullyInitializeSymbol
23+
val inheritedFromGenericCollection
24+
= termMembers filterNot (_.owner.name.decoded contains "ViewLike") filterNot (_.owner == viewType.typeSymbol)
25+
def returnsView(sym: Symbol) = viewType.memberType(sym).finalResultType contains viewType.typeSymbol
26+
val needOverride = inheritedFromGenericCollection filterNot isExempt filter returnsView
27+
28+
val grouped = needOverride.groupBy(_.owner).toSeq.sortBy { case (owner, _) => viewType baseTypeIndex owner }
29+
val report = grouped.map {
30+
case (owner, syms) => s"\n$owner\n${"-" * 70}\n${syms.map(_.defString).sorted.mkString("\n")}"
31+
}.mkString("\n")
32+
println(report)
33+
}
34+
35+
def checkViews() {
36+
import collection._
37+
checkView(typeOf[TraversableView[_, _]], typeOf[TraversableViewLike[_, _, _]])
38+
checkView(typeOf[IterableView[_, _]], typeOf[IterableViewLike[_, _, _]])
39+
checkView(typeOf[SeqView[_, _]], typeOf[SeqViewLike[_, _, _]])
40+
checkView(typeOf[mutable.IndexedSeqView[_, _]], typeOf[SeqViewLike[_, _, _]])
41+
checkView(typeOf[immutable.StreamView[_, _]], typeOf[immutable.StreamViewLike[_, _, _]])
42+
// Parallel views not checked, assuming we will drop them in 2.11
43+
}
44+
}

test/files/run/t4332b.scala

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
object Test extends App {
2+
def check(expected: Any, actual: Any, msg: String = "") = {
3+
if (expected != actual)
4+
sys.error(s"($actual != $expected) $msg")
5+
}
6+
val ls = List(1, 3, 2, 1)
7+
for (N <- -1 to (ls.length + 1)) {
8+
check(ls.takeRight(N), ls.view.takeRight(N).toList, s"takeRight($N)")
9+
check(ls.dropRight(N), ls.view.dropRight(N).toList, s"dropRight($N)")
10+
}
11+
for (N <- 1 to (ls.length + 1)) {
12+
check(ls.sliding(N).toList, ls.view.sliding(N).toList.map(_.toList), s"sliding($N)")
13+
check(ls.sliding(N, 2).toList, ls.view.sliding(N, 2).toList.map(_.toList), s"sliding($N, 2)")
14+
}
15+
for (b <- List(true, false))
16+
check(ls.filterNot(x => true), ls.view.filterNot(x => true), s"filterNot($b)")
17+
18+
check(ls.inits.toList, ls.view.inits.toList.map(_.toList), "inits")
19+
check(ls.tails.toList, ls.view.tails.toList.map(_.toList), "tails")
20+
21+
check(ls.combinations(2).toList.map(_.toList), ls.view.combinations(2).toList.map(_.toList), "combinations(2)")
22+
check(ls.permutations.toList.map(_.toList), ls.view.permutations.toList.map(_.toList), "permutations")
23+
24+
check(ls.sortBy(_ * -1), ls.view.sortBy(_ * -1).toList, "sortBy")
25+
check(ls.sortWith((x, y) => y < x), ls.view.sortWith((x, y) => y < x).toList, "sortWith")
26+
check(ls.sorted, ls.view.sorted.toList, "sorted")
27+
28+
check(ls.distinct, ls.view.distinct.toList, "distinct")
29+
30+
check(ls.tail, ls.view.tail.toList, "tail")
31+
32+
import collection.mutable.Buffer
33+
check(Buffer(1, 2, 3).tail, Buffer(1, 2, 3).view.tail.toList, "Buffer#tail")
34+
check(Buffer(1, 2, 3).tail.length, Buffer(1, 2, 3).view.tail.length, "Buffer#tail#length")
35+
}

0 commit comments

Comments
 (0)