Merge pull request scala/scala#6926 from joshlemer/bug_8469

julienrf · web-flow · commit e1028058047d · 2018-08-06T10:37:11.000+02:00
IterableOnce#copyToArray returns number of elements copied. Fixes scala/bug#8469 scala/bug#11048
diff --git a/library/src/scala/collection/ArrayOps.scala b/library/src/scala/collection/ArrayOps.scala
@@ -1263,13 +1263,20 @@ final class ArrayOps[A](val xs: Array[A]) extends AnyVal {
     *  @param  len    the maximal number of elements to copy.
     *  @tparam B      the type of the elements of the array.
     */
-  def copyToArray[B >: A](dest: Array[B], start: Int, len: Int = Int.MaxValue): dest.type = {
-    Array.copy(xs, 0, dest, start, min(xs.length, min(dest.length-start, len)))
-    dest
+  def copyToArray[B >: A](dest: Array[B], start: Int, len: Int = Int.MaxValue): Int = {
+    val copied = IterableOnce.elemsToCopyToArray(xs.length, dest.length, start, len)
+    if (copied > 0) {
+      Array.copy(xs, 0, dest, start, copied)
+    }
+    copied
   }
 
   /** Create a copy of this array with the specified element type. */
-  def toArray[B >: A: ClassTag]: Array[B] = copyToArray(new Array[B](xs.length), 0)
+  def toArray[B >: A: ClassTag]: Array[B] = {
+    val destination = new Array[B](xs.length)
+    copyToArray(destination, 0)
+    destination
+  }
 
   /** Counts the number of elements in this array which satisfy a predicate */
   def count(p: A => Boolean): Int = {
diff --git a/library/src/scala/collection/IterableOnce.scala b/library/src/scala/collection/IterableOnce.scala
@@ -215,6 +215,17 @@ final class IterableOnceExtensionMethods[A](private val it: IterableOnce[A]) ext
 object IterableOnce {
   @`inline` implicit def iterableOnceExtensionMethods[A](it: IterableOnce[A]): IterableOnceExtensionMethods[A] =
     new IterableOnceExtensionMethods[A](it)
+
+  /** Computes the number of elements to copy to an array from a source IterableOnce
+    *
+    * @param srcLen the length of the source collection
+    * @param destLen the length of the destination array
+    * @param start the index in the destination array at which to start copying elements to
+    * @param len the requested number of elements to copy (we may only be able to copy less than this)
+    * @return the number of elements that will be copied to the destination array
+    */
+  @inline private[collection] def elemsToCopyToArray(srcLen: Int, destLen: Int, start: Int, len: Int): Int =
+    math.max(math.min(math.min(len, srcLen), destLen - start), 0)
 }
 
 /** This implementation trait can be mixed into an `IterableOnce` to get the basic methods that are shared between
@@ -702,7 +713,7 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] =>
   @deprecated("Use `dest ++= coll` instead", "2.13.0")
   @inline final def copyToBuffer[B >: A](dest: mutable.Buffer[B]): Unit = dest ++= this
 
-  /** Copy elements to an array.
+  /** Copy elements to an array, returning the number of elements written.
    *
    *  Fills the given array `xs` starting at index `start` with values of this $coll.
    *
@@ -711,14 +722,15 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] =>
    *
    *  @param  xs     the array to fill.
    *  @tparam B      the type of the elements of the array.
+   *  @return        the number of elements written to the array
    *
-   *  @usecase def copyToArray(xs: Array[A]): Unit
+   *  @usecase def copyToArray(xs: Array[A]): Int
    *
    *  $willNotTerminateInf
    */
-  def copyToArray[B >: A](xs: Array[B]): xs.type = copyToArray(xs, 0)
+  def copyToArray[B >: A](xs: Array[B]): Int = copyToArray(xs, 0)
 
-  /** Copy elements to an array.
+  /** Copy elements to an array, returning the number of elements written.
     *
     *  Fills the given array `xs` starting at index `start` with values of this $coll.
     *
@@ -728,24 +740,25 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] =>
     *  @param  xs     the array to fill.
     *  @param  start  the starting index of xs.
     *  @tparam B      the type of the elements of the array.
+    *  @return        the number of elements written to the array
     *
-    *  @usecase def copyToArray(xs: Array[A], start: Int): Unit
+    *  @usecase def copyToArray(xs: Array[A], start: Int): Int
     *
     *  $willNotTerminateInf
     */
 
-  def copyToArray[B >: A](xs: Array[B], start: Int): xs.type = {
+  def copyToArray[B >: A](xs: Array[B], start: Int): Int = {
     val xsLen = xs.length
     val it = iterator
     var i = start
     while (i < xsLen && it.hasNext) {
       xs(i) = it.next()
       i += 1
     }
-    xs
+    i - start
   }
 
-  /** Copy elements to an array.
+  /** Copy elements to an array, returning the number of elements written.
     *
     *  Fills the given array `xs` starting at index `start` with at most `len` elements of this $coll.
     *
@@ -756,22 +769,23 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] =>
     *  @param  start  the starting index of xs.
     *  @param  len    the maximal number of elements to copy.
     *  @tparam B      the type of the elements of the array.
+    *  @return        the number of elements written to the array
     *
     *  @note    Reuse: $consumesIterator
     *
-    *  @usecase def copyToArray(xs: Array[A], start: Int, len: Int): Unit
+    *  @usecase def copyToArray(xs: Array[A], start: Int, len: Int): Int
     *
     *    $willNotTerminateInf
     */
-  def copyToArray[B >: A](xs: Array[B], start: Int, len: Int): xs.type = {
+  def copyToArray[B >: A](xs: Array[B], start: Int, len: Int): Int = {
     val it = iterator
     var i = start
     val end = start + math.min(len, xs.length - start)
     while (i < end && it.hasNext) {
       xs(i) = it.next()
       i += 1
     }
-    xs
+    i - start
   }
 
   /** Sums up the elements of this collection.
@@ -1204,7 +1218,11 @@ trait IterableOnceOps[+A, +CC[_], +C] extends Any { this: IterableOnce[A] =>
 
   /** Convert collection to array. */
   def toArray[B >: A: ClassTag]: Array[B] =
-    if (knownSize >= 0) copyToArray(new Array[B](knownSize), 0)
+    if (knownSize >= 0) {
+      val destination = new Array[B](knownSize)
+      copyToArray(destination, 0)
+      destination
+    }
     else mutable.ArrayBuffer.from(this).toArray[B]
 
   // For internal use
diff --git a/library/src/scala/collection/StringOps.scala b/library/src/scala/collection/StringOps.scala
@@ -1141,16 +1141,14 @@ final class StringOps(private val s: String) extends AnyVal {
   @`inline` def filterNot(pred: Char => Boolean): String = filter(c => !pred(c))
 
   /** Copy chars of this string to an array.
-    * Fills the given array `xs` starting at index `start` with at most `len` chars.
-    * Copying will stop once either the entire string has been copied,
-    * or the end of the array is reached or `len` chars have been copied.
+    * Fills the given array `xs` starting at index 0.
+    * Copying will stop once either the entire string has been copied
+    * or the end of the array is reached
     *
     *  @param  xs     the array to fill.
-    *  @param  start  the starting index.
-    *  @param  len    the maximal number of elements to copy.
     */
-  def copyToArray(xs: Array[Char], start: Int, len: Int): Unit =
-    s.getChars(0, min(min(s.length, len), xs.length-start), xs, start)
+  @`inline` def copyToArray(xs: Array[Char]): Int =
+    copyToArray(xs, 0, Int.MaxValue)
 
   /** Copy chars of this string to an array.
     * Fills the given array `xs` starting at index `start`.
@@ -1159,11 +1157,27 @@ final class StringOps(private val s: String) extends AnyVal {
     *
     *  @param  xs     the array to fill.
     *  @param  start  the starting index.
-    *  @param  len    the maximal number of elements to copy.
     */
-  @`inline` def copyToArray(xs: Array[Char], start: Int): Unit =
+  @`inline` def copyToArray(xs: Array[Char], start: Int): Int =
     copyToArray(xs, start, Int.MaxValue)
 
+  /** Copy chars of this string to an array.
+    * Fills the given array `xs` starting at index `start` with at most `len` chars.
+    * Copying will stop once either the entire string has been copied,
+    * or the end of the array is reached or `len` chars have been copied.
+    *
+    *  @param  xs     the array to fill.
+    *  @param  start  the starting index.
+    *  @param  len    the maximal number of elements to copy.
+    */
+  def copyToArray(xs: Array[Char], start: Int, len: Int): Int = {
+    val copied = IterableOnce.elemsToCopyToArray(s.length, xs.length, start, len)
+    if (copied > 0) {
+      s.getChars(0, copied, xs, start)
+    }
+    copied
+  }
+
   /** Finds index of the first char satisfying some predicate after or at some start index.
     *
     *  @param   p     the predicate used to test elements.
diff --git a/library/src/scala/collection/immutable/ArraySeq.scala b/library/src/scala/collection/immutable/ArraySeq.scala
@@ -5,6 +5,7 @@ import java.io.{ObjectInputStream, ObjectOutputStream}
 
 import scala.collection.mutable.{ArrayBuffer, ArrayBuilder, Builder, ArraySeq => MutableArraySeq}
 import scala.collection.{ArrayOps, ClassTagSeqFactory, SeqFactory, StrictOptimizedClassTagSeqFactory, View}
+import scala.collection.IterableOnce
 import scala.annotation.unchecked.uncheckedVariance
 import scala.util.hashing.MurmurHash3
 import scala.reflect.ClassTag
@@ -133,12 +134,14 @@ sealed abstract class ArraySeq[+A]
 
   override protected[this] def className = "ArraySeq"
 
-  override def copyToArray[B >: A](xs: Array[B], start: Int = 0): xs.type = copyToArray[B](xs, start, length)
+  override def copyToArray[B >: A](xs: Array[B], start: Int = 0): Int = copyToArray[B](xs, start, length)
 
-  override def copyToArray[B >: A](xs: Array[B], start: Int, len: Int): xs.type = {
-    val l = scala.math.min(scala.math.min(len, length), xs.length-start)
-    if(l > 0) Array.copy(unsafeArray, 0, xs, start, l)
-    xs
+  override def copyToArray[B >: A](xs: Array[B], start: Int, len: Int): Int = {
+    val copied = IterableOnce.elemsToCopyToArray(length, xs.length, start, len)
+    if(copied > 0) {
+      Array.copy(unsafeArray, 0, xs, start, copied)
+    }
+    copied
   }
 
   override protected[this] def writeReplace(): AnyRef = this
diff --git a/library/src/scala/collection/mutable/ArrayBuffer.scala b/library/src/scala/collection/mutable/ArrayBuffer.scala
@@ -2,9 +2,6 @@ package scala
 package collection
 package mutable
 
-import java.lang.{IndexOutOfBoundsException, IllegalArgumentException}
-
-
 /** An implementation of the `Buffer` class using an array to
   *  represent the assembled sequence internally. Append, update and random
   *  access take constant time (amortized time). Prepends and removes are
@@ -159,12 +156,14 @@ class ArrayBuffer[A] private (initialElements: Array[AnyRef], initialSize: Int)
 
   override protected[this] def stringPrefix = "ArrayBuffer"
 
-  override def copyToArray[B >: A](xs: Array[B], start: Int): xs.type = copyToArray[B](xs, start, length)
+  override def copyToArray[B >: A](xs: Array[B], start: Int): Int = copyToArray[B](xs, start, length)
 
-  override def copyToArray[B >: A](xs: Array[B], start: Int, len: Int): xs.type = {
-    val l = scala.math.min(scala.math.min(len, length), xs.length-start)
-    if(l > 0) Array.copy(array, 0, xs, start, l)
-    xs
+  override def copyToArray[B >: A](xs: Array[B], start: Int, len: Int): Int = {
+    val copied = IterableOnce.elemsToCopyToArray(length, xs.length, start, len)
+    if(copied > 0) {
+      Array.copy(array, 0, xs, start, copied)
+    }
+    copied
   }
 
   /** Sorts this $coll in place according to an Ordering.
diff --git a/library/src/scala/collection/mutable/ArrayDeque.scala b/library/src/scala/collection/mutable/ArrayDeque.scala
@@ -434,8 +434,13 @@ class ArrayDeque[A] protected (
 
   override def grouped(n: Int): Iterator[IterableCC[A]] = sliding(n, n)
 
-  override def copyToArray[B >: A](dest: Array[B], destStart: Int, len: Int): dest.type =
-    copySliceToArray(srcStart = 0, dest = dest, destStart = destStart, maxItems = len)
+  override def copyToArray[B >: A](dest: Array[B], destStart: Int, len: Int): Int = {
+    val copied = IterableOnce.elemsToCopyToArray(length, dest.length, destStart, len)
+    if (copied > 0) {
+      copySliceToArray(srcStart = 0, dest = dest, destStart = destStart, maxItems = len)
+    }
+    copied
+  }
 
   override def toArray[B >: A: ClassTag]: Array[B] =
     copySliceToArray(srcStart = 0, dest = new Array[B](length), destStart = 0, maxItems = length)
diff --git a/library/src/scala/collection/mutable/ArraySeq.scala b/library/src/scala/collection/mutable/ArraySeq.scala
@@ -59,12 +59,14 @@ sealed abstract class ArraySeq[T]
   /** Clones this object, including the underlying Array. */
   override def clone(): ArraySeq[T] = ArraySeq.make(array.clone()).asInstanceOf[ArraySeq[T]]
 
-  override def copyToArray[B >: T](xs: Array[B], start: Int): xs.type = copyToArray[B](xs, start, length)
+  override def copyToArray[B >: T](xs: Array[B], start: Int): Int = copyToArray[B](xs, start, length)
 
-  override def copyToArray[B >: T](xs: Array[B], start: Int, len: Int): xs.type = {
-    val l = scala.math.min(scala.math.min(len, length), xs.length-start)
-    if(l > 0) Array.copy(array, 0, xs, start, l)
-    xs
+  override def copyToArray[B >: T](xs: Array[B], start: Int, len: Int): Int = {
+    val copied = IterableOnce.elemsToCopyToArray(length, xs.length, start, len)
+    if(copied > 0) {
+      Array.copy(array, 0, xs, start, copied)
+    }
+    copied
   }
 
   override protected[this] def writeReplace(): AnyRef = this
