SI-6519 PagedSeq is not lazy enough

This was actually an issue with `length` of all things--it wasn't stopping scanning when it was past the end of what was requested.  It'd just gratuitously read everything.

Also fixed an overflow bug with isDefinedAt along the way (start + index > Int.MaxValue would always return true despite never working).

2 files changed, 21 insertions, 5 deletions

diff --git a/src/library/scala/collection/immutable/PagedSeq.scala b/src/library/scala/collection/immutable/PagedSeq.scala
index 3a64820be6..f11217d26a 100644
--- a/src/library/scala/collection/immutable/PagedSeq.scala
+++ b/src/library/scala/collection/immutable/PagedSeq.scala
@@ -158,7 +158,7 @@ extends scala.collection.AbstractSeq[T]
    *  @note Calling this method will force the entire sequence to be read.
    */
   def length: Int = {
-    while (!latest.isLast) addMore()
+    while (!latest.isLast && latest.end < end) addMore()
     (latest.end min end) - start
   }
 
@@ -175,7 +175,8 @@ extends scala.collection.AbstractSeq[T]
    */
   override def isDefinedAt(index: Int) =
     index >= 0 && index < end - start && {
-      val p = page(index + start); index + start < p.end
+      val absidx = index + start
+      absidx >= 0 && absidx < page(absidx).end
     }
 
    /** The subsequence from index `start` up to `end -1` if `end`
@@ -192,6 +193,9 @@ extends scala.collection.AbstractSeq[T]
       if (f.next eq null) f.addMore(more)
       f = f.next
     }
+    // Warning -- not refining `more` means that slices can freely request and obtain
+    // data outside of their slice.  This is part of the design of PagedSeq
+    // (to read pages!) but can be surprising.
     new PagedSeq(more, f, s, e)
   }
 
diff --git a/test/junit/scala/collection/immutable/PagedSeqTest.scala b/test/junit/scala/collection/immutable/PagedSeqTest.scala
index 5f83cf6f31..2b576a3655 100644
--- a/test/junit/scala/collection/immutable/PagedSeqTest.scala
+++ b/test/junit/scala/collection/immutable/PagedSeqTest.scala
@@ -5,12 +5,24 @@ import org.junit.runners.JUnit4
 import org.junit.Test
 import org.junit.Assert._
 
-/* Test for SI-6615  */
 @RunWith(classOf[JUnit4])
 class PagedSeqTest {
+  // should not NPE, and should equal the given Seq
   @Test
-  def rovingDoesNotNPE(): Unit = {
-    // should not NPE, and should equal the given Seq
+  def test_SI6615(): Unit = {
     assertEquals(Seq('a'), PagedSeq.fromStrings(List.fill(5000)("a")).slice(4096, 4097))
   }
+
+  // Slices shouldn't read outside where they belong
+  @Test
+  def test_SI6519 {
+    var readAttempt = 0
+    val sideEffectingIterator = new Iterator[Int] {
+      def hasNext = readAttempt < 65536
+      def next = { readAttempt += 1; readAttempt }
+    }
+    val s = PagedSeq.fromIterator(sideEffectingIterator).slice(0,2).mkString
+    assertEquals(s, "12")
+    assert(readAttempt <= 4096)
+  }
 }