Merge pull request #9799 from som-snytt/issue/12479

Fix slicing of views of `IndexedSeq`s (including fixing 2.13.7-only `reverseIterator` regression)

src/library/scala/collection/IndexedSeqView.scala

@@ -84,8 +84,8 @@ object IndexedSeqView {
   }
   @SerialVersionUID(3L)
   private[collection] class IndexedSeqViewReverseIterator[A](self: IndexedSeqView[A]) extends AbstractIterator[A] with Serializable {
-    private[this] var pos = self.length - 1
     private[this] var remainder = self.length
+    private[this] var pos = remainder - 1
     @inline private[this] def _hasNext: Boolean = remainder > 0
     def hasNext: Boolean = _hasNext
     def next(): A =
@@ -96,23 +96,25 @@ object IndexedSeqView {
         r
       } else Iterator.empty.next()
 
-    override def drop(n: Int): Iterator[A] = {
-      if (n > 0) {
-        pos -= n
-        remainder = Math.max(0, remainder - n)
+    // from < 0 means don't move pos, until < 0 means don't limit remainder
+    //
+    override protected def sliceIterator(from: Int, until: Int): Iterator[A] = {
+      if (_hasNext) {
+        if (remainder <= from) remainder = 0                              // exhausted by big skip
+        else if (from <= 0) {                                             // no skip, pos is same
+          if (until >= 0 && until < remainder) remainder = until          // ...limited by until
+        }
+        else {
+          pos -= from                                                     // skip ahead
+          if (until >= 0 && until < remainder) {                          // ...limited by until
+            if (until <= from) remainder = 0                              // ...exhausted if limit is smaller than skip
+            else remainder = until - from                                 // ...limited by until, less the skip
+          }
+          else remainder -= from                                          // ...otherwise just less the skip
+        }
       }
       this
     }
-
-    override def sliceIterator(from: Int, until: Int): Iterator[A] = {
-      val startCutoff = pos
-      val untilCutoff = startCutoff - remainder + 1
-      val nextStartCutoff = if (from < 0) startCutoff else if (startCutoff - from < 0) 0 else startCutoff - from
-      val nextUntilCutoff = if (until < 0) startCutoff else if (startCutoff - until < untilCutoff) untilCutoff else startCutoff - until + 1
-      remainder = Math.max(0, nextStartCutoff - nextUntilCutoff + 1)
-      pos = nextStartCutoff
-      this
-    }
   }
 
   /** An `IndexedSeqOps` whose collection type and collection type constructor are unknown */

src/library/scala/collection/Iterator.scala

@@ -409,9 +409,9 @@ trait Iterator[+A] extends IterableOnce[A] with IterableOnceOps[A, Iterator, Ite
 
   def indexWhere(p: A => Boolean, from: Int = 0): Int = {
     var i = math.max(from, 0)
-    drop(from)
-    while (hasNext) {
-      if (p(next())) return i
+    val dropped = drop(from)
+    while (dropped.hasNext) {
+      if (p(dropped.next())) return i
       i += 1
     }
     -1
@@ -635,14 +635,7 @@ trait Iterator[+A] extends IterableOnce[A] with IterableOnceOps[A, Iterator, Ite
     def next() = if (hasNext) { hdDefined = false; hd } else Iterator.empty.next()
   }
 
-  def drop(n: Int): Iterator[A] = {
-    var i = 0
-    while (i < n && hasNext) {
-      next()
-      i += 1
-    }
-    this
-  }
+  def drop(n: Int): Iterator[A] = sliceIterator(n, -1)
 
   def dropWhile(p: A => Boolean): Iterator[A] = new AbstractIterator[A] {
     // Magic value: -1 = hasn't dropped, 0 = found first, 1 = defer to parent iterator
@@ -972,6 +965,7 @@ object Iterator extends IterableFactory[Iterator] {
     def hasNext = false
     def next() = throw new NoSuchElementException("next on empty iterator")
     override def knownSize: Int = 0
+    override protected def sliceIterator(from: Int, until: Int) = this
   }
 
   /** Creates a target $coll from an existing source collection
@@ -989,6 +983,9 @@ object Iterator extends IterableFactory[Iterator] {
     private[this] var consumed: Boolean = false
     def hasNext = !consumed
     def next() = if (consumed) empty.next() else { consumed = true; a }
+    override protected def sliceIterator(from: Int, until: Int) =
+      if (consumed || from > 0 || until == 0) empty
+      else this
   }
 
   override def apply[A](xs: A*): Iterator[A] = xs.iterator

test/junit/scala/collection/IndexedSeqViewTest.scala

@@ -18,4 +18,9 @@ class IndexedSeqViewTest {
     assertEquals(2, IndexedSeq(1, 2, 3, 4, 5).view.iterator.take(2).knownSize)
     assertEquals(2, IndexedSeq(1, 2, 3, 4, 5).view.iterator.slice(2, 4).knownSize)
   }
+
+  @Test
+  def reverseEmptyIterator(): Unit = {
+    assertEquals(0, Vector.empty[Int].reverseIterator.take(1).toList.size)
+  }
 }

test/junit/scala/collection/immutable/SmallMapTest.scala

@@ -6,14 +6,14 @@ import org.junit._
 import scala.tools.testkit.AllocationTest
 
 class SmallMapTest extends AllocationTest {
-  def iterator(m:Map[_,_]) = m.iterator
-  def keysIterator(m:Map[_,_]) = m.keysIterator
-  def valuesIterator(m:Map[_,_]) = m.valuesIterator
+  def iterator(m: Map[_,_])       = m.iterator
+  def keysIterator(m: Map[_,_])   = m.keysIterator
+  def valuesIterator(m: Map[_,_]) = m.valuesIterator
 
-  //we use this side effect to avoid the git optimising away the tuples
-  //but without affecting the allocations
+  // we use this side effect to avoid the jit optimising away the tuples
+  // but without affecting the allocations
   val nonAllocationResult = new Array[Any](5)
-  def consume(it:Iterator[_]): Int = {
+  def consume(it: Iterator[_]): Int = {
     var size = 0
     nonAllocationResult(0) = it
     while (it.hasNext) {
@@ -22,7 +22,7 @@ class SmallMapTest extends AllocationTest {
     }
     size
   }
-  def consume1(it:Iterator[_]): Int = {
+  def consume1(it: Iterator[_]): Int = {
     nonAllocationResult(0) = it
     nonAllocationResult(1) = it.next()
     1