Merge pull request #4133 from som-snytt/issue/8976

SI-8976 MutableList.tail.iterator.size is length

5 files changed, 127 insertions, 9 deletions

diff --git a/src/library/scala/collection/mutable/MutableList.scala b/src/library/scala/collection/mutable/MutableList.scala
index b852a4747b..646023f469 100644
--- a/src/library/scala/collection/mutable/MutableList.scala
+++ b/src/library/scala/collection/mutable/MutableList.scala
@@ -13,7 +13,6 @@ package mutable
 import generic._
 import immutable.{List, Nil}
 
-// !!! todo: convert to LinkedListBuffer?
 /**
  *  This class is used internally to represent mutable lists. It is the
  *  basis for the implementation of the class `Queue`.
@@ -113,9 +112,21 @@ extends AbstractSeq[A]
     }
   }
 
-  /** Returns an iterator over all elements of this list.
+  /** Returns an iterator over up to `length` elements of this list.
    */
-  override def iterator: Iterator[A] = first0.iterator
+  override def iterator: Iterator[A] = if (isEmpty) Iterator.empty else
+    new AbstractIterator[A] {
+      var elems   = first0
+      var count   = len
+      def hasNext = count > 0 && elems.nonEmpty
+      def next()  = {
+        if (!hasNext) throw new NoSuchElementException
+        count = count - 1
+        val e = elems.elem
+        elems = if (count == 0) null else elems.next
+        e
+      }
+    }
 
   override def last = {
     if (isEmpty) throw new NoSuchElementException("MutableList.empty.last")
diff --git a/test/junit/scala/collection/mutable/MutableListTest.scala b/test/junit/scala/collection/mutable/MutableListTest.scala
new file mode 100644
index 0000000000..ac6d30def0
--- /dev/null
+++ b/test/junit/scala/collection/mutable/MutableListTest.scala
@@ -0,0 +1,37 @@
+package scala.collection.mutable
+
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.junit.Test
+import org.junit.Assert._
+
+import scala.tools.testing.AssertUtil._
+
+@RunWith(classOf[JUnit4])
+class MutableListTest {
+  
+  // Tests SI-8976
+  @Test def tailIteratorMustTerminateAtLength(): Unit = {
+    val is = MutableList(1,2,3)
+    val tl = is.tail
+    assertEquals(tl.length, tl.iterator.length)
+    is += 5
+    assertEquals(tl.length, tl.iterator.length)
+    assertSameElements(tl, tl.iterator)
+  }
+  @Test def iteratorMustFailEventually(): Unit = assertThrows[NoSuchElementException] {
+    MutableList[Unit]().iterator.next()
+  }
+  // was: Root empty iterator held reference
+  @Test def iteratorMustNotHoldOntoLast(): Unit = {
+    val is = MutableList(Some(1), Some(2))
+    val it = is.iterator
+    val x  = Some(3)
+    is += x
+    assertNotReachable(x, it) {
+      it.next()
+      it.next()
+    }
+    assertTrue(it.isEmpty)
+  }
+}
diff --git a/test/junit/scala/tools/testing/AssertThrowsTest.scala b/test/junit/scala/tools/testing/AssertThrowsTest.scala
index d91e450bac..76758f51d2 100644
--- a/test/junit/scala/tools/testing/AssertThrowsTest.scala
+++ b/test/junit/scala/tools/testing/AssertThrowsTest.scala
@@ -38,6 +38,6 @@ class AssertThrowsTest {
     } catch {
       case e: AssertionError => return
     }
-    assert(false, "assertThrows should error if the tested expression does not throw anything")
+    fail("assertThrows should error if the tested expression does not throw anything")
   }
 }
diff --git a/test/junit/scala/tools/testing/AssertUtil.scala b/test/junit/scala/tools/testing/AssertUtil.scala
index 83a637783f..d29f9a473f 100644
--- a/test/junit/scala/tools/testing/AssertUtil.scala
+++ b/test/junit/scala/tools/testing/AssertUtil.scala
@@ -2,18 +2,42 @@ package scala.tools
 package testing
 
 import org.junit.Assert
-import Assert.fail
+import Assert._
 import scala.runtime.ScalaRunTime.stringOf
 import scala.collection.{ GenIterable, IterableLike }
+import scala.collection.JavaConverters._
+import scala.collection.mutable
+import java.lang.ref._
+import java.lang.reflect._
+import java.util.IdentityHashMap
 
 /** This module contains additional higher-level assert statements
  *  that are ultimately based on junit.Assert primitives.
  */
 object AssertUtil {
-  /**
-   * Check if throwable T (or a subclass) was thrown during evaluation of f, and that its message
-   * satisfies the `checkMessage` predicate.
-   * If any other exception will be re-thrown.
+  private final val timeout = 60 * 1000L                 // wait a minute
+
+  private implicit class `ref helper`[A](val r: Reference[A]) extends AnyVal {
+    def isEmpty: Boolean  = r.get == null
+    def nonEmpty: Boolean = !isEmpty
+  }
+  private implicit class `class helper`(val clazz: Class[_]) extends AnyVal {
+    def allFields: List[Field] = {
+      def loop(k: Class[_]): List[Field] =
+        if (k == null) Nil
+        else k.getDeclaredFields.toList ::: loop(k.getSuperclass)
+      loop(clazz)
+    }
+  }
+  private implicit class `field helper`(val f: Field) extends AnyVal {
+    def follow(o: AnyRef): AnyRef = {
+      f setAccessible true
+      f get o
+    }
+  }
+
+  /** Check if throwable T (or a subclass) was thrown during evaluation of f, and that its message
+   *  satisfies the `checkMessage` predicate. If any other exception will be re-thrown.
    */
   def assertThrows[T <: Throwable](f: => Any,
                                    checkMessage: String => Boolean = s => true)
@@ -41,4 +65,29 @@ object AssertUtil {
    */
   def assertSameElements[A, B >: A](expected: IterableLike[A, _], actual: Iterator[B]): Unit =
     assertSameElements(expected, actual.toList, "")
+
+  /** Value is not strongly reachable from roots after body is evaluated.
+   */
+  def assertNotReachable[A <: AnyRef](a: => A, roots: AnyRef*)(body: => Unit): Unit = {
+    val wkref = new WeakReference(a)
+    def refs(root: AnyRef): mutable.Set[AnyRef] = {
+      val seen = new IdentityHashMap[AnyRef, Unit]
+      def loop(o: AnyRef): Unit =
+        if (wkref.nonEmpty && o != null && !seen.containsKey(o)) {
+          seen.put(o, ())
+          for {
+            f <- o.getClass.allFields
+            if !Modifier.isStatic(f.getModifiers)
+            if !f.getType.isPrimitive
+            if !classOf[Reference[_]].isAssignableFrom(f.getType)
+          } loop(f follow o)
+        }
+      loop(root)
+      seen.keySet.asScala
+    }
+    body
+    for (r <- roots if wkref.nonEmpty) {
+      assertFalse(s"Root $r held reference", refs(r) contains wkref.get)
+    }
+  }
 }
diff --git a/test/junit/scala/tools/testing/AssertUtilTest.scala b/test/junit/scala/tools/testing/AssertUtilTest.scala
new file mode 100644
index 0000000000..03d8815ab2
--- /dev/null
+++ b/test/junit/scala/tools/testing/AssertUtilTest.scala
@@ -0,0 +1,21 @@
+package scala.tools
+package testing
+
+import org.junit.Assert._
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import AssertUtil._
+
+import java.lang.ref._
+
+@RunWith(classOf[JUnit4])
+class AssertUtilTest {
+
+  @Test def reachableIgnoresReferences(): Unit = {
+    class Holder[A](val ref: SoftReference[A])
+    val o = new Object
+    val r = new SoftReference(o)
+    assertNotReachable(o, new Holder(r)) { }
+  }
+}