It seems that varargs.toArray returns the array that’s backing the varargs directly. Because of that modification to that array are visible as modifications to varargs themselves. Is that intentional or a bug? I’ve stumbled upon that today.
Following code:
object ArrayReuse {
def main(args: Array[String]): Unit = {
val test = new Test(7, 8, 9)
test.printArgs()
test.modifyArray()
test.printArgs()
}
class Test(values: Int*) {
def modifyArray(): Unit = {
val array = values.toArray
array(1) = 5
}
def printArgs(): Unit =
println(values)
}
}
Varargs is aan array to be compatible with Java varargs. That’s not the array backing the varargs, that’s the actual varargs. Array.toArray returns itself. This can trip you up but is as specified.
Where is it specified? I just added the same code back to https://github.com/scala/collection-strawman/pull/278/ but I think we should consider changing this behavior. As far as I can tell the spec doesn’t say anything about what kind of Seq backs varargs and what its toArray method should do. Returning the underlying array in WrappedArray.toArray is unexpected and inconcistent with ArrayBuffer.toArray.
I stand corrected, I can’t read it back in the spec, which just says it’s a collection.Seq. For java interop with varargs, there must be a version in bytecode that takes an array, but that doesn’t mean that can’t be wrapped (and indeed, it is)
Whether toArray should return a copy or not could be debatable. if Array.toArray should return itself (it does; should it?), and WrappedArray.toArray should return a copy, that means that if you have a Seq, you don’t know whether mutations to the array returned by toArray are going to mutate the original data.
if I already don’t know if
def printargs(args: Seq[Int]): Int = {
if(args.isEmpty) 1
else {
val argsarray = args.toArray
argsarray(0) = 1
args(0)
}
}
will ever return anything other than 1
why should I be able to assume
def printargs(args: Int*): Int = {
if(args.isEmpty) 1
else {
val argsarray = args.toArray
argsarray(0) = 1
args(0)
}
}
On second thought, this situation is already changing with my recent array-related additions. While WrappedArray still unwraps the original array on toArray, it is no longer used for varargs. They now pretend that the array is immutable and wrap it as an ImmutableArray which does copy on toArray. This is required because we want to make the default scala.Seq a scala.collection.immutable.Seq. The reason why it was not immutable by default (unlike Set and Map) is that it is required for varargs and they used WrappedArray.