Review cleanups

src/library/scala/collection/IterableOnce.scala

@@ -283,15 +283,6 @@ object IterableOnce {
       case src: Iterable[A] => src.copyToArray[B](xs, start, len)
       case src              => src.iterator.copyToArray[B](xs, start, len)
     }
-
-  @inline private[collection] def checkArraySizeWithinVMLimit(size: Int): Unit = {
-    import scala.runtime.PStatics.VM_MaxArraySize
-    if (size > VM_MaxArraySize) {
-      throw new Exception(s"Size of array-backed collection exceeds VM array size limit of $VM_MaxArraySize.  Encountered size: $size")
-    } else if (size < 0) {
-      throw new Exception(s"Size of array-backed collection must exceed 0.  Encountered size: $size")
-    }
-  }
 }
 
 /** This implementation trait can be mixed into an `IterableOnce` to get the basic methods that are shared between

src/library/scala/collection/mutable/ArrayBuffer.scala

@@ -15,11 +15,10 @@ package collection
 package mutable
 
 import java.util.Arrays
-
-import scala.annotation.nowarn
-import scala.annotation.tailrec
+import scala.annotation.{nowarn, tailrec}
 import scala.collection.Stepper.EfficientSplit
 import scala.collection.generic.DefaultSerializable
+import scala.runtime.PStatics.VM_MaxArraySize
 
 /** An implementation of the `Buffer` class using an array to
   *  represent the assembled sequence internally. Append, update and random
@@ -307,24 +306,29 @@ object ArrayBuffer extends StrictOptimizedSeqFactory[ArrayBuffer] {
 
   def empty[A]: ArrayBuffer[A] = new ArrayBuffer[A]()
 
-  import scala.runtime.PStatics.VM_MaxArraySize
+  @inline private def checkArrayLengthLimit(length: Int, currentLength: Int): Unit =
+    if (length > VM_MaxArraySize)
+      throw new Exception(s"Array of array-backed collection exceeds VM length limit of $VM_MaxArraySize. Requested length: $length; current length: $currentLength")
+    else if (length < 0)
+      throw new Exception(s"Overflow while resizing array of array-backed collection. Requested length: $length; current length: $currentLength; increase: ${length - currentLength}")
+
   /**
+   * The increased size for an array-backed collection.
+   *
    * @param arrayLen  the length of the backing array
    * @param targetLen the minimum length to resize up to
-   * @return -1 if no resizing is needed, the greater of targetLen and 2 * arrayLen if neither exceeds VM_MaxArraySize,
-   *         or VM_MaxArraySize otherwise.
+   * @return
+   *   - `-1` if no resizing is needed, else
+   *   - `VM_MaxArraySize` if `arrayLen` is too large to be doubled, else
+   *   - `max(targetLen, arrayLen * 2, , DefaultInitialSize)`.
+   *   - Throws an exception if `targetLen` exceeds `VM_MaxArraySize` or is negative (overflow).
    */
   private[mutable] def resizeUp(arrayLen: Int, targetLen: Int): Int = {
-    // Hybrid
     if (targetLen > 0 && targetLen <= arrayLen) -1
     else {
-      IterableOnce.checkArraySizeWithinVMLimit(targetLen) // safe because `targetSize <= Int.MaxValue`
-
-      math.min(
-        if (targetLen > (Int.MaxValue >> 1)) VM_MaxArraySize
-        else math.max(targetLen, math.max(arrayLen << 1, DefaultInitialSize)),
-        VM_MaxArraySize
-      )
+      checkArrayLengthLimit(targetLen, arrayLen)
+      if (arrayLen > VM_MaxArraySize / 2) VM_MaxArraySize
+      else math.max(targetLen, math.max(arrayLen * 2, DefaultInitialSize))
     }
   }
 

src/library/scala/collection/mutable/ArrayBuilder.scala

@@ -488,21 +488,21 @@ object ArrayBuilder {
     protected def elems: Array[Unit] = throw new UnsupportedOperationException()
 
     def addOne(elem: Unit): this.type = {
-      val newSize:Int = size + 1
+      val newSize = size + 1
       ensureSize(newSize)
       size = newSize
       this
     }
 
     override def addAll(xs: IterableOnce[Unit]): this.type = {
-      val newSize:Int = size + xs.iterator.size
+      val newSize = size + xs.iterator.size
       ensureSize(newSize)
       size = newSize
       this
     }
 
     override def addAll(xs: Array[_ <: Unit], offset: Int, length: Int): this.type = {
-      val newSize: Int = size + length
+      val newSize = size + length
       ensureSize(newSize)
       size = newSize
       this
@@ -520,9 +520,8 @@ object ArrayBuilder {
       case _ => false
     }
 
-    protected[this] def resize(size: Int): Unit = ()
+    protected[this] def resize(size: Int): Unit = capacity = size
 
     override def toString = "ArrayBuilder.ofUnit"
   }
 }
-

src/library/scala/runtime/PStatics.scala

@@ -15,5 +15,7 @@ package scala.runtime
 // things that should be in `Statics`, but can't be yet for bincompat reasons
 // TODO 3.T: move to `Statics`
 private[scala] object PStatics {
-  final val VM_MaxArraySize:Int = 2147483639 // `Int.MaxValue - 8` traditional soft limit to maximize compatibility with diverse JVMs.
+  // `Int.MaxValue - 8` traditional soft limit to maximize compatibility with diverse JVMs
+  // See https://stackoverflow.com/a/8381338 for example
+  final val VM_MaxArraySize = 2147483639
 }

test/junit/scala/collection/mutable/ArrayBufferTest.scala

@@ -1,10 +1,11 @@
 package scala.collection.mutable
 
-import org.junit.Test
 import org.junit.Assert.{assertEquals, assertTrue}
+import org.junit.Test
 
 import java.lang.reflect.InvocationTargetException
 import scala.annotation.nowarn
+import scala.runtime.PStatics.VM_MaxArraySize
 import scala.tools.testkit.AssertUtil.{assertSameElements, assertThrows, fail}
 import scala.tools.testkit.ReflectUtil.{getMethodAccessible, _}
 import scala.util.chaining._
@@ -390,25 +391,26 @@ class ArrayBufferTest {
     def resizeUp(arrayLen: Int, targetLen: Int): Int = sut.invoke(ArrayBuffer, arrayLen, targetLen).asInstanceOf[Int]
 
     // check termination and correctness
-    assertTrue(7 < ArrayBuffer.DefaultInitialSize)  // condition of test
+    assertTrue(7 < ArrayBuffer.DefaultInitialSize) // condition of test
     assertEquals(ArrayBuffer.DefaultInitialSize, resizeUp(7, 10))
-    assertEquals(Int.MaxValue - 8, resizeUp(Int.MaxValue / 2, Int.MaxValue / 2 + 1))  // was: ok
-    assertEquals(Int.MaxValue - 8, resizeUp(Int.MaxValue / 2, Int.MaxValue / 2 + 2))  // was: ok
-    assertEquals(-1, resizeUp(Int.MaxValue / 2 + 1, Int.MaxValue / 2 + 1))  // was: wrong
-    assertEquals(Int.MaxValue - 8, resizeUp(Int.MaxValue / 2 + 1, Int.MaxValue / 2 + 2))  // was: hang
-    assertEquals(Int.MaxValue - 8, resizeUp(Int.MaxValue / 2, Int.MaxValue - 8))
+    assertEquals(VM_MaxArraySize, resizeUp(Int.MaxValue / 2, Int.MaxValue / 2 + 1)) // `MaxValue / 2` cannot be doubled
+    assertEquals(VM_MaxArraySize / 2 * 2, resizeUp(VM_MaxArraySize / 2, VM_MaxArraySize / 2 + 1)) // `VM_MaxArraySize / 2` can be doubled
+    assertEquals(VM_MaxArraySize, resizeUp(Int.MaxValue / 2, Int.MaxValue / 2 + 2))
+    assertEquals(-1, resizeUp(Int.MaxValue / 2 + 1, Int.MaxValue / 2 + 1)) // no resize needed
+    assertEquals(VM_MaxArraySize, resizeUp(Int.MaxValue / 2 + 1, Int.MaxValue / 2 + 2))
+    assertEquals(VM_MaxArraySize, resizeUp(Int.MaxValue / 2, VM_MaxArraySize))
+    assertEquals(123456*2+33, resizeUp(123456, 123456*2+33)) // use targetLen if it's larger than double the current
 
     // check limits
     def rethrow(op: => Any): Unit =
       try op catch { case e: InvocationTargetException => throw e.getCause }
     def checkExceedsMaxInt(targetLen: Int): Unit = {
       assertThrows[Exception](rethrow(resizeUp(0, targetLen)),
-                              _ == s"Size of array-backed collection must exceed 0.  Encountered size: $targetLen")
+                              _ == s"Overflow while resizing array of array-backed collection. Requested length: $targetLen; current length: 0; increase: $targetLen")
     }
-    import scala.runtime.PStatics
     def checkExceedsVMArrayLimit(targetLen: Int): Unit =
       assertThrows[Exception](rethrow(resizeUp(0, targetLen)),
-                              _ == s"Size of array-backed collection exceeds VM array size limit of ${PStatics.VM_MaxArraySize}.  Encountered size: $targetLen")
+                              _ == s"Array of array-backed collection exceeds VM length limit of $VM_MaxArraySize. Requested length: $targetLen; current length: 0")
 
     checkExceedsMaxInt(Int.MaxValue + 1)
     checkExceedsVMArrayLimit(Int.MaxValue)

test/junit/scala/collection/mutable/ArrayBuilderTest.scala

@@ -1,40 +1,35 @@
 package scala.collection.mutable
 
-import org.junit.Test
 import org.junit.Assert.assertEquals
+import org.junit.Test
+
+import scala.runtime.PStatics.VM_MaxArraySize
 import scala.tools.testkit.AssertUtil.assertThrows
 
-import scala.runtime.PStatics
 class ArrayBuilderTest {
-
-  /* Test for scala/bug#12617 */
-
-  val MaxArraySize: Int = PStatics.VM_MaxArraySize
-
   @Test
-  def testCapacityGrowthLimit: Unit = {
-
+  def t12617: Unit = {
     val ab: ArrayBuilder[Unit] = ArrayBuilder.make[Unit]
 
-    var i: Int = 0
-    while (i < MaxArraySize) {
-      ab.addOne(())
-      i += 1
-    }
+    // ArrayBuilder.ofUnit.addAll doesn't iterate if the iterator has a `knownSize`
+    ab.addAll(new Iterator[Unit] {
+      override def knownSize: Int = VM_MaxArraySize
+      def hasNext: Boolean = true
+      def next(): Unit = ()
+    })
 
     // reached maximum size without entering an infinite loop?
-    assertEquals(ab.length, MaxArraySize)
+    assertEquals(ab.length, VM_MaxArraySize)
 
     // expect an exception when trying to grow larger than maximum size by addOne
-    assertThrows[Exception](ab.addOne(()))
+    assertThrows[Exception](ab.addOne(()), _.endsWith("Requested length: 2147483640; current length: 2147483639"))
 
-    val arr:Array[Unit] = Array[Unit]((), (), (), (), (), (), (), (), (), (), (), ())
+    val arr = Array[Unit]((), (), (), (), (), (), (), (), (), (), (), ())
 
     // expect an exception when trying to grow larger than maximum size by addAll(iterator)
-    assertThrows[Exception](ab.addAll(arr.iterator))
+    assertThrows[Exception](ab.addAll(arr.iterator), _.endsWith("Requested length: -2147483645; current length: 2147483639; increase: 12"))
 
     // expect an exception when trying to grow larger than maximum size by addAll(array)
     assertThrows[Exception](ab.addAll(arr))
-
   }
 }