Skip to content

Commit ff1b667

Browse files
committed
SI-8691 SeqView throws exception when prepending a collection
Prepend was throwing segfaults as it was handled differently from Append. This brings the two into line with each other. There are various optimizations that could be applied that have not been, however, such as intercepting sequential prepends and generating one multi-prepend instead of nested single-element prepends. Unit test added to verify minimally that bug is gone.
1 parent f13fc65 commit ff1b667

File tree

5 files changed

+50
-17
lines changed

5 files changed

+50
-17
lines changed

src/library/scala/collection/IterableViewLike.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,10 @@ trait IterableViewLike[+A,
6969
trait Appended[B >: A] extends super.Appended[B] with Transformed[B] {
7070
def iterator = self.iterator ++ rest
7171
}
72+
73+
trait Prepended[B >: A] extends super.Prepended[B] with Transformed[B] {
74+
def iterator = fst.toIterator ++ self
75+
}
7276

7377
trait Filtered extends super.Filtered with Transformed[A] {
7478
def iterator = self.iterator filter pred
@@ -110,6 +114,7 @@ trait IterableViewLike[+A,
110114
} with AbstractTransformed[(A1, B)] with ZippedAll[A1, B]
111115
protected override def newForced[B](xs: => GenSeq[B]): Transformed[B] = new { val forced = xs } with AbstractTransformed[B] with Forced[B]
112116
protected override def newAppended[B >: A](that: GenTraversable[B]): Transformed[B] = new { val rest = that } with AbstractTransformed[B] with Appended[B]
117+
protected override def newPrepended[B >: A](that: GenTraversable[B]): Transformed[B] = new { val fst = that } with AbstractTransformed[B] with Prepended[B]
113118
protected override def newMapped[B](f: A => B): Transformed[B] = new { val mapping = f } with AbstractTransformed[B] with Mapped[B]
114119
protected override def newFlatMapped[B](f: A => GenTraversableOnce[B]): Transformed[B] = new { val mapping = f } with AbstractTransformed[B] with FlatMapped[B]
115120
protected override def newFiltered(p: A => Boolean): Transformed[A] = new { val pred = p } with AbstractTransformed[A] with Filtered

src/library/scala/collection/SeqViewLike.scala

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,14 @@ trait SeqViewLike[+A,
9595
if (idx < self.length) self(idx) else restSeq(idx - self.length)
9696
}
9797

98+
trait Prepended[B >: A] extends super.Prepended[B] with Transformed[B] {
99+
protected[this] lazy val fstSeq = fst.toSeq
100+
def length: Int = fstSeq.length + self.length
101+
def apply(idx: Int): B =
102+
if (idx < fstSeq.length) fstSeq(idx)
103+
else self.apply(idx - fstSeq.length)
104+
}
105+
98106
trait Filtered extends super.Filtered with Transformed[A] {
99107
protected[this] lazy val index = {
100108
var len = 0
@@ -178,21 +186,12 @@ trait SeqViewLike[+A,
178186
final override protected[this] def viewIdentifier = "P"
179187
}
180188

181-
trait Prepended[B >: A] extends Transformed[B] {
182-
protected[this] val fst: B
183-
override def iterator: Iterator[B] = Iterator.single(fst) ++ self.iterator
184-
def length: Int = 1 + self.length
185-
def apply(idx: Int): B =
186-
if (idx == 0) fst
187-
else self.apply(idx - 1)
188-
final override protected[this] def viewIdentifier = "A"
189-
}
190-
191189
/** Boilerplate method, to override in each subclass
192190
* This method could be eliminated if Scala had virtual classes
193191
*/
194192
protected override def newForced[B](xs: => GenSeq[B]): Transformed[B] = new { val forced = xs } with AbstractTransformed[B] with Forced[B]
195193
protected override def newAppended[B >: A](that: GenTraversable[B]): Transformed[B] = new { val rest = that } with AbstractTransformed[B] with Appended[B]
194+
protected override def newPrepended[B >: A](that: GenTraversable[B]): Transformed[B] = new { protected[this] val fst = that } with AbstractTransformed[B] with Prepended[B]
196195
protected override def newMapped[B](f: A => B): Transformed[B] = new { val mapping = f } with AbstractTransformed[B] with Mapped[B]
197196
protected override def newFlatMapped[B](f: A => GenTraversableOnce[B]): Transformed[B] = new { val mapping = f } with AbstractTransformed[B] with FlatMapped[B]
198197
protected override def newFiltered(p: A => Boolean): Transformed[A] = new { val pred = p } with AbstractTransformed[A] with Filtered
@@ -211,7 +210,6 @@ trait SeqViewLike[+A,
211210
val patch = _patch
212211
val replaced = _replaced
213212
} with AbstractTransformed[B] with Patched[B]
214-
protected def newPrepended[B >: A](elem: B): Transformed[B] = new { protected[this] val fst = elem } with AbstractTransformed[B] with Prepended[B]
215213

216214
// see comment in IterableViewLike.
217215
protected override def newTaken(n: Int): Transformed[A] = newSliced(SliceInterval(0, n))
@@ -241,7 +239,7 @@ trait SeqViewLike[+A,
241239
}
242240

243241
override def +:[B >: A, That](elem: B)(implicit bf: CanBuildFrom[This, B, That]): That =
244-
newPrepended(elem).asInstanceOf[That]
242+
newPrepended(elem :: Nil).asInstanceOf[That]
245243

246244
override def :+[B >: A, That](elem: B)(implicit bf: CanBuildFrom[This, B, That]): That =
247245
++(Iterator.single(elem))(bf)

src/library/scala/collection/TraversableViewLike.scala

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,15 @@ trait TraversableViewLike[+A,
189189
}
190190
final override protected[this] def viewIdentifier = "A"
191191
}
192+
193+
trait Prepended[B >: A] extends Transformed[B] {
194+
protected[this] val fst: GenTraversable[B]
195+
def foreach[U](f: B => U) {
196+
fst foreach f
197+
self foreach f
198+
}
199+
final override protected[this] def viewIdentifier = "A"
200+
}
192201

193202
trait Filtered extends Transformed[A] {
194203
protected[this] val pred: A => Boolean
@@ -222,11 +231,15 @@ trait TraversableViewLike[+A,
222231
final override protected[this] def viewIdentifier = "D"
223232
}
224233

225-
override def ++[B >: A, That](xs: GenTraversableOnce[B])(implicit bf: CanBuildFrom[This, B, That]): That = {
234+
override def ++[B >: A, That](xs: GenTraversableOnce[B])(implicit bf: CanBuildFrom[This, B, That]): That =
226235
newAppended(xs.seq.toTraversable).asInstanceOf[That]
227-
// was: if (bf.isInstanceOf[ByPassCanBuildFrom]) newAppended(that).asInstanceOf[That]
228-
// else super.++[B, That](that)(bf)
229-
}
236+
237+
override def ++:[B >: A, That](xs: TraversableOnce[B])(implicit bf: CanBuildFrom[This, B, That]): That =
238+
newPrepended(xs.seq.toTraversable).asInstanceOf[That]
239+
240+
// Need second one because of optimization in TraversableLike
241+
override def ++:[B >: A, That](xs: Traversable[B])(implicit bf: CanBuildFrom[This, B, That]): That =
242+
newPrepended(xs).asInstanceOf[That]
230243

231244
override def map[B, That](f: A => B)(implicit bf: CanBuildFrom[This, B, That]): That = {
232245
newMapped(f).asInstanceOf[That]
@@ -253,6 +266,7 @@ trait TraversableViewLike[+A,
253266
*/
254267
protected def newForced[B](xs: => GenSeq[B]): Transformed[B] = new { val forced = xs } with AbstractTransformed[B] with Forced[B]
255268
protected def newAppended[B >: A](that: GenTraversable[B]): Transformed[B] = new { val rest = that } with AbstractTransformed[B] with Appended[B]
269+
protected def newPrepended[B >: A](that: GenTraversable[B]): Transformed[B] = new { val fst = that } with AbstractTransformed[B] with Prepended[B]
256270
protected def newMapped[B](f: A => B): Transformed[B] = new { val mapping = f } with AbstractTransformed[B] with Mapped[B]
257271
protected def newFlatMapped[B](f: A => GenTraversableOnce[B]): Transformed[B] = new { val mapping = f } with AbstractTransformed[B] with FlatMapped[B]
258272
protected def newFiltered(p: A => Boolean): Transformed[A] = new { val pred = p } with AbstractTransformed[A] with Filtered

src/library/scala/collection/immutable/StreamViewLike.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ extends SeqView[A, Coll]
5353
/** boilerplate */
5454
protected override def newForced[B](xs: => scala.collection.GenSeq[B]): Transformed[B] = new { val forced = xs } with AbstractTransformed[B] with Forced[B]
5555
protected override def newAppended[B >: A](that: scala.collection.GenTraversable[B]): Transformed[B] = new { val rest = that } with AbstractTransformed[B] with Appended[B]
56+
protected override def newPrepended[B >: A](that: scala.collection.GenTraversable[B]): Transformed[B] = new { protected[this] val fst = that } with AbstractTransformed[B] with Prepended[B]
5657
protected override def newMapped[B](f: A => B): Transformed[B] = new { val mapping = f } with AbstractTransformed[B] with Mapped[B]
5758
protected override def newFlatMapped[B](f: A => scala.collection.GenTraversableOnce[B]): Transformed[B] = new { val mapping = f } with AbstractTransformed[B] with FlatMapped[B]
5859
protected override def newFiltered(p: A => Boolean): Transformed[A] = new { val pred = p } with AbstractTransformed[A] with Filtered
@@ -67,7 +68,6 @@ extends SeqView[A, Coll]
6768
protected override def newPatched[B >: A](_from: Int, _patch: scala.collection.GenSeq[B], _replaced: Int): Transformed[B] = {
6869
new { val from = _from; val patch = _patch; val replaced = _replaced } with AbstractTransformed[B] with Patched[B]
6970
}
70-
protected override def newPrepended[B >: A](elem: B): Transformed[B] = new { protected[this] val fst = elem } with AbstractTransformed[B] with Prepended[B]
7171

7272
override def stringPrefix = "StreamView"
7373
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
package scala.collection
2+
3+
import org.junit.runner.RunWith
4+
import org.junit.runners.JUnit4
5+
import org.junit.Assert._
6+
import org.junit.Test
7+
8+
@RunWith(classOf[JUnit4])
9+
class SeqViewTest {
10+
11+
@Test
12+
def test_SI8691() {
13+
// Really just testing to make sure ++: doesn't throw an exception
14+
assert( Seq(1,2) ++: Seq(3,4).view == Seq(1,2,3,4) )
15+
}
16+
}

0 commit comments

Comments
 (0)