Fix problems with byte string copy to array 2.13 (#28468)

* copyToArray unbroken on 2.13 #28465
* Skip discipline flags in console (driving me crazy!)
This commit is contained in:
Johan Andrén 2020-01-14 10:51:37 +01:00 committed by Patrik Nordwall
parent 856c4ff6f6
commit b5d2bcdbb0
5 changed files with 103 additions and 25 deletions

View file

@ -149,7 +149,13 @@ class ByteStringSpec extends WordSpec with Matchers with Checkers {
def likeVector(bs: ByteString)(body: IndexedSeq[Byte] => Any): Boolean = {
val vec = Vector(bs: _*)
body(bs) == body(vec)
val a = body(bs)
val b = body(vec)
val result = a == b
if (!result) {
println(s"$bs => $a != $vec => $b")
}
result
}
def likeVectors(bsA: ByteString, bsB: ByteString)(body: (IndexedSeq[Byte], IndexedSeq[Byte]) => Any): Boolean = {
@ -383,6 +389,27 @@ class ByteStringSpec extends WordSpec with Matchers with Checkers {
ByteString1.fromString("0123456789").take(3).drop(1) should ===(ByteString("12"))
ByteString1.fromString("0123456789").take(10).take(8).drop(3).take(5) should ===(ByteString("34567"))
}
"copyToArray" in {
val byteString = ByteString1(Array[Byte](1, 2, 3, 4, 5), startIndex = 1, length = 3)
def verify(f: Array[Byte] => Unit)(expected: Byte*): Unit = {
val array = Array.fill[Byte](3)(0)
f(array)
array should ===(expected.toArray)
}
verify(byteString.copyToArray(_, 0, 1))(2, 0, 0)
verify(byteString.copyToArray(_, 1, 1))(0, 2, 0)
verify(byteString.copyToArray(_, 2, 1))(0, 0, 2)
verify(byteString.copyToArray(_, 3, 1))(0, 0, 0)
verify(byteString.copyToArray(_, 0, 2))(2, 3, 0)
verify(byteString.copyToArray(_, 1, 2))(0, 2, 3)
verify(byteString.copyToArray(_, 2, 2))(0, 0, 2)
verify(byteString.copyToArray(_, 3, 2))(0, 0, 0)
verify(byteString.copyToArray(_, 0, 3))(2, 3, 4)
verify(byteString.copyToArray(_, 1, 3))(0, 2, 3)
verify(byteString.copyToArray(_, 2, 3))(0, 0, 2)
verify(byteString.copyToArray(_, 3, 3))(0, 0, 0)
}
}
"ByteString1C" must {
"drop" in {
@ -426,6 +453,27 @@ class ByteStringSpec extends WordSpec with Matchers with Checkers {
ByteString1.fromString("abcdefg").drop(2) should ===(ByteString("cdefg"))
ByteString1.fromString("abcdefg").drop(2).take(1) should ===(ByteString("c"))
}
"copyToArray" in {
val byteString = ByteString1C(Array[Byte](1, 2, 3))
def verify(f: Array[Byte] => Unit)(expected: Byte*): Unit = {
val array = Array.fill[Byte](3)(0)
f(array)
array should ===(expected.toArray)
}
verify(byteString.copyToArray(_, 0, 1))(1, 0, 0)
verify(byteString.copyToArray(_, 1, 1))(0, 1, 0)
verify(byteString.copyToArray(_, 2, 1))(0, 0, 1)
verify(byteString.copyToArray(_, 3, 1))(0, 0, 0)
verify(byteString.copyToArray(_, 0, 2))(1, 2, 0)
verify(byteString.copyToArray(_, 1, 2))(0, 1, 2)
verify(byteString.copyToArray(_, 2, 2))(0, 0, 1)
verify(byteString.copyToArray(_, 3, 2))(0, 0, 0)
verify(byteString.copyToArray(_, 0, 3))(1, 2, 3)
verify(byteString.copyToArray(_, 1, 3))(0, 1, 2)
verify(byteString.copyToArray(_, 2, 3))(0, 0, 1)
verify(byteString.copyToArray(_, 3, 3))(0, 0, 0)
}
}
"ByteStrings" must {
"drop" in {
@ -655,6 +703,28 @@ class ByteStringSpec extends WordSpec with Matchers with Checkers {
compact.indexOf('g', 5) should ===(5)
compact.indexOf('g', 6) should ===(-1)
}
"copyToArray" in {
val byteString = ByteString(1, 2) ++ ByteString(3) ++ ByteString(4)
def verify(f: Array[Byte] => Unit)(expected: Byte*): Unit = {
val array = Array.fill[Byte](3)(0)
f(array)
array should ===(expected.toArray)
}
verify(byteString.copyToArray(_, 0, 1))(1, 0, 0)
verify(byteString.copyToArray(_, 1, 1))(0, 1, 0)
verify(byteString.copyToArray(_, 2, 1))(0, 0, 1)
verify(byteString.copyToArray(_, 3, 1))(0, 0, 0)
verify(byteString.copyToArray(_, 0, 2))(1, 2, 0)
verify(byteString.copyToArray(_, 1, 2))(0, 1, 2)
verify(byteString.copyToArray(_, 2, 2))(0, 0, 1)
verify(byteString.copyToArray(_, 3, 2))(0, 0, 0)
verify(byteString.copyToArray(_, 0, 3))(1, 2, 3)
verify(byteString.copyToArray(_, 1, 3))(0, 1, 2)
verify(byteString.copyToArray(_, 2, 3))(0, 0, 1)
verify(byteString.copyToArray(_, 3, 3))(0, 0, 0)
}
}
"A ByteString" must {
@ -885,7 +955,7 @@ class ByteStringSpec extends WordSpec with Matchers with Checkers {
case (xs, from, until) =>
likeVector(xs)({ it =>
val array = new Array[Byte](xs.length)
it.slice(from, until).copyToArray(array, from, until)
it.copyToArray(array, from, until)
array.toSeq
})
}
@ -1124,6 +1194,14 @@ class ByteStringSpec extends WordSpec with Matchers with Checkers {
iterator.copyToArray(array, 4, 2)
assert(new String(array) === "123456")
}
"calling copyToArray with length passing end of destination" in {
// Pre fix len passing the end of the destination would cause never ending loop inside iterator copyToArray
val iterator = (ByteString(1, 2) ++ ByteString(3) ++ ByteString(4)).iterator
val array = Array.fill[Byte](3)(0)
iterator.copyToArray(array, 2, 2)
array.toSeq should ===(Seq(0, 0, 1))
}
}
"decode data correctly" when {

View file

@ -300,7 +300,7 @@ object ByteIterator {
final override def copyToArray[B >: Byte](xs: Array[B], start: Int, len: Int): Unit = {
var pos = start
var rest = len
while ((rest > 0) && !iterators.isEmpty) {
while ((rest > 0) && !iterators.isEmpty && pos < xs.length) {
val n = 0 max ((xs.length - pos) min current.len min rest)
current.copyToArray(xs, pos, n)
pos += n

View file

@ -309,7 +309,7 @@ object ByteIterator {
final override def copyToArray[B >: Byte](xs: Array[B], start: Int, len: Int): Int = {
var pos = start
var rest = len
while ((rest > 0) && !iterators.isEmpty) {
while ((rest > 0) && !iterators.isEmpty && pos < xs.length) {
val n = 0 max ((xs.length - pos) min current.len min rest)
current.copyToArray(xs, pos, n)
pos += n

View file

@ -253,11 +253,11 @@ object ByteString {
}
override def copyToArray[B >: Byte](dest: Array[B], start: Int, len: Int): Int = {
val copied = math.min(math.min(len, bytes.length), dest.length - start)
if (copied > 0) {
System.arraycopy(bytes, 0, dest, start, copied)
val toCopy = math.min(math.min(len, bytes.length), dest.length - start)
if (toCopy > 0) {
Array.copy(bytes, 0, dest, start, toCopy)
}
copied
toCopy
}
}
@ -395,11 +395,11 @@ object ByteString {
override def copyToArray[B >: Byte](dest: Array[B], start: Int, len: Int): Int = {
// min of the bytes available to copy, bytes there is room for in dest and the requested number of bytes
val copied = math.max(math.min(math.min(len, length), dest.length - start), 0)
if (copied > 0) {
System.arraycopy(bytes, 0, dest, start, copied)
val toCopy = math.min(math.min(len, length), dest.length - start)
if (toCopy > 0) {
Array.copy(bytes, startIndex, dest, start, toCopy)
}
copied
toCopy
}
protected def writeReplace(): AnyRef = new SerializationProxy(this)
@ -645,20 +645,19 @@ object ByteString {
}
override def copyToArray[B >: Byte](dest: Array[B], start: Int, len: Int): Int = {
if (isCompact) bytestrings.head.copyToArray(dest, start, len)
if (bytestrings.size == 1) bytestrings.head.copyToArray(dest, start, len)
else {
// min of the bytes available top copy, bytes there is room for in dest and the requested number of bytes
val copied = math.min(math.min(len, length), dest.length - start)
if (copied > 0) {
// min of the bytes available to copy, bytes there is room for in dest and the requested number of bytes
val totalToCopy = math.min(math.min(len, length), dest.length - start)
if (totalToCopy > 0) {
val bsIterator = bytestrings.iterator
var pos = 0
while (pos < copied) {
var copied = 0
while (copied < totalToCopy) {
val current = bsIterator.next()
val copied = current.copyToArray(dest, pos, len - pos)
pos += copied
copied += current.copyToArray(dest, start + copied, totalToCopy - copied)
}
}
copied
totalToCopy
}
}

View file

@ -27,8 +27,8 @@ object AkkaDisciplinePlugin extends AutoPlugin with ScalafixSupport {
val silencerVersion = "1.4.4"
Seq(
libraryDependencies ++= Seq(
compilerPlugin("com.github.ghik" %% "silencer-plugin" % silencerVersion cross CrossVersion.patch),
"com.github.ghik" %% "silencer-lib" % silencerVersion % Provided cross CrossVersion.patch))
compilerPlugin(("com.github.ghik" %% "silencer-plugin" % silencerVersion).cross(CrossVersion.patch)),
("com.github.ghik" %% "silencer-lib" % silencerVersion % Provided).cross(CrossVersion.patch)))
}
lazy val disciplineSettings =
@ -39,7 +39,6 @@ object AkkaDisciplinePlugin extends AutoPlugin with ScalafixSupport {
else Seq.empty
),
Test / scalacOptions --= testUndicipline,
Compile / console / scalacOptions --= Seq("-deprecation", "-Xfatal-warnings", "-Xlint", "-Ywarn-unused:imports"),
Compile / scalacOptions ++= (CrossVersion.partialVersion(scalaVersion.value) match {
case Some((2, 13)) =>
disciplineScalacOptions -- Set(
@ -61,7 +60,9 @@ object AkkaDisciplinePlugin extends AutoPlugin with ScalafixSupport {
// different compiler phases from the regular run), and in particular
// '-Ywarn-unused:explicits' breaks 'sbt ++2.13.0-M5 akka-actor/doc'
// https://github.com/akka/akka/issues/26119
Compile / doc / scalacOptions --= disciplineScalacOptions.toSeq :+ "-Xfatal-warnings")
Compile / doc / scalacOptions --= disciplineScalacOptions.toSeq :+ "-Xfatal-warnings",
// having discipline warnings in console is just an annoyance
Compile / console / scalacOptions --= disciplineScalacOptions.toSeq)
val testUndicipline = Seq(
"-Ywarn-dead-code", // ??? used in compile only specs