Merge pull request #10642 from som-snytt/followup/array-builder

Revert maladapted ArrayBuffer builder [ci: last-only]

src/compiler/scala/tools/nsc/transform/patmat/Logic.scala

@@ -529,7 +529,7 @@ object Logic {
     with IterableFactoryDefaults[A, LogicLinkedHashSet] {
     override def iterableFactory: IterableFactory[LogicLinkedHashSet] = LogicLinkedHashSet
     override def addAll(xs: IterableOnce[A]): this.type = {
-      sizeHint(xs.knownSize)
+      sizeHint(xs)
       super.addAll(xs)
     }
   }

src/library/scala/collection/ArrayOps.scala

@@ -1174,8 +1174,7 @@ final class ArrayOps[A](private val xs: Array[A]) extends AnyVal {
   /** A copy of this array with all elements of a collection appended. */
   def appendedAll[B >: A : ClassTag](suffix: IterableOnce[B]): Array[B] = {
     val b = ArrayBuilder.make[B]
-    val k = suffix.knownSize
-    if(k >= 0) b.sizeHint(k + xs.length)
+    b.sizeHint(suffix, delta = xs.length)
     b.addAll(xs)
     b.addAll(suffix)
     b.result()

src/library/scala/collection/Factory.scala

@@ -61,7 +61,7 @@ object Factory {
   private class ArrayFactory[A: ClassTag] extends Factory[A, Array[A]] with Serializable {
     def fromSpecific(it: IterableOnce[A]): Array[A] = {
       val b = newBuilder
-      b.sizeHint(scala.math.max(0, it.knownSize))
+      b.sizeHint(it, delta = 0)
       b ++= it
       b.result()
     }

src/library/scala/collection/StrictOptimizedSeqOps.scala

@@ -34,19 +34,15 @@ trait StrictOptimizedSeqOps [+A, +CC[_], +C]
 
   override def prepended[B >: A](elem: B): CC[B] = {
     val b = iterableFactory.newBuilder[B]
-    if (knownSize >= 0) {
-      b.sizeHint(size + 1)
-    }
+    b.sizeHint(this, delta = 1)
     b += elem
     b ++= this
     b.result()
   }
 
   override def appended[B >: A](elem: B): CC[B] = {
     val b = iterableFactory.newBuilder[B]
-    if (knownSize >= 0) {
-      b.sizeHint(size + 1)
-    }
+    b.sizeHint(this, delta = 1)
     b ++= this
     b += elem
     b.result()

src/library/scala/collection/immutable/StrictOptimizedSeqOps.scala

@@ -14,9 +14,8 @@ package scala
 package collection
 package immutable
 
-/**
-  * Trait that overrides operations to take advantage of strict builders.
-  */
+/** Trait that overrides operations to take advantage of strict builders.
+ */
 trait StrictOptimizedSeqOps[+A, +CC[_], +C]
   extends Any
     with SeqOps[A, CC, C]
@@ -41,9 +40,7 @@ trait StrictOptimizedSeqOps[+A, +CC[_], +C]
   override def updated[B >: A](index: Int, elem: B): CC[B] = {
     if (index < 0) throw new IndexOutOfBoundsException(s"$index is out of bounds (min 0, max ${if (knownSize>=0) knownSize else "unknown"})")
     val b = iterableFactory.newBuilder[B]
-    if (knownSize >= 0) {
-      b.sizeHint(size)
-    }
+    b.sizeHint(this)
     var i = 0
     val it = iterator
     while (i < index && it.hasNext) {

src/library/scala/collection/immutable/WrappedString.scala

@@ -125,8 +125,7 @@ final class WrappedString(private val self: String) extends AbstractSeq[Char] wi
 object WrappedString extends SpecificIterableFactory[Char, WrappedString] {
   def fromSpecific(it: IterableOnce[Char]): WrappedString = {
     val b = newBuilder
-    val s = it.knownSize
-    if(s >= 0) b.sizeHint(s)
+    b.sizeHint(it)
     b ++= it
     b.result()
   }

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

@@ -299,19 +299,12 @@ object ArrayBuffer extends StrictOptimizedSeqFactory[ArrayBuffer] {
     else new ArrayBuffer[B] ++= coll
   }
 
-  def newBuilder[A]: Builder[A, ArrayBuffer[A]] =
-    new GrowableBuilder[A, ArrayBuffer[A]](empty) {
-      override def sizeHint(size: Int): Unit = elems.ensureSize(size)
-    }
+  def newBuilder[A]: Builder[A, ArrayBuffer[A]] = new GrowableBuilder[A, ArrayBuffer[A]](empty[A]) {
+    override def sizeHint(size: Int): Unit = elems.sizeHint(size)
+  }
 
   def empty[A]: ArrayBuffer[A] = new ArrayBuffer[A]()
 
-  @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.
    *
@@ -320,13 +313,19 @@ object ArrayBuffer extends StrictOptimizedSeqFactory[ArrayBuffer] {
    * @return
    *   - `-1` if no resizing is needed, else
    *   - `VM_MaxArraySize` if `arrayLen` is too large to be doubled, else
-   *   - `max(targetLen, arrayLen * 2, , DefaultInitialSize)`.
+   *   - `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 = {
+    def checkArrayLengthLimit(): Unit =
+      if (targetLen > VM_MaxArraySize)
+        throw new Exception(s"Array of array-backed collection exceeds VM length limit of $VM_MaxArraySize. Requested length: $targetLen; current length: $arrayLen")
+      else if (targetLen < 0)
+        throw new Exception(s"Overflow while resizing array of array-backed collection. Requested length: $targetLen; current length: $arrayLen; increase: ${targetLen - arrayLen}")
+
     if (targetLen > 0 && targetLen <= arrayLen) -1
     else {
-      checkArrayLengthLimit(targetLen, arrayLen)
+      checkArrayLengthLimit()
       if (arrayLen > VM_MaxArraySize / 2) VM_MaxArraySize
       else math.max(targetLen, math.max(arrayLen * 2, DefaultInitialSize))
     }

src/library/scala/collection/mutable/ArraySeq.scala

@@ -43,8 +43,7 @@ sealed abstract class ArraySeq[T]
 
   override protected def fromSpecific(coll: scala.collection.IterableOnce[T]): ArraySeq[T] = {
     val b = ArrayBuilder.make(elemTag).asInstanceOf[ArrayBuilder[T]]
-    val s = coll.knownSize
-    if(s > 0) b.sizeHint(s)
+    b.sizeHint(coll, delta = 0)
     b ++= coll
     ArraySeq.make(b.result())
   }

src/library/scala/collection/mutable/Builder.scala

@@ -30,31 +30,45 @@ trait Builder[-A, +To] extends Growable[A] { self =>
   /** Result collection consisting of all elements appended so far. */
   def result(): To
 
-  /** Gives a hint how many elements are expected to be added
-    *  when the next `result` is called. Some builder classes
-    *  will optimize their representation based on the hint. However,
-    *  builder implementations are still required to work correctly even if the hint is
-    *  wrong, i.e. a different number of elements is added.
-    *
-    *  @param size  the hint how many elements will be added.
-    */
+  /** Gives a hint how many elements are expected to be added in total
+   *  by the time `result` is called.
+   *
+   *  Some builder classes will optimize their representation based on the hint.
+   *  However, builder implementations are required to work correctly even if the hint is
+   *  wrong, i.e., if a different number of elements is added.
+   *
+   *  The semantics of supplying a hint out of range, such as a size that is negative
+   *  or unreasonably large, is not specified but is implementation-dependent.
+   *
+   *  The default implementation simply ignores the hint.
+   *
+   *  @param size  the hint how many elements will be added.
+   */
   def sizeHint(size: Int): Unit = ()
 
-  /** Gives a hint that one expects the `result` of this builder
-    *  to have the same size as the given collection, plus some delta. This will
-    *  provide a hint only if the collection has a known size
-    *  Some builder classes
-    *  will optimize their representation based on the hint. However,
-    *  builder implementations are still required to work correctly even if the hint is
-    *  wrong, i.e. a different number of elements is added.
-    *
-    *  @param coll  the collection which serves as a hint for the result's size.
-    *  @param delta a correction to add to the `coll.size` to produce the size hint.
-    */
-  final def sizeHint(coll: scala.collection.IterableOnce[_], delta: Int = 0): Unit = {
-    val s = coll.knownSize
-    if (s != -1) sizeHint(s + delta)
-  }
+  /** Gives a hint that the `result` of this builder is expected
+   *  to have the same size as the given collection, plus some delta.
+   *
+   *  This method provides a hint only if the collection has a known size,
+   *  as specified by the following pseudocode:
+   *
+   *  {{{
+   *    if (coll.knownSize != -1)
+   *      sizeHint(coll.knownSize + delta)
+   *  }}}
+   *
+   *  Some builder classes will optimize their representation based on the hint.
+   *  However, builder implementations are required to work correctly even if the hint is
+   *  wrong, i.e., if a different number of elements is added.
+   *
+   *  @param coll  the collection which serves as a hint for the result's size.
+   *  @param delta a correction to add to the `coll.size` to produce the size hint (zero if omitted).
+   */
+  final def sizeHint(coll: scala.collection.IterableOnce[_], delta: Int = 0): Unit =
+    coll.knownSize match {
+      case -1 =>
+      case sz => sizeHint(sz + delta)
+    }
 
   /** Gives a hint how many elements are expected to be added
     *  when the next `result` is called, together with an upper bound

src/library/scala/collection/mutable/CollisionProofHashMap.scala

@@ -174,8 +174,7 @@ final class CollisionProofHashMap[K, V](initialCapacity: Int, loadFactor: Double
   }
 
   override def addAll(xs: IterableOnce[(K, V)]): this.type = {
-    val k = xs.knownSize
-    if(k > 0) sizeHint(contentSize + k)
+    sizeHint(xs, delta = contentSize)
     super.addAll(xs)
   }
 

src/library/scala/collection/mutable/HashMap.scala

@@ -95,7 +95,7 @@ class HashMap[K, V](initialCapacity: Int, loadFactor: Double)
   }
 
   override def addAll(xs: IterableOnce[(K, V)]): this.type = {
-    sizeHint(xs.knownSize)
+    sizeHint(xs)
 
     xs match {
       case hm: immutable.HashMap[K, V] =>

src/library/scala/collection/mutable/HashSet.scala

@@ -91,7 +91,7 @@ final class HashSet[A](initialCapacity: Int, loadFactor: Double)
   }
 
   override def addAll(xs: IterableOnce[A]): this.type = {
-    sizeHint(xs.knownSize)
+    sizeHint(xs, delta = 0)
     xs match {
       case hs: immutable.HashSet[A] =>
         hs.foreachWithHash((k, h) => addElem(k, improveHash(h)))

src/library/scala/collection/mutable/LinkedHashMap.scala

@@ -480,7 +480,7 @@ object LinkedHashMap extends MapFactory[LinkedHashMap] {
 
   def from[K, V](it: collection.IterableOnce[(K, V)]) = {
     val newlhm = empty[K, V]
-    newlhm.sizeHint(it.knownSize)
+    newlhm.sizeHint(it, delta = 0)
     newlhm.addAll(it)
     newlhm
   }

src/library/scala/collection/mutable/LinkedHashSet.scala

@@ -318,7 +318,7 @@ object LinkedHashSet extends IterableFactory[LinkedHashSet] {
 
   def from[E](it: collection.IterableOnce[E]) = {
     val newlhs = empty[E]
-    newlhs.sizeHint(it.knownSize)
+    newlhs.sizeHint(it, delta = 0)
     newlhs.addAll(it)
     newlhs
   }

src/library/scala/runtime/Tuple2Zipped.scala

@@ -47,7 +47,7 @@ final class Tuple2Zipped[El1, It1 <: Iterable[El1], El2, It2 <: Iterable[El2]](p
 
   def map[B, To](f: (El1, El2) => B)(implicit bf: BuildFrom[It1, B, To]): To = {
     val b = bf.newBuilder(coll1)
-    b.sizeHint(coll1, 0)
+    b.sizeHint(coll1, delta = 0)
     val elems1 = coll1.iterator
     val elems2 = coll2.iterator
 

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

@@ -508,4 +508,15 @@ class ArrayBufferTest {
     check(_ ++= (1 to 100))
     check(_.insertAll(1, 1 to 100))
   }
+
+  @Test def `drop right when empty is empty`: Unit = {
+    assertTrue(ArrayBuffer().dropRight(1).isEmpty)
+  }
+
+  @Test def `refuse to build from an overly huge thing`: Unit = {
+    val bld = ArrayBuffer.newBuilder[String]
+    assertThrows[Exception](bld.sizeHint(Int.MaxValue), _.contains("exceeds VM length limit"))
+    bld.addOne("hello, world")
+    assertTrue(bld.result().contains("hello, world"))
+  }
 }