Repository: apple/swift-nio
File: Sources/NIOCore/ByteBuffer-core.swift (Swift)

Code before:

        if self._readerIndex == self._writerIndex {
            // If the whole buffer was consumed we can just reset the readerIndex and writerIndex to 0 and move on.
            self._moveWriterIndex(to: 0)
            self._moveReaderIndex(to: 0)
            return true
        }

        if isKnownUniquelyReferenced(&self._storage) {
            self._storage.bytes.advanced(by: Int(self._slice.lowerBound))
                .copyMemory(from: self._storage.bytes.advanced(by: Int(self._slice.lowerBound + self._readerIndex)),
                            byteCount: self.readableBytes)
            let indexShift = self._readerIndex
            self._moveReaderIndex(to: 0)
            self._moveWriterIndex(to: self._writerIndex - indexShift)
        } else {
            self._copyStorageAndRebase(extraCapacity: 0, resetIndices: true)
        }
        return true
    }
    
    /// Reset capacity of the buffer to the specified amount.
    ///
    /// - returns: `true` if one or more bytes have been discarded, `false` if there are no bytes to discard.
    @inlinable
    @discardableResult public mutating func clampBufferCapacity(to maxRetainedCapacity: Int) -> Bool {
        guard storageCapacity > maxRetainedCapacity else {
            return false
        }
        
        if isKnownUniquelyReferenced(&self._storage) {
            self._storage.clampStorage(to: maxRetainedCapacity)
            self._slice = self._storage.fullSlice
        } else {
            self._copyStorageAndRebase(capacity: _toCapacity(maxRetainedCapacity), resetIndices: false)
        }
        return true
    }

    /// The reader index or the number of bytes previously read from this `ByteBuffer`. `readerIndex` is `0` for a
    /// newly allocated `ByteBuffer`.
    @inlinable
    public var readerIndex: Int {
        return Int(self._readerIndex)
    }

    /// The write index or the number of bytes previously written to this `ByteBuffer`. `writerIndex` is `0` for a
    /// newly allocated `ByteBuffer`.
    @inlinable
    public var writerIndex: Int {
        return Int(self._writerIndex)

Review comment (suggestion):
This method is pretty scary to me, as it behaves very strangely if there are more readable bytes in the buffer than the requested capacity. This behaviour is also out-of-line with the comment: the comment says it returns true if bytes have been discarded, but it returns true even if those bytes are not in the readable slice.

I think it might be better to have this refuse to shrink if the target capacity is _smaller_ than the number of readable bytes.

Code after:
        guard
            index >= 0 && length >= 0 && index >= self.readerIndex && length <= self.writerIndex
                && index <= self.writerIndex &- length
        else {
            return nil
        }
        let index = _toIndex(index)
        let length = _toCapacity(length)

        // The arithmetic below is safe because:
        // 1. maximum `writerIndex` <= self._slice.count (see `_moveWriterIndex`)
        // 2. `self._slice.lowerBound + self._slice.count` is always safe (because it's `self._slice.upperBound`)
        // 3. `index` is inside the range `self.readerIndex ... self.writerIndex` (the `guard` above)
        //
        // This means that the largest number that `index` could have is equal to
        // `self._slice_.upperBound = self._slice.lowerBound + self._slice.count` and that
        // is guaranteed to be expressible as a `UInt32` (because it's actually stored as such).
        let sliceStartIndex: UInt32 = self._slice.lowerBound + index

        guard sliceStartIndex <= ByteBuffer.Slice.maxSupportedLowerBound else {
            // the slice's begin is past the maximum supported slice begin value (16 MiB) so the only option we have
            // is copy the slice into a fresh buffer. The slice begin will then be at index 0.
            return self._copyIntoByteBufferWithSliceIndex0_slowPath(index: index, length: length)
        }
        var new = self
        assert(sliceStartIndex == self._slice.lowerBound &+ index)

        // - The arithmetic below is safe because
        //   1. `writerIndex` <= `self._slice.count` (see `_moveWriterIndex`)
        //   2. `length` <= `self.writerIndex` (see `guard`s)
        //   3. `sliceStartIndex` + `self._slice.count` is always safe (because that's `self._slice.upperBound`.
        // - The range construction is safe because `length` >= 0 (see `guard` at the beginning of the function).
        new._slice = _ByteBufferSlice(
            Range(
                uncheckedBounds: (
                    lower: sliceStartIndex,
                    upper: sliceStartIndex &+ length
                )
            )
        )
        new._moveReaderIndex(to: 0)
        new._moveWriterIndex(to: length)
        return new
    }

    /// Discard the bytes before the reader index. The byte at index `readerIndex` before calling this method will be
    /// at index `0` after the call returns.
    ///
    /// - returns: `true` if one or more bytes have been discarded, `false` if there are no bytes to discard.
    @inlinable
    @discardableResult public mutating func discardReadBytes() -> Bool {