compactUntagged class cast exception

[0minus273]

Bonjour!

Reproduction:

Chunk.Queue.singleton(Chunk.singleton(1)).traverse(x => Option(x))

Result: java.lang.ClassCastException: class fs2.Chunk$Singleton cannot be cast to class fs2.Chunk$ArraySlice

First was discovered when using evalMapChunk.

Introduced here in v3.1.3 with compactUntagged implementation.

[Alexey-Yuferov]

is any reason to split this logic onto compact and compactUntagged ?
it brings runtime reflection in compact and undefined behaviour for compactUntagged (two casts over Any)
if we know that O2 :> O why not

case class ArraySlice[O](values: Array[O], offset: Int, length: Int) extends Chunk[O] {
...
   override def compact[O2 >: O]: ArraySlice[O2] =  this.asInstanceOf[ArraySlice[O2]]
...
   def copyToArray[O2 >: O](xs: Array[O2], start: Int): Unit = System.arraycopy(values, offset, xs, start, length)
...
}

or is any reason to change value type on compaction at all?

[mpilquist]

@Alexey-Yuferov compact needs a ClassTag[O2] -- in contexts where you don't have a class tag and don't mind boxed arrays, compactUntagged is useful (e.g. in the implementation of traverse). See #2680 for the fix.

[Alexey-Yuferov]

my point was "is compact really needs ClassTag[O2] and ArraySlice ClassTag[O] if we can avoid it like in compactUntagged"
just copy paste from fix:

def compactUntagged[O2 >: O]: Chunk.ArraySlice[O2] = {
   Chunk.ArraySlice(toArray[Any], 0, size).asInstanceOf[Chunk.ArraySlice[O2]]
}

why can't it be default implementation for performance without fallback to iterative arrays fill?

[mpilquist]

Then you'll never get a primitive array and it will always box.

[Alexey-Yuferov]

Array is a foreign entity for ArraySlice, one can create ArraySlice of primitives:

val intArray = Array[Int]
val chunk = Chunk.ArraySlice(intArray)

and one does not need ClassTag in this case
and there is only one(two actually) option for O2
i don't think that case chunk.compact[AnyVal] is very frequent
but i am afraid that with this fix my code will copy paste huge amount of data without reason

[mpilquist]

In that example, you need a class tag to construct the array. Once you've constructed an array, you can wrap it in an ArraySlice without a class tag.

The PR which addresses this issue (#2680) doesn't introduce any new copying.

[Alexey-Yuferov]

but this one does #2680
evalMapChunk => chunk.traverse => compactBoxed => copy to mutable.Buffer

[mpilquist]

Okay I see, you're concerned about an extra copy when calling traverse on an ArraySlice? That's likely addressable by overriding compactBoxed in ArraySlice and returning this.

[mpilquist]

@Alexey-Yuferov Please review latest version of #2680

[Alexey-Yuferov]

@mpilquist it looks good for me