Copilot commented on code in PR #2306:
URL: https://github.com/apache/pekko/pull/2306#discussion_r2418693821
##########
actor/src/main/scala/org/apache/pekko/util/ByteString.scala:
##########
@@ -1056,6 +1056,72 @@ sealed abstract class ByteString
*/
def indexOf(elem: Byte): Int = indexOf(elem, 0)
+ override def indexOfSlice[B >: Byte](slice: scala.collection.Seq[B], from:
Int): Int = {
+ // this is only called if the first byte matches, so we can skip that check
+ def check(startPos: Int): Boolean = {
+ var i = startPos + 1
+ var j = 1
+ // let's trust the calling code has ensured that we have enough bytes in
this ByteString
+ while (j < slice.length) {
+ if (apply(i) != slice(j)) return false
+ i += 1
+ j += 1
+ }
+ j == slice.length
+ }
+ val headByte = slice.head.asInstanceOf[Byte]
+ @tailrec def rec(from: Int): Int = {
+ val startPos = indexOf(headByte, from, length - slice.length + 1)
+ if (startPos == -1) -1
+ else if (check(startPos)) startPos
+ else rec(startPos + 1)
+ }
+ if (slice.isEmpty) 0 else rec(math.max(0, from))
+ }
+
+ /**
+ * Finds index of first occurrence of some slice in this ByteString.
+ *
+ * @param slice the slice to search for.
+ * @param from the start index
+ * @return the index greater than or equal to `from` of the first element
of this
+ * ByteString that starts a slice equal (as determined by `==`)
+ * to `slice`, or `-1`, if none exists.
+ * @since 2.0.0
+ */
+ def indexOfSlice(slice: Array[Byte], from: Int): Int = {
+ // this is only called if the first byte matches, so we can skip that check
Review Comment:
The comment is misleading. The `check` function is called after finding a
potential match with `indexOf`, but it still needs to verify the first byte
match since `indexOf` only finds the position of the head byte.
```suggestion
// The check function is called after finding a potential match with
indexOf,
// but it still needs to verify the first byte match since indexOf only
finds
// the position of the head byte, not the full slice.
```
##########
actor/src/main/scala/org/apache/pekko/util/ByteString.scala:
##########
@@ -1056,6 +1056,72 @@ sealed abstract class ByteString
*/
def indexOf(elem: Byte): Int = indexOf(elem, 0)
+ override def indexOfSlice[B >: Byte](slice: scala.collection.Seq[B], from:
Int): Int = {
+ // this is only called if the first byte matches, so we can skip that check
Review Comment:
The comment is misleading. The `check` function is called after finding a
potential match with `indexOf`, but it still needs to verify the first byte
match since `indexOf` only finds the position of the head byte.
```suggestion
// check verifies the rest of the slice after a potential match for the
first byte found by indexOf;
// it assumes the first byte matches at startPos, but this must be
verified by indexOfSlice logic.
```
##########
actor/src/main/scala/org/apache/pekko/util/ByteString.scala:
##########
@@ -1056,6 +1056,72 @@ sealed abstract class ByteString
*/
def indexOf(elem: Byte): Int = indexOf(elem, 0)
+ override def indexOfSlice[B >: Byte](slice: scala.collection.Seq[B], from:
Int): Int = {
+ // this is only called if the first byte matches, so we can skip that check
+ def check(startPos: Int): Boolean = {
+ var i = startPos + 1
+ var j = 1
+ // let's trust the calling code has ensured that we have enough bytes in
this ByteString
+ while (j < slice.length) {
+ if (apply(i) != slice(j)) return false
+ i += 1
+ j += 1
+ }
+ j == slice.length
Review Comment:
This condition is redundant since the while loop ensures `j == slice.length`
when it exits normally. The function should simply return `true` at the end.
```suggestion
true
```
##########
actor/src/main/scala/org/apache/pekko/util/ByteString.scala:
##########
@@ -1056,6 +1056,72 @@ sealed abstract class ByteString
*/
def indexOf(elem: Byte): Int = indexOf(elem, 0)
+ override def indexOfSlice[B >: Byte](slice: scala.collection.Seq[B], from:
Int): Int = {
+ // this is only called if the first byte matches, so we can skip that check
+ def check(startPos: Int): Boolean = {
+ var i = startPos + 1
+ var j = 1
+ // let's trust the calling code has ensured that we have enough bytes in
this ByteString
Review Comment:
These comments suggest unsafe assumptions. The code should validate bounds
or the comments should clarify that the bounds are guaranteed by the `indexOf`
call limiting the search range.
```suggestion
// Bounds are guaranteed by the indexOf call limiting the search
range, so startPos + slice.length <= length.
```
##########
actor/src/main/scala/org/apache/pekko/util/ByteString.scala:
##########
@@ -1056,6 +1056,72 @@ sealed abstract class ByteString
*/
def indexOf(elem: Byte): Int = indexOf(elem, 0)
+ override def indexOfSlice[B >: Byte](slice: scala.collection.Seq[B], from:
Int): Int = {
+ // this is only called if the first byte matches, so we can skip that check
+ def check(startPos: Int): Boolean = {
+ var i = startPos + 1
+ var j = 1
+ // let's trust the calling code has ensured that we have enough bytes in
this ByteString
+ while (j < slice.length) {
+ if (apply(i) != slice(j)) return false
+ i += 1
+ j += 1
+ }
+ j == slice.length
+ }
+ val headByte = slice.head.asInstanceOf[Byte]
+ @tailrec def rec(from: Int): Int = {
+ val startPos = indexOf(headByte, from, length - slice.length + 1)
+ if (startPos == -1) -1
+ else if (check(startPos)) startPos
+ else rec(startPos + 1)
+ }
+ if (slice.isEmpty) 0 else rec(math.max(0, from))
+ }
+
+ /**
+ * Finds index of first occurrence of some slice in this ByteString.
+ *
+ * @param slice the slice to search for.
+ * @param from the start index
+ * @return the index greater than or equal to `from` of the first element
of this
+ * ByteString that starts a slice equal (as determined by `==`)
+ * to `slice`, or `-1`, if none exists.
+ * @since 2.0.0
+ */
+ def indexOfSlice(slice: Array[Byte], from: Int): Int = {
+ // this is only called if the first byte matches, so we can skip that check
+ def check(startPos: Int): Boolean = {
+ var i = startPos + 1
+ var j = 1
+ // let's trust the calling code has ensured that we have enough bytes in
this ByteString
Review Comment:
These comments suggest unsafe assumptions. The code should validate bounds
or the comments should clarify that the bounds are guaranteed by the `indexOf`
call limiting the search range.
##########
actor/src/main/scala/org/apache/pekko/util/ByteString.scala:
##########
@@ -1056,6 +1056,72 @@ sealed abstract class ByteString
*/
def indexOf(elem: Byte): Int = indexOf(elem, 0)
+ override def indexOfSlice[B >: Byte](slice: scala.collection.Seq[B], from:
Int): Int = {
+ // this is only called if the first byte matches, so we can skip that check
+ def check(startPos: Int): Boolean = {
+ var i = startPos + 1
+ var j = 1
+ // let's trust the calling code has ensured that we have enough bytes in
this ByteString
+ while (j < slice.length) {
+ if (apply(i) != slice(j)) return false
+ i += 1
+ j += 1
+ }
+ j == slice.length
+ }
+ val headByte = slice.head.asInstanceOf[Byte]
+ @tailrec def rec(from: Int): Int = {
+ val startPos = indexOf(headByte, from, length - slice.length + 1)
+ if (startPos == -1) -1
+ else if (check(startPos)) startPos
+ else rec(startPos + 1)
+ }
+ if (slice.isEmpty) 0 else rec(math.max(0, from))
+ }
+
+ /**
+ * Finds index of first occurrence of some slice in this ByteString.
+ *
+ * @param slice the slice to search for.
+ * @param from the start index
+ * @return the index greater than or equal to `from` of the first element
of this
+ * ByteString that starts a slice equal (as determined by `==`)
+ * to `slice`, or `-1`, if none exists.
+ * @since 2.0.0
+ */
+ def indexOfSlice(slice: Array[Byte], from: Int): Int = {
+ // this is only called if the first byte matches, so we can skip that check
+ def check(startPos: Int): Boolean = {
+ var i = startPos + 1
+ var j = 1
+ // let's trust the calling code has ensured that we have enough bytes in
this ByteString
+ while (j < slice.length) {
+ if (apply(i) != slice(j)) return false
+ i += 1
+ j += 1
+ }
+ j == slice.length
Review Comment:
This condition is redundant since the while loop ensures `j == slice.length`
when it exits normally. The function should simply return `true` at the end.
```suggestion
true
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
