ronantakizawa/codereview-bench huggingface dataset:
Repository: apple/swift-nio
File: Sources/NIOCore/Codec.swift (Swift)

Code before:
            promise?.fail(error)
            context.fireErrorCaught(error)
            return
        case .done:
            // let's just ignore this
            return
        case .operational:
            // there's actually some work to do here
            break
        }
        let data = Self.unwrapOutboundIn(data)

        do {
            self.buffer!.clear()
            try self.encoder.encode(data: data, out: &self.buffer!)
            context.write(Self.wrapOutboundOut(self.buffer!), promise: promise)
        } catch {
            self.state = .error(error)
            promise?.fail(error)
            context.fireErrorCaught(error)
        }
    }
    
    public func flush(context: ChannelHandlerContext) {
        context.flush()
        if let desiredBufferCapacity {
            self.buffer?.shrinkBufferCapacity(to: desiredBufferCapacity)
        }
    }
}


Review comment (suggestion):
```suggestion
        if let self.desiredBufferCapacity {
```

Code after:
            promise?.fail(error)
            context.fireErrorCaught(error)
            return
        case .done:
            // let's just ignore this
            return
        case .operational:
            // there's actually some work to do here
            break
        }
        let data = Self.unwrapOutboundIn(data)

        do {
            self.buffer!.clear()
            try self.encoder.encode(data: data, out: &self.buffer!)
            context.write(Self.wrapOutboundOut(self.buffer!), promise: promise)
        } catch {
            self.state = .error(error)
            promise?.fail(error)
            context.fireErrorCaught(error)
        }
    }

    public func flush(context: ChannelHandlerContext) {
        if let desiredBufferCapacity = self.desiredBufferCapacity {
            self.buffer?.shrinkBufferCapacity(to: desiredBufferCapacity)
        }
        context.flush()
    }
}
Repository: apple/swift-nio
File: Sources/NIOCore/Codec.swift (Swift)

Code before:
        case .error(let error):
            promise?.fail(error)
            context.fireErrorCaught(error)
            return
        case .done:
            // let's just ignore this
            return
        case .operational:
            // there's actually some work to do here
            break
        }
        let data = Self.unwrapOutboundIn(data)

        do {
            self.buffer!.clear()
            try self.encoder.encode(data: data, out: &self.buffer!)
            context.write(Self.wrapOutboundOut(self.buffer!), promise: promise)
        } catch {
            self.state = .error(error)
            promise?.fail(error)
            context.fireErrorCaught(error)
        }
    }
    
    public func flush(context: ChannelHandlerContext) {
        context.flush()
        if let desiredBufferCapacity {
            self.buffer?.shrinkBufferCapacity(to: desiredBufferCapacity)
        }
    }
}


Review comment (suggestion):
This should happen _after_ reconciling the state. `context.flush()` calls other code which may re-enter this method

Code after:
        case .error(let error):
            promise?.fail(error)
            context.fireErrorCaught(error)
            return
        case .done:
            // let's just ignore this
            return
        case .operational:
            // there's actually some work to do here
            break
        }
        let data = Self.unwrapOutboundIn(data)

        do {
            self.buffer!.clear()
            try self.encoder.encode(data: data, out: &self.buffer!)
            context.write(Self.wrapOutboundOut(self.buffer!), promise: promise)
        } catch {
            self.state = .error(error)
            promise?.fail(error)
            context.fireErrorCaught(error)
        }
    }

    public func flush(context: ChannelHandlerContext) {
        if let desiredBufferCapacity = self.desiredBufferCapacity {
            self.buffer?.shrinkBufferCapacity(to: desiredBufferCapacity)
        }
        context.flush()
    }
}
Repository: apple/swift-nio
File: Sources/NIOHTTP1/NIOTypedHTTPServerUpgraderStateMachine.swift (Swift)

Code before:
            if let element = unbuffering.buffer.popFirst() {
                self.state = .unbuffering(unbuffering)

                return .fireChannelRead(element)
            } else {
                self.state = .finished

                return .fireChannelReadCompleteAndRemoveHandler
            }

        case .modifying:
            fatalError("Internal inconsistency in HTTPServerUpgradeStateMachine")
            
        }
    }

    @usableFromInline
    enum CloseInboundAction {
        case close
        case `continue`
    }

    @inlinable
    mutating func closeInbound() -> CloseInboundAction {
        switch self.state {
        case .initial, .awaitingUpgrader:
            self.state = .finished
            return .close

        default:
            return .continue
        }
    }
 
}
#endif


Review comment (suggestion):
We should immediately close in the state `initial`. In the state `awaitingUpgrader` we have to check if we have `seenFirstRequest == true`. If have seen that we should buffer the `inputClosed` and send it after the upgrading is finished. In the state `upgraderReady` we need to buffer the `inputClosed` indefinitely.

The reason why this is so complicated is that the client could potentially send a half closure after it send the initial upgrade request + some data on the upgraded protocol. We can't just unconditionally close here but we rather need to buffer the `inputClose` and unbuffer it with the data. I would recommend changing the `var buffer: Deque<NIOAny>` in the various states to `var buffer: Deque<enum(NIOAny | inputClosed)>`.

Code after:

    @usableFromInline
    enum UnbufferAction {
        case close
        case fireChannelRead(NIOAny)
        case fireChannelReadCompleteAndRemoveHandler
        case fireInputClosedEvent
    }

    @inlinable
    mutating func unbuffer() -> UnbufferAction {
        switch self.state {
        case .initial, .awaitingUpgrader, .upgraderReady, .upgrading, .finished:
            preconditionFailure("Invalid state \(self.state)")

        case .unbuffering(var unbuffering):
            self.state = .modifying

            if let element = unbuffering.buffer.popFirst() {
                self.state = .unbuffering(unbuffering)
                switch element {
                case .data(let data):
                    return .fireChannelRead(data)
                case .inputClosed:
                    return .fireInputClosedEvent
                }
            } else {
                self.state = .finished

                return .fireChannelReadCompleteAndRemoveHandler
            }

        case .modifying:
            fatalError("Internal inconsistency in HTTPServerUpgradeStateMachine")

        }
    }

    @usableFromInline
    enum InputClosedAction {
        case close
        case `continue`
        case fireInputClosedEvent
    }

    @inlinable
    mutating func inputClosed() -> InputClosedAction {
        switch self.state {
        case .initial:
            self.state = .finished
            return .close
Repository: apple/swift-nio
File: Sources/NIOHTTP1/NIOTypedHTTPServerUpgraderStateMachine.swift (Swift)

Code before:
                // We should buffer the input close since we have seen the full request.
                awaitingUpgrader.buffer.append(.inputClosed)
                self.state = .awaitingUpgrader(awaitingUpgrader)
                return .continue
            } else {
                // We shouldn't buffer. This means we were still expecting HTTP parts.
                return .close
            }

        case .upgrading(var upgrading):
            upgrading.buffer.append(.inputClosed)
            self.state = .upgrading(upgrading)
            return .continue

        case .upgraderReady(var upgraderReady):
            upgraderReady.buffer.append(.inputClosed)
            self.state = .upgraderReady(upgraderReady)
            return .continue

        case .unbuffering(var unbuffering):
            unbuffering.buffer.append(.inputClosed)
            self.state = .unbuffering(unbuffering)
            return .continue

        case .modifying, .finished:
            return .continue
        }
    }

}
#endif


Review comment (suggestion):
In finished we should forward the `inputClosed` event. In `modifying` we should `fatalError` as we do in the other methods.

Code after:
        case .awaitingUpgrader(var awaitingUpgrader):
            if awaitingUpgrader.seenFirstRequest {
                // We should buffer the input close since we have seen the full request.
                awaitingUpgrader.buffer.append(.inputClosed)
                self.state = .awaitingUpgrader(awaitingUpgrader)
                return .continue
            } else {
                // We shouldn't buffer. This means we were still expecting HTTP parts.
                return .close
            }

        case .upgrading(var upgrading):
            upgrading.buffer.append(.inputClosed)
            self.state = .upgrading(upgrading)
            return .continue

        case .upgraderReady:
            // if the state is `upgraderReady` we have received a `.head` but not an `.end`.
            // If input is closed then there is no way to move this forward so we should
            // close.
            self.state = .finished
            return .close

        case .unbuffering(var unbuffering):
            unbuffering.buffer.append(.inputClosed)
            self.state = .unbuffering(unbuffering)
            return .continue

        case .finished:
            return .fireInputClosedEvent

        case .modifying:
            fatalError("Internal inconsistency in HTTPServerUpgradeStateMachine")
        }
    }

}
#endif
Repository: apple/swift-nio
File: Sources/NIOFileSystem/Internal/System Calls/Syscalls.swift (Swift)

Code before:
    _ dir: CInterop.DirPointer
) -> UnsafeMutablePointer<CInterop.DirEnt>? {
    readdir(dir)
}

/// readdir(3): Closes the directory stream and frees associated structures
internal func libc_closedir(
    _ dir: CInterop.DirPointer
) -> CInt {
    closedir(dir)
}

/// remove(3): Remove directory entry
internal func libc_remove(
    _ path: UnsafePointer<CInterop.PlatformChar>
) -> CInt {
    #if ENABLE_MOCKING
    if mockingEnabled {
        return mock(path)
    }
    #endif
    return remove(path)
}

#if canImport(Darwin)
/// copyfile(3): Copy a file from one file to another. (fcopyfile)
internal func libc_fcopyfile(
    _ from: CInt,
    _ to: CInt,
    _ state: copyfile_state_t?,
    _ flags: copyfile_flags_t
) -> CInt {
    #if ENABLE_MOCKING
    if mockingEnabled {
        return mock(from, to, state, flags)
    }
    #endif
    return fcopyfile(from, to, state, flags)
}
#endif

#if canImport(Darwin)
/// copyfile(3): Copy a file from one file to another. (copyfile)
internal func libc_copyfile(
    _ from: UnsafePointer<CInterop.PlatformChar>,
    _ to: UnsafePointer<CInterop.PlatformChar>,
    _ state: copyfile_state_t?,
    _ flags: copyfile_flags_t
) -> CInt {
    #if ENABLE_MOCKING
    if mockingEnabled {

Review comment (suggestion):
nit
```suggestion
/// fcopyfile(3): Copy a file from one file to another.
```

Code after:
    _ dir: CInterop.DirPointer
) -> UnsafeMutablePointer<CInterop.DirEnt>? {
    readdir(dir)
}

/// readdir(3): Closes the directory stream and frees associated structures
internal func libc_closedir(
    _ dir: CInterop.DirPointer
) -> CInt {
    closedir(dir)
}

/// remove(3): Remove directory entry
internal func libc_remove(
    _ path: UnsafePointer<CInterop.PlatformChar>
) -> CInt {
    #if ENABLE_MOCKING
    if mockingEnabled {
        return mock(path)
    }
    #endif
    return remove(path)
}

#if canImport(Darwin)
/// fcopyfile(3): Copy a file from one file to another.
internal func libc_fcopyfile(
    _ from: CInt,
    _ to: CInt,
    _ state: copyfile_state_t?,
    _ flags: copyfile_flags_t
) -> CInt {
    #if ENABLE_MOCKING
    if mockingEnabled {
        return mock(from, to, state, flags)
    }
    #endif
    return fcopyfile(from, to, state, flags)
}
#endif

#if canImport(Darwin)
/// copyfile(3): Copy a file from one file to another.
internal func libc_copyfile(
    _ from: UnsafePointer<CInterop.PlatformChar>,
    _ to: UnsafePointer<CInterop.PlatformChar>,
    _ state: copyfile_state_t?,
    _ flags: copyfile_flags_t
) -> CInt {
    #if ENABLE_MOCKING
    if mockingEnabled {
Repository: apple/swift-nio
File: Sources/NIOWebSocket/WebSocketFrame.swift (Swift)

Code before:
    ///
    /// The format of the description is not API.
    ///
    /// - returns: A description of this `WebSocketFrame`.
    public var description: String {
        """
        maskKey: \(String(describing: self.maskKey)), \
        fin: \(self.fin), \
        rsv1: \(self.rsv1), \
        rsv2: \(self.rsv2), \
        rsv3: \(self.rsv3), \
        opcode: \(self.opcode), \
        length: \(self.length), \
        data: \(String(describing: self.data)), \
        extensionData: \(String(describing: self.extensionData)), \
        unmaskedData: \(String(describing: self.unmaskedData)), \
        unmaskedDataExtension: \(String(describing: self.unmaskedExtensionData))
        """
    }
}

extension WebSocketFrame: CustomDebugStringConvertible {
    public var debugDescription: String {
        """
        ( \
        maskKey: \(String(describing: self.maskKey)), \
        fin: \(self.fin), \
        rsv1: \(self.rsv1), \
        rsv2: \(self.rsv2), \
        rsv3: \(self.rsv3), \
        opcode: \(self.opcode), \
        length: \(self.length), \
        data: \(String(describing: self.data)), \
        extensionData: \(String(describing: self.extensionData)), \
        unmaskedData: \(String(describing: self.unmaskedData)), \
        unmaskedDataExtension: \(String(describing: self.unmaskedExtensionData)) \
        )
        """
    }
}


Review comment (bug):
You can just interpolate `self.description` here instead of duplicating the code. Can we also add a test for the `debugDescription` please

Code after:
    ///
    /// The format of the description is not API.
    ///
    /// - returns: A description of this `WebSocketFrame`.
    public var description: String {
        """
        maskKey: \(String(describing: self.maskKey)), \
        fin: \(self.fin), \
        rsv1: \(self.rsv1), \
        rsv2: \(self.rsv2), \
        rsv3: \(self.rsv3), \
        opcode: \(self.opcode), \
        length: \(self.length), \
        data: \(String(describing: self.data)), \
        extensionData: \(String(describing: self.extensionData)), \
        unmaskedData: \(String(describing: self.unmaskedData)), \
        unmaskedDataExtension: \(String(describing: self.unmaskedExtensionData))
        """
    }
}

extension WebSocketFrame: CustomDebugStringConvertible {
    public var debugDescription: String {
        "(\(self.description))"
    }
}
Repository: apple/swift-nio
File: Sources/NIOCore/Utilities.swift (Swift)

Code before:
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramSegmentSize`` channel option.
    public static let supportsUDPSegmentationOffload: Bool = CNIOLinux_supports_udp_segment()
    #else
    /// Returns true if the platform supports `UDP_SEGMENT` (GSO).
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramSegmentSize`` channel option.
    public static let supportsUDPSegmentationOffload: Bool = false
    #endif

    #if os(Linux)
    /// Returns true if the platform supports `UDP_GRO`.
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramReceiveOffload`` channel option.
    public static let supportsUDPReceiveOffload: Bool = CNIOLinux_supports_udp_gro()
    #else
    /// Returns true if the platform supports `UDP_GRO`.
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramReceiveOffload`` channel option.
    public static let supportsUDPReceiveOffload: Bool = false
    #endif
}

extension System {
    /// Returns the UDP maximum segment count if the platform supports and defines it.
    public static func udpMaxSegments() -> Int? {
        #if os(Linux)
        if CNIOLinux_udp_max_segments_defined() {
            return Int(CNIOLinux_UDP_MAX_SEGMENTS)
        } else {
            return nil
        }
        #else
        return nil
        #endif
    }
}


Review comment (suggestion):
This should really be a `static let` that's computed on first access

Code after:
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramSegmentSize`` channel option.
    public static let supportsUDPSegmentationOffload: Bool = CNIOLinux_supports_udp_segment()
    #else
    /// Returns true if the platform supports `UDP_SEGMENT` (GSO).
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramSegmentSize`` channel option.
    public static let supportsUDPSegmentationOffload: Bool = false
    #endif

    #if os(Linux)
    /// Returns true if the platform supports `UDP_GRO`.
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramReceiveOffload`` channel option.
    public static let supportsUDPReceiveOffload: Bool = CNIOLinux_supports_udp_gro()
    #else
    /// Returns true if the platform supports `UDP_GRO`.
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramReceiveOffload`` channel option.
    public static let supportsUDPReceiveOffload: Bool = false
    #endif

    /// Returns the UDP maximum segment count if the platform supports and defines it.
    public static var udpMaxSegments: Int? {
        #if os(Linux)
        let maxSegments = CNIOLinux_UDP_MAX_SEGMENTS
        if maxSegments != -1 {
            return maxSegments
        }
        #endif
        return nil
    }
}
Repository: apple/swift-nio
File: Sources/NIOCore/Utilities.swift (Swift)

Code before:
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramSegmentSize`` channel option.
    public static let supportsUDPSegmentationOffload: Bool = CNIOLinux_supports_udp_segment()
    #else
    /// Returns true if the platform supports `UDP_SEGMENT` (GSO).
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramSegmentSize`` channel option.
    public static let supportsUDPSegmentationOffload: Bool = false
    #endif

    #if os(Linux)
    /// Returns true if the platform supports `UDP_GRO`.
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramReceiveOffload`` channel option.
    public static let supportsUDPReceiveOffload: Bool = CNIOLinux_supports_udp_gro()
    #else
    /// Returns true if the platform supports `UDP_GRO`.
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramReceiveOffload`` channel option.
    public static let supportsUDPReceiveOffload: Bool = false
    #endif

    /// Returns the UDP maximum segment count if the platform supports and defines it.
    public static var udpMaxSegments: Int? {
        #if os(Linux)
        if CNIOLinux_UDP_MAX_SEGMENTS != -1 {
            return CNIOLinux_UDP_MAX_SEGMENTS
        }
        #endif
        return nil
    }
}


Review comment (performance):
Mild nit: I suspect the optimizer will end up calling this function twice, as it doesn't know what it does. I recommend that you save the value to a temporary and branch from there.

Code after:
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramSegmentSize`` channel option.
    public static let supportsUDPSegmentationOffload: Bool = CNIOLinux_supports_udp_segment()
    #else
    /// Returns true if the platform supports `UDP_SEGMENT` (GSO).
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramSegmentSize`` channel option.
    public static let supportsUDPSegmentationOffload: Bool = false
    #endif

    #if os(Linux)
    /// Returns true if the platform supports `UDP_GRO`.
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramReceiveOffload`` channel option.
    public static let supportsUDPReceiveOffload: Bool = CNIOLinux_supports_udp_gro()
    #else
    /// Returns true if the platform supports `UDP_GRO`.
    ///
    /// The option can be enabled by setting the ``ChannelOptions/Types/DatagramReceiveOffload`` channel option.
    public static let supportsUDPReceiveOffload: Bool = false
    #endif

    /// Returns the UDP maximum segment count if the platform supports and defines it.
    public static var udpMaxSegments: Int? {
        #if os(Linux)
        let maxSegments = CNIOLinux_UDP_MAX_SEGMENTS
        if maxSegments != -1 {
            return maxSegments
        }
        #endif
        return nil
    }
}
Repository: apple/swift-nio
File: Sources/NIOFileSystem/ByteCount.swift (Swift)

Code before:
    /// - Parameter count: The number of kibibytes
    public static func kibibytes(_ count: Int64) -> ByteCount {
        ByteCount(bytes: 1024 * count)
    }

    /// Returns a ``ByteCount`` with a given number of mebibytes
    ///
    /// One mebibyte is 10,485,760 bytes.
    ///
    /// - Parameter count: The number of mebibytes
    public static func mebibytes(_ count: Int64) -> ByteCount {
        ByteCount(bytes: 1024 * 1024 * count)
    }

    /// Returns a ``ByteCount`` with a given number of gibibytes
    ///
    /// One gibibyte is 10,737,418,240 bytes.
    ///
    /// - Parameter count: The number of gibibytes
    public static func gibibytes(_ count: Int64) -> ByteCount {
        ByteCount(bytes: 1024 * 1024 * 1024 * count)
    }
}

extension ByteCount: AdditiveArithmetic {
    public static var zero = ByteCount(bytes: 0)

    public static func + (lhs: ByteCount, rhs: ByteCount) -> ByteCount {
        ByteCount(bytes: lhs.bytes + rhs.bytes)
    }

    public static func - (lhs: ByteCount, rhs: ByteCount) -> ByteCount {
        ByteCount(bytes: lhs.bytes - rhs.bytes)
    }
}

extension ByteCount: Comparable {
    public static func < (lhs: ByteCount, rhs: ByteCount) -> Bool {
        lhs.bytes < rhs.bytes
    }
}

#endif


Review comment (suggestion):
This should either be a `static let` or a `static` computed property:

```suggestion
    public static var zero: ByteCount {  ByteCount(bytes: 0) }
```

It's not a good idea to let any user change the meaning of `zero`!

Code after:
    /// - Parameter count: The number of kibibytes
    public static func kibibytes(_ count: Int64) -> ByteCount {
        ByteCount(bytes: 1024 * count)
    }

    /// Returns a ``ByteCount`` with a given number of mebibytes
    ///
    /// One mebibyte is 10,485,760 bytes.
    ///
    /// - Parameter count: The number of mebibytes
    public static func mebibytes(_ count: Int64) -> ByteCount {
        ByteCount(bytes: 1024 * 1024 * count)
    }

    /// Returns a ``ByteCount`` with a given number of gibibytes
    ///
    /// One gibibyte is 10,737,418,240 bytes.
    ///
    /// - Parameter count: The number of gibibytes
    public static func gibibytes(_ count: Int64) -> ByteCount {
        ByteCount(bytes: 1024 * 1024 * 1024 * count)
    }
}

extension ByteCount: AdditiveArithmetic {
    public static var zero: ByteCount { ByteCount(bytes: 0) }

    public static func + (lhs: ByteCount, rhs: ByteCount) -> ByteCount {
        ByteCount(bytes: lhs.bytes + rhs.bytes)
    }

    public static func - (lhs: ByteCount, rhs: ByteCount) -> ByteCount {
        ByteCount(bytes: lhs.bytes - rhs.bytes)
    }
}

extension ByteCount: Comparable {
    public static func < (lhs: ByteCount, rhs: ByteCount) -> Bool {
        lhs.bytes < rhs.bytes
    }
}

#endif