ronantakizawa/codereview-bench huggingface dataset:
Repository: apple/pkl
File: pkl-cli/src/test/kotlin/org/pkl/cli/CliFormatterTest.kt (Kotlin)

Code before:
/*
 * Copyright © 2025-2026 Apple Inc. and the Pkl project authors. All rights reserved.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     https://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package org.pkl.cli

import java.nio.file.Path
import kotlin.io.path.writeText
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.io.TempDir
import org.pkl.commons.cli.CliException
import org.pkl.core.util.StringBuilderWriter
import org.pkl.formatter.GrammarVersion

class CliFormatterTest {

Review comment (security):
```suggestion
 * Copyright © 2025 Apple Inc. and the Pkl project authors. All rights reserved.
```

Code after:
/*
 * Copyright © 2025 Apple Inc. and the Pkl project authors. All rights reserved.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     https://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package org.pkl.cli

import java.nio.file.Path
import kotlin.io.path.writeText
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.io.TempDir
import org.pkl.commons.cli.CliException
import org.pkl.core.util.StringBuilderWriter
import org.pkl.formatter.GrammarVersion

class CliFormatterTest {
Repository: apple/pkl
File: pkl-cli/src/test/kotlin/org/pkl/cli/CliFormatterTest.kt (Kotlin)

Code before:
/*
 * Copyright © 2025Apple Inc. and the Pkl project authors. All rights reserved.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     https://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package org.pkl.cli

import java.nio.file.Path
import kotlin.io.path.writeText
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.io.TempDir
import org.pkl.commons.cli.CliException
import org.pkl.core.util.StringBuilderWriter
import org.pkl.formatter.GrammarVersion

class CliFormatterTest {

Review comment (security):
```suggestion
 * Copyright © 2025 Apple Inc. and the Pkl project authors. All rights reserved.
```

Code after:
/*
 * Copyright © 2025 Apple Inc. and the Pkl project authors. All rights reserved.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     https://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package org.pkl.cli

import java.nio.file.Path
import kotlin.io.path.writeText
import org.assertj.core.api.Assertions.assertThat
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.io.TempDir
import org.pkl.commons.cli.CliException
import org.pkl.core.util.StringBuilderWriter
import org.pkl.formatter.GrammarVersion

class CliFormatterTest {
Repository: apple/pkl
File: pkl-core/src/main/java/org/pkl/core/runtime/VmContext.java (Java)

Code before:

  public Logger getLogger() {
    return holder.logger;
  }

  public Map<String, String> getEnvironmentVariables() {
    return holder.environmentVariables;
  }

  public Map<String, String> getExternalProperties() {
    return holder.externalProperties;
  }

  public @Nullable PackageResolver getPackageResolver() {
    return holder.packageResolver;
  }

  public @Nullable ProjectDependenciesManager getProjectDependenciesManager() {
    return holder.projectDependenciesManager;
  }

  public TraceMode getTraceMode() {
    return holder.traceMode;
  }

  public boolean getPowerAssertions() {
    return holder.powerAssertions;
  }

  public VmValueTrackerFactory getValueTrackerFactory() {
    return valueTrackerFactory;
  }
}


Review comment (suggestion):
Nit: naming?
```suggestion
  public boolean getPowerAssertionsEnabled() {
```

Code after:
  }

  public Logger getLogger() {
    return holder.logger;
  }

  public Map<String, String> getEnvironmentVariables() {
    return holder.environmentVariables;
  }

  public Map<String, String> getExternalProperties() {
    return holder.externalProperties;
  }

  public @Nullable PackageResolver getPackageResolver() {
    return holder.packageResolver;
  }

  public @Nullable ProjectDependenciesManager getProjectDependenciesManager() {
    return holder.projectDependenciesManager;
  }

  public TraceMode getTraceMode() {
    return holder.traceMode;
  }

  public boolean getPowerAssertionsEnabled() {
    return holder.powerAssertions;
  }

  public VmValueTrackerFactory getValueTrackerFactory() {
    return valueTrackerFactory;
  }
}
Repository: apple/pkl
File: pkl-core/src/main/java/org/pkl/core/module/ResolvedModuleKeys.java (Java)

Code before:
    final URI uri;
    final Path path;
    final boolean nofollow;

    FileKey(ModuleKey original, URI uri, Path path, boolean nofollow) {
      this.original = original;
      this.uri = uri;
      this.path = path;
      this.nofollow = nofollow;
    }

    @Override
    public ModuleKey getOriginal() {
      return original;
    }

    @Override
    public URI getUri() {
      return uri;
    }

    @Override
    public String loadSource() throws IOException {
      try {
        if (nofollow) {
          try (InputStream in = Files.newInputStream(path, LinkOption.NOFOLLOW_LINKS)) {
            return new String(in.readAllBytes(), StandardCharsets.UTF_8);
          }
        }
        return Files.readString(path, StandardCharsets.UTF_8);
      } catch (AccessDeniedException e) {
        // Windows throws `AccessDeniedException` when reading directories.
        // Sync error between different OSes.
        if (Files.isDirectory(path)) {
          throw new IOException("Is a directory");
        }
        throw e;
      }
    }
  }

  private static class Url implements ResolvedModuleKey {
    final ModuleKey original;
    final URI uri;
    final URL url;

    Url(ModuleKey original, URI uri, URL url) {
      this.original = original;
      this.uri = uri;
      this.url = url;
    }

Review comment (suggestion):
```suggestion
          try (var in = Files.newInputStream(path, LinkOption.NOFOLLOW_LINKS)) {
```


Code after:
    final Path path;
    final boolean nofollow;

    FileKey(ModuleKey original, URI uri, Path path, boolean nofollow) {
      this.original = original;
      this.uri = uri;
      this.path = path;
      this.nofollow = nofollow;
    }

    @Override
    public ModuleKey getOriginal() {
      return original;
    }

    @Override
    public URI getUri() {
      return uri;
    }

    @Override
    public String loadSource() throws IOException {
      try {
        if (nofollow) {
          try (var in = Files.newInputStream(path, LinkOption.NOFOLLOW_LINKS)) {
            return new String(in.readAllBytes(), StandardCharsets.UTF_8);
          }
        }
        return Files.readString(path, StandardCharsets.UTF_8);
      } catch (AccessDeniedException e) {
        // Windows throws `AccessDeniedException` when reading directories.
        // Sync error between different OSes.
        if (Files.isDirectory(path)) {
          throw new IOException("Is a directory");
        }
        throw e;
      }
    }
  }

  private static class Url implements ResolvedModuleKey {
    final ModuleKey original;
    final URI uri;
    final URL url;

    Url(ModuleKey original, URI uri, URL url) {
      this.original = original;
      this.uri = uri;
      this.url = url;
    }
Repository: apple/swift-nio
File: Sources/NIOPosix/System.swift (Swift)

Code before:
        let res = try body()
        if res == -1 {
#if os(Windows)
            var err: CInt = 0
            _get_errno(&err)
#else
            let err = errno
#endif
            switch (err, blocking) {
            case (EINTR, _):
                continue
            case (EWOULDBLOCK, true):
                return .wouldBlock(0)
            default:
                preconditionIsNotUnacceptableErrno(err: err, where: function)
                throw IOError(errnoCode: err, reason: function)
            }
        }
        return .processed(res)
    }
}

@inline(__always)
@discardableResult
internal func syscall<T>(where function: String = #function,
                         _ body: () throws -> UnsafeMutablePointer<T>?)
        throws -> UnsafeMutablePointer<T> {
    while true {
        if let res = try body() {
            return res
        } else {
#if os(Windows)
            var err: CInt = 0
            _get_errno(&err)
#else
            let err = errno
#endif
            switch err {
            case EINTR:
                continue
            default:
                preconditionIsNotUnacceptableErrno(err: err, where: function)
                throw IOError(errnoCode: err, reason: function)
            }
        }
    }
}

@inline(__always)
@discardableResult
internal func syscallOptional<T>(where function: String = #function,

Review comment (refactor):
this helper and `syscall` variant that operates on the `OpaquePointer` seem to be platform-specific and used once (at least for now), do you think it's worth inlining them or making them plaftorm-specific?

Code after:
        let res = try body()
        if res == -1 {
#if os(Windows)
            var err: CInt = 0
            _get_errno(&err)
#else
            let err = errno
#endif
            switch (err, blocking) {
            case (EINTR, _):
                continue
            case (EWOULDBLOCK, true):
                return .wouldBlock(0)
            default:
                preconditionIsNotUnacceptableErrno(err: err, where: function)
                throw IOError(errnoCode: err, reason: function)
            }
        }
        return .processed(res)
    }
}

#if os(macOS) || os(iOS) || os(watchOS) || os(tvOS)
@inline(__always)
@discardableResult
internal func syscall<T>(where function: String = #function,
                         _ body: () throws -> UnsafeMutablePointer<T>?)
        throws -> UnsafeMutablePointer<T> {
    while true {
        if let res = try body() {
            return res
        } else {
            let err = errno
            switch err {
            case EINTR:
                continue
            default:
                preconditionIsNotUnacceptableErrno(err: err, where: function)
                throw IOError(errnoCode: err, reason: function)
            }
        }
    }
}
#elseif os(Linux) || os(Android)
@inline(__always)
@discardableResult
internal func syscall(where function: String = #function,
                      _ body: () throws -> OpaquePointer?)
            throws -> OpaquePointer {
    while true {
        if let res = try body() {
Repository: apple/swift-nio
File: Sources/NIOConcurrencyHelpers/LockedValue.swift (Swift)

Code before:
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) YEARS Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

/// Provides locked access to `Value`.
///
/// - note: ``NIOLockedValueBox`` has reference semantics and holds the ``Value``
///         alongside a lock behind a reference.
///
/// This is no different than creating a ``Lock`` and protecting all
/// accesses to a value using the lock. But it's easy to forget to actually
/// acquire/release the lock in the correct place. ``NIOLockedValueBox`` makes
/// that much easier.
public struct NIOLockedValueBox<Value> {
    @usableFromInline
    internal final class _Storage {
        @usableFromInline
        internal let _lock = Lock()
        
        @usableFromInline

Review comment (security):
```suggestion
// Copyright (c) 2022 Apple Inc. and the SwiftNIO project authors
```

Code after:
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2022 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

/// Provides locked access to `Value`.
///
/// - note: ``NIOLockedValueBox`` has reference semantics and holds the ``Value``
///         alongside a lock behind a reference.
///
/// This is no different than creating a ``Lock`` and protecting all
/// accesses to a value using the lock. But it's easy to forget to actually
/// acquire/release the lock in the correct place. ``NIOLockedValueBox`` makes
/// that much easier.
public struct NIOLockedValueBox<Value> {
    @usableFromInline
    internal final class _Storage {
        @usableFromInline
        internal let _lock = Lock()
        
        @usableFromInline
Repository: apple/swift-nio
File: Sources/NIOPerformanceTester/TCPThroughputBenchmark.swift (Swift)

Code before:
import NIOCore
import NIOPosix

/// Test measure a TCP channel throughput.
/// Server send 100K messages to the client,
/// measure the time from the very first message sent by the server
/// to the last message received by the clien.

class TCPThroughputBenchmark: Benchmark {

    private let messages: Int
    private let messageSize: Int

    private var group: EventLoopGroup!
    private var serverChannel: Channel!
    private var serverHandler: ServerHandler!
    private var clientChannel: Channel!

    private var message: ByteBuffer!
    private var isDonePromise: EventLoopPromise<Void>!

    final class ServerHandler: ChannelInboundHandler {
        public typealias InboundIn = ByteBuffer
        public typealias OutboundOut = ByteBuffer

        private var channel: Channel!

        public func channelActive(context: ChannelHandlerContext) {
            self.channel = context.channel
        }

        public func send(_ message: ByteBuffer, times count: Int) {
            for _ in 0..<count {
                _ = self.channel.writeAndFlush(message.slice())

Review comment (suggestion):
Suggestion:

```suggestion
final class TCPThroughputBenchmark: Benchmark {
```

Code after:
//===----------------------------------------------------------------------===//
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2017-2023 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

import NIOCore
import NIOPosix

/// Test measure a TCP channel throughput.
/// Server send 100K messages to the client,
/// measure the time from the very first message sent by the server
/// to the last message received by the client.

final class TCPThroughputBenchmark: Benchmark {

    private let messages: Int
    private let messageSize: Int

    private var group: EventLoopGroup!
    private var serverChannel: Channel!
    private var serverHandler: ServerHandler!
    private var clientChannel: Channel!

    private var message: ByteBuffer!
    private var isDonePromise: EventLoopPromise<Void>!
Repository: apple/swift-nio
File: Sources/CNIOLinux/shim.c (C)

Code before:
    return CMSG_NXTHDR(mhdr, cmsg);
}

const void *CNIOLinux_CMSG_DATA(const struct cmsghdr *cmsg) {
    assert(cmsg != NULL);
    return CMSG_DATA(cmsg);
}

void *CNIOLinux_CMSG_DATA_MUTABLE(struct cmsghdr *cmsg) {
    assert(cmsg != NULL);
    return CMSG_DATA(cmsg);
}

size_t CNIOLinux_CMSG_LEN(size_t payloadSizeBytes) {
    return CMSG_LEN(payloadSizeBytes);
}

size_t CNIOLinux_CMSG_SPACE(size_t payloadSizeBytes) {
    return CMSG_SPACE(payloadSizeBytes);
}

const int CNIOLinux_SO_TIMESTAMP = SO_TIMESTAMP;
const int CNIOLinux_SO_RCVTIMEO = SO_RCVTIMEO;

int CNIOLinux_supports_udp_segment() {
    #ifndef UDP_SEGMENT
    return -1;
    #endif

    int fd = socket(AF_INET, SOCK_DGRAM, 0);
    if (fd == -1) {
        return -1;
    }

    int gso_size = 512;
    int rc = setsockopt(fd, SOL_UDP, UDP_SEGMENT, &gso_size, sizeof(gso_size));
    close(fd);
    return rc;
}

#endif


Review comment (bug):
This guard isn't enough: we need to compile out the function below if `UDP_SEGMENT` isn't defined, or the compiler will still fail.

Code after:
    return CMSG_NXTHDR(mhdr, cmsg);
}

const void *CNIOLinux_CMSG_DATA(const struct cmsghdr *cmsg) {
    assert(cmsg != NULL);
    return CMSG_DATA(cmsg);
}

void *CNIOLinux_CMSG_DATA_MUTABLE(struct cmsghdr *cmsg) {
    assert(cmsg != NULL);
    return CMSG_DATA(cmsg);
}

size_t CNIOLinux_CMSG_LEN(size_t payloadSizeBytes) {
    return CMSG_LEN(payloadSizeBytes);
}

size_t CNIOLinux_CMSG_SPACE(size_t payloadSizeBytes) {
    return CMSG_SPACE(payloadSizeBytes);
}

const int CNIOLinux_SO_TIMESTAMP = SO_TIMESTAMP;
const int CNIOLinux_SO_RCVTIMEO = SO_RCVTIMEO;

int CNIOLinux_supports_udp_segment() {
    #ifndef UDP_SEGMENT
    return -1;
    #else
    int fd = socket(AF_INET, SOCK_DGRAM, 0);
    if (fd == -1) {
        return -1;
    }

    int gso_size = 512;
    int rc = setsockopt(fd, IPPROTO_UDP, UDP_SEGMENT, &gso_size, sizeof(gso_size));
    close(fd);
    return rc;
    #endif
}

#endif
Repository: apple/swift-nio
File: Sources/NIOAsyncAwaitDemo/AsyncChannelIO.swift (Swift)

Code before:
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2021 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

import NIOCore
import NIOHTTP1

@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
struct AsyncChannelIO<Request, Response> {
    let channel: Channel

    init(_ channel: Channel) {
        self.channel = channel
    }

    func start() async throws -> AsyncChannelIO<Request, Response> {
        self.channel.eventLoop.preconditionNotInEventLoop()
        try await channel.pipeline.addHandler(RequestResponseHandler<HTTPRequestHead, NIOHTTPClientResponseFull>()).get()
        return self
    }

    func sendRequest(_ request: Request) async throws -> Response {
        self.channel.eventLoop.preconditionNotInEventLoop()
        let responsePromise: EventLoopPromise<Response> = channel.eventLoop.makePromise()
        try await self.channel.writeAndFlush((request, responsePromise)).get()
        self.channel.eventLoop.preconditionNotInEventLoop()
        return try await responsePromise.futureResult.get()
    }

    func close() async throws {
        self.channel.eventLoop.preconditionNotInEventLoop()
        try await self.channel.close()
    }
}


Review comment (suggestion):
Hm yeah all those are cases where we'd want to stay on the EL... this is useful to point at as specific examples, thanks Cory!



Code after:
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2021 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of SwiftNIO project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//

import NIOCore
import NIOHTTP1

@available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *)
struct AsyncChannelIO<Request, Response> {
    let channel: Channel

    init(_ channel: Channel) {
        self.channel = channel
    }

    func start() async throws -> AsyncChannelIO<Request, Response> {
        try await channel.pipeline.addHandler(RequestResponseHandler<HTTPRequestHead, NIOHTTPClientResponseFull>()).get()
        return self
    }

    func sendRequest(_ request: Request) async throws -> Response {
        let responsePromise: EventLoopPromise<Response> = channel.eventLoop.makePromise()
        try await self.channel.writeAndFlush((request, responsePromise)).get()
        return try await responsePromise.futureResult.get()
    }

    func close() async throws {
        try await self.channel.close()
    }
}