Skip to content

Commit

Permalink
Merge pull request #8194 from joshlemer/issue/11600
Browse files Browse the repository at this point in the history
[#11600] Vector.from(ArraySeq) copies elems rather than reusing unsafeArray
  • Loading branch information
szeiger committed Sep 11, 2019
2 parents d602ff4 + 23e75f5 commit 64a5a5d
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 12 deletions.
30 changes: 18 additions & 12 deletions src/library/scala/collection/immutable/Vector.scala
Expand Up @@ -30,11 +30,23 @@ object Vector extends StrictOptimizedSeqFactory[Vector] {

def from[E](it: collection.IterableOnce[E]): Vector[E] =
it match {
case as: ArraySeq[E] if as.length <= 32 && as.unsafeArray.isInstanceOf[Array[AnyRef]] =>
case as: ArraySeq[E] if as.length <= 32 =>
if (as.isEmpty) NIL
else {
val v = new Vector(0, as.length, 0)
v.display0 = as.unsafeArray.asInstanceOf[Array[AnyRef]]
val unsafeArray = as.unsafeArray
val len = unsafeArray.length
val v = new Vector(0, len, 0)
val display0 = new Array[Any](len)
if (unsafeArray.isInstanceOf[Array[AnyRef]]) {
System.arraycopy(unsafeArray, 0, display0, 0, len)
} else {
var i = 0
while (i < len) {
display0(i) = unsafeArray(i)
i += 1
}
}
v.display0 = display0.asInstanceOf[Array[AnyRef]]
v.depth = 1
releaseFence()
v
Expand All @@ -45,17 +57,11 @@ object Vector extends StrictOptimizedSeqFactory[Vector] {

if (knownSize == 0) empty[E]
else if (knownSize > 0 && knownSize <= 32) {
val display0 = new Array[AnyRef](knownSize)

var i = 0
val iterator = it.iterator
while (iterator.hasNext) {
display0(i) = iterator.next().asInstanceOf[AnyRef]
i += 1
}
val display0 = new Array[Any](knownSize)
it.iterator.copyToArray(display0)
val v = new Vector[E](0, knownSize, 0)
v.depth = 1
v.display0 = display0
v.display0 = display0.asInstanceOf[Array[AnyRef]]
releaseFence()
v
} else {
Expand Down
44 changes: 44 additions & 0 deletions test/junit/scala/collection/immutable/VectorTest.scala
Expand Up @@ -219,4 +219,48 @@ class VectorTest {
assertArrayEquals(s"<${v2.length}>.slice($j, $k)", v2.toArray.slice(j, k), v2.iterator.slice(j, k).toArray)
}
}

@Test
def t11600(): Unit = {
locally {
abstract class Base
class Derived1 extends Base
class Derived2 extends Base
val d1 = new Derived1
val d2 = new Derived2

locally {
val arraySeq = ArraySeq(d1)
val vector = Vector(arraySeq: _*)
assertEquals(arraySeq, ArraySeq(d1)) // ensure arraySeq is not mutated
assertEquals(vector.updated(0, d2), Vector(d2))
}

locally {
val list = List(d1)
val vector = Vector.from(list)
assertEquals(list, vector)
assertEquals(List(d2), vector.updated(0, d2))
}
}

locally {
// ensure boxing logic works:
val arraySeq = ArraySeq(1,2,3,4,5)
val vector = Vector(arraySeq: _*)

assertEquals(1 to 5, vector)
assertEquals(vector.updated(0, 20), Vector(20,2,3,4,5))
assertEquals(vector.updated(0, ""), Vector("",2,3,4,5))
assertEquals(1 to 5, arraySeq) // ensure arraySeq is not mutated
}
locally {
// ensure boxing logic works:
val arr = Array(1)
val vector = Vector.from(arr)
assertEquals(arr.toList, vector)
assertEquals(List(20), vector.updated(0, 20))
assertEquals(List(""), vector.updated(0, ""))
}
}
}

0 comments on commit 64a5a5d

Please sign in to comment.