Skip to content

Commit

Permalink
Merge pull request #2680 from mpilquist/bugfix/2679
Browse files Browse the repository at this point in the history
Fix #2679 - bug in Chunk.compactUntagged when used with empty or singleton chunks
  • Loading branch information
mpilquist authored Oct 14, 2021
2 parents c945743 + c551fd3 commit 1d4f250
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 8 deletions.
34 changes: 26 additions & 8 deletions core/shared/src/main/scala/fs2/Chunk.scala
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,18 @@ abstract class Chunk[+O] extends Serializable with ChunkPlatform[O] with ChunkRu
/** Copies the elements of this chunk in to the specified array at the specified start index. */
def copyToArray[O2 >: O](xs: Array[O2], start: Int = 0): Unit

/** Converts this chunk to a chunk backed by a single array. */
/** Converts this chunk to a chunk backed by a single array.
*
* Alternatively, call `toIndexedChunk` to get back a chunk with guaranteed O(1) indexed lookup
* while also minimizing copying.
*/
def compact[O2 >: O](implicit ct: ClassTag[O2]): Chunk.ArraySlice[O2] =
Chunk.ArraySlice(toArray[O2], 0, size)

/** Like `compact` but does not require a `ClassTag`. Elements are boxed and stored in an `Array[Any]`. */
def compactUntagged[O2 >: O]: Chunk.ArraySlice[O2] = {
val arr = new Array[Any](size)
copyToArray(arr, 0)
Chunk.array(arr).asInstanceOf[Chunk.ArraySlice[O2]]
}
@deprecated("Unsound when used with primitives, use compactBoxed instead", "3.1.6")
def compactUntagged[O2 >: O]: Chunk.ArraySlice[O2] =
Chunk.ArraySlice(toArray[Any], 0, size).asInstanceOf[Chunk.ArraySlice[O2]]

/** Drops the first `n` elements of this chunk. */
def drop(n: Int): Chunk[O] = splitAt(n)._2
Expand Down Expand Up @@ -311,6 +313,21 @@ abstract class Chunk[+O] extends Serializable with ChunkPlatform[O] with ChunkRu
if (isEmpty) Chain.empty
else Chain.fromSeq(toList)

/** Returns a chunk with guaranteed O(1) lookup by index.
*
* Unlike `compact`, this operation does not copy any elements unless this chunk
* does not provide O(1) lookup by index -- e.g., a chunk built via 1 or more usages
* of `++`.
*/
def toIndexedChunk: Chunk[O] = this match {
case _: Chunk.Queue[_] =>
val b = collection.mutable.Buffer.newBuilder[O]
b.sizeHint(size)
foreach(o => b += o)
Chunk.buffer(b.result())
case other => other
}

/** Converts this chunk to a list. */
def toList: List[O] =
if (isEmpty) Nil
Expand Down Expand Up @@ -678,6 +695,7 @@ object Chunk
this.asInstanceOf[ArraySlice[O2]]
else super.compact

@deprecated("Unsound", "3.1.6")
override def compactUntagged[O2 >: O]: ArraySlice[O2] =
if ((classOf[Array[Any]] eq values.getClass) && offset == 0 && length == values.length)
this.asInstanceOf[ArraySlice[O2]]
Expand Down Expand Up @@ -1066,12 +1084,12 @@ object Chunk
}

override def traverse[F[_], O2](f: O => F[O2])(implicit F: Applicative[F]): F[Chunk[O2]] =
compactUntagged.traverse(f)
toIndexedChunk.traverse(f)

override def traverseFilter[F[_], O2](f: O => F[Option[O2]])(implicit
F: Applicative[F]
): F[Chunk[O2]] =
compactUntagged.traverseFilter(f)
toIndexedChunk.traverseFilter(f)
}

object Queue {
Expand Down
4 changes: 4 additions & 0 deletions core/shared/src/test/scala/fs2/ChunkSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -211,4 +211,8 @@ class ChunkSuite extends Fs2Suite {
Chunk.ArraySlice(Array[Any](0)).asInstanceOf[Chunk[Int]].toArray[Any]
Chunk.ArraySlice(Array[Any](0)).asInstanceOf[Chunk[Int]].toArray[Int]
}

test("compactUntagged - regression #2679") {
Chunk.Queue.singleton(Chunk.singleton(1)).traverse(x => Option(x))
}
}

0 comments on commit 1d4f250

Please # to comment.