ronantakizawa/codereview-bench huggingface dataset:
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-core/common/src/CoroutineContext.common.kt (Kotlin)

Code before:
/*
 * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */

package kotlinx.coroutines

import kotlin.coroutines.*

/**
 * Creates a context for the new coroutine. It installs [Dispatchers.Default] when no other dispatcher or
 * [ContinuationInterceptor] is specified, and adds optional support for debugging facilities (when turned on)
 * and copyable thread local facilities on JVM.
 */
public expect fun CoroutineScope.newCoroutineContext(context: CoroutineContext): CoroutineContext

/**
 * Creates context for coroutine builder functions that do not launch a new coroutine, namely [withContext].
 * @suppress
 */
@InternalCoroutinesApi
public expect fun CoroutineContext.newCoroutineContext(addedContext: CoroutineContext): CoroutineContext

@PublishedApi
@Suppress("PropertyName")
internal expect val DefaultDelay: Delay

// countOrElement -- pre-cached value for ThreadContext.kt
internal expect inline fun <T> withCoroutineContext(context: CoroutineContext, countOrElement: Any?, block: () -> T): T
internal expect inline fun <T> withContinuationContext(continuation: Continuation<*>, countOrElement: Any?, block: () -> T): T
internal expect fun Continuation<*>.toDebugString(): String
internal expect val CoroutineContext.coroutineName: String?


Review comment (suggestion):
```suggestion
 * and copyable-thread-local facilities on JVM.
```
See https://www.grammarbook.com/punctuation/hyphens.asp, rule 1a.

Code after:
/*
 * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */

package kotlinx.coroutines

import kotlin.coroutines.*

/**
 * Creates a context for a new coroutine. It installs [Dispatchers.Default] when no other dispatcher or
 * [ContinuationInterceptor] is specified and adds optional support for debugging facilities (when turned on)
 * and copyable-thread-local facilities on JVM.
 */
public expect fun CoroutineScope.newCoroutineContext(context: CoroutineContext): CoroutineContext

/**
 * Creates a context for coroutine builder functions that do not launch a new coroutine, e.g. [withContext].
 * @suppress
 */
@InternalCoroutinesApi
public expect fun CoroutineContext.newCoroutineContext(addedContext: CoroutineContext): CoroutineContext

@PublishedApi
@Suppress("PropertyName")
internal expect val DefaultDelay: Delay

// countOrElement -- pre-cached value for ThreadContext.kt
internal expect inline fun <T> withCoroutineContext(context: CoroutineContext, countOrElement: Any?, block: () -> T): T
internal expect inline fun <T> withContinuationContext(continuation: Continuation<*>, countOrElement: Any?, block: () -> T): T
internal expect fun Continuation<*>.toDebugString(): String
internal expect val CoroutineContext.coroutineName: String?
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-core/common/src/CoroutineContext.common.kt (Kotlin)

Code before:
/*
 * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */

package kotlinx.coroutines

import kotlin.coroutines.*

/**
 * Creates a context for the new coroutine. It installs [Dispatchers.Default] when no other dispatcher or
 * [ContinuationInterceptor] is specified, and adds optional support for debugging facilities (when turned on)
 * and copyable thread local facilities on JVM.
 */
public expect fun CoroutineScope.newCoroutineContext(context: CoroutineContext): CoroutineContext

/**
 * Creates context for coroutine builder functions that do not launch a new coroutine, namely [withContext].
 * @suppress
 */
@InternalCoroutinesApi
public expect fun CoroutineContext.newCoroutineContext(addedContext: CoroutineContext): CoroutineContext

@PublishedApi
@Suppress("PropertyName")
internal expect val DefaultDelay: Delay

// countOrElement -- pre-cached value for ThreadContext.kt
internal expect inline fun <T> withCoroutineContext(context: CoroutineContext, countOrElement: Any?, block: () -> T): T
internal expect inline fun <T> withContinuationContext(continuation: Continuation<*>, countOrElement: Any?, block: () -> T): T
internal expect fun Continuation<*>.toDebugString(): String
internal expect val CoroutineContext.coroutineName: String?


Review comment (suggestion):
```suggestion
 * Creates a context for a new coroutine. It installs [Dispatchers.Default] when no other dispatcher or
```
"Any new coroutine, without specifying which one"

Code after:
/*
 * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */

package kotlinx.coroutines

import kotlin.coroutines.*

/**
 * Creates a context for a new coroutine. It installs [Dispatchers.Default] when no other dispatcher or
 * [ContinuationInterceptor] is specified and adds optional support for debugging facilities (when turned on)
 * and copyable-thread-local facilities on JVM.
 */
public expect fun CoroutineScope.newCoroutineContext(context: CoroutineContext): CoroutineContext

/**
 * Creates a context for coroutine builder functions that do not launch a new coroutine, e.g. [withContext].
 * @suppress
 */
@InternalCoroutinesApi
public expect fun CoroutineContext.newCoroutineContext(addedContext: CoroutineContext): CoroutineContext

@PublishedApi
@Suppress("PropertyName")
internal expect val DefaultDelay: Delay

// countOrElement -- pre-cached value for ThreadContext.kt
internal expect inline fun <T> withCoroutineContext(context: CoroutineContext, countOrElement: Any?, block: () -> T): T
internal expect inline fun <T> withContinuationContext(continuation: Continuation<*>, countOrElement: Any?, block: () -> T): T
internal expect fun Continuation<*>.toDebugString(): String
internal expect val CoroutineContext.coroutineName: String?
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-core/common/src/CoroutineContext.common.kt (Kotlin)

Code before:
/*
 * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */

package kotlinx.coroutines

import kotlin.coroutines.*

/**
 * Creates a context for the new coroutine. It installs [Dispatchers.Default] when no other dispatcher or
 * [ContinuationInterceptor] is specified, and adds optional support for debugging facilities (when turned on)
 * and copyable thread local facilities on JVM.
 */
public expect fun CoroutineScope.newCoroutineContext(context: CoroutineContext): CoroutineContext

/**
 * Creates context for coroutine builder functions that do not launch a new coroutine, namely [withContext].
 * @suppress
 */
@InternalCoroutinesApi
public expect fun CoroutineContext.newCoroutineContext(addedContext: CoroutineContext): CoroutineContext

@PublishedApi
@Suppress("PropertyName")
internal expect val DefaultDelay: Delay

// countOrElement -- pre-cached value for ThreadContext.kt
internal expect inline fun <T> withCoroutineContext(context: CoroutineContext, countOrElement: Any?, block: () -> T): T
internal expect inline fun <T> withContinuationContext(continuation: Continuation<*>, countOrElement: Any?, block: () -> T): T
internal expect fun Continuation<*>.toDebugString(): String
internal expect val CoroutineContext.coroutineName: String?


Review comment (bug):
```suggestion
 * [ContinuationInterceptor] is specified and adds optional support for debugging facilities (when turned on)
```
The Oxford comma is only used when the enumeration has > 2 elements.

Code after:
/*
 * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */

package kotlinx.coroutines

import kotlin.coroutines.*

/**
 * Creates a context for a new coroutine. It installs [Dispatchers.Default] when no other dispatcher or
 * [ContinuationInterceptor] is specified and adds optional support for debugging facilities (when turned on)
 * and copyable-thread-local facilities on JVM.
 */
public expect fun CoroutineScope.newCoroutineContext(context: CoroutineContext): CoroutineContext

/**
 * Creates a context for coroutine builder functions that do not launch a new coroutine, e.g. [withContext].
 * @suppress
 */
@InternalCoroutinesApi
public expect fun CoroutineContext.newCoroutineContext(addedContext: CoroutineContext): CoroutineContext

@PublishedApi
@Suppress("PropertyName")
internal expect val DefaultDelay: Delay

// countOrElement -- pre-cached value for ThreadContext.kt
internal expect inline fun <T> withCoroutineContext(context: CoroutineContext, countOrElement: Any?, block: () -> T): T
internal expect inline fun <T> withContinuationContext(continuation: Continuation<*>, countOrElement: Any?, block: () -> T): T
internal expect fun Continuation<*>.toDebugString(): String
internal expect val CoroutineContext.coroutineName: String?
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-core/jvm/test/ThreadContextMutableCopiesTest.kt (Kotlin)

Code before:
/*
 * Copyright 2016-2018 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */

package kotlinx.coroutines

import kotlin.coroutines.*
import kotlin.test.*

class ThreadContextMutableCopiesTest : TestBase() {
    companion object {
        val threadLocalData: ThreadLocal<MutableList<String>> = ThreadLocal.withInitial { ArrayList() }
    }

    class MyMutableElement(
        val mutableData: MutableList<String>
    ) : CopyableThreadContextElement<MutableList<String>> {

        companion object Key : CoroutineContext.Key<MyMutableElement>

        override val key: CoroutineContext.Key<*>
            get() = Key

        override fun updateThreadContext(context: CoroutineContext): MutableList<String> {
            val st = threadLocalData.get()
            threadLocalData.set(mutableData)
            return st

Review comment (suggestion):
```suggestion
 * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
```

Code after:
/*
 * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */

package kotlinx.coroutines

import kotlin.coroutines.*
import kotlin.test.*

class ThreadContextMutableCopiesTest : TestBase() {
    companion object {
        val threadLocalData: ThreadLocal<MutableList<String>> = ThreadLocal.withInitial { ArrayList() }
    }

    class MyMutableElement(
        val mutableData: MutableList<String>
    ) : CopyableThreadContextElement<MutableList<String>> {

        companion object Key : CoroutineContext.Key<MyMutableElement>

        override val key: CoroutineContext.Key<*>
            get() = Key

        override fun updateThreadContext(context: CoroutineContext): MutableList<String> {
            val st = threadLocalData.get()
            threadLocalData.set(mutableData)
            return st
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-core/jvm/test/internal/ThreadSafeHeapStressTest.kt (Kotlin)

Code before:
                    barrier.await()
                    heap.remove(node)
                }
                launch {
                    barrier.await()
                    Thread.yield()
                    node.dispose()
                }
            }
        }
    }

    @Test()
    fun testConcurrentAddDispose() = runTest {
        repeat(10_000 * stressTestMultiplierSqrt) {
            val jobToCancel = Job()
            val barrier = CyclicBarrier(2)
            val jobToJoin = launch(Dispatchers.Default) {
                barrier.await()
                jobToCancel.cancelAndJoin()
            }

            try {
                runBlocking { // Use event loop impl
                    withContext(jobToCancel) {
                        // This one is to workaround heap allocation optimization
                        launch(start = CoroutineStart.UNDISPATCHED) {
                            delay(100_000)
                        }
                        barrier.await()
                        delay(100_000)
                    }
                }
            } catch (e: CancellationException) {
                // Expected exception
            }
            jobToJoin.join()
        }
    }
}


Review comment (suggestion):
```suggestion
                        // This one is to work around heap allocation optimization
```

Code after:
                    barrier.await()
                    heap.remove(node)
                }
                launch {
                    barrier.await()
                    Thread.yield()
                    node.dispose()
                }
            }
        }
    }

    @Test()
    fun testConcurrentAddDispose() = runTest {
        repeat(10_000 * stressTestMultiplierSqrt) {
            val jobToCancel = Job()
            val barrier = CyclicBarrier(2)
            val jobToJoin = launch(Dispatchers.Default) {
                barrier.await()
                jobToCancel.cancelAndJoin()
            }

            try {
                runBlocking { // Use event loop impl
                    withContext(jobToCancel) {
                        // This one is to work around heap allocation optimization
                        launch(start = CoroutineStart.UNDISPATCHED) {
                            delay(100_000)
                        }
                        barrier.await()
                        delay(100_000)
                    }
                }
            } catch (e: CancellationException) {
                // Expected exception
            }
            jobToJoin.join()
        }
    }
}
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-core/jvm/test/guide/example-context-06.kt (Kotlin)

Code before:
// This file was automatically generated from coroutine-context-and-dispatchers.md by Knit tool. Do not edit.
package kotlinx.coroutines.guide.exampleContext06

import kotlinx.coroutines.*

fun main() = runBlocking<Unit> {
    // launch a coroutine to process some kind of incoming request
    val request = launch {
        // it spawns two other jobs
        launch(Job()) { 
            println("job1: I run in my own Job and execute independently!")
            delay(1000)
            println("job1: I am not affected by cancellation of the request")
        }
        // and the other inherits the parent context
        launch {
            delay(100)
            println("job2: I am a child of the request coroutine")
            delay(1000)
            println("job2: I will not execute this line if my parent request is cancelled")
        }
    }
    delay(500)
    request.cancel() // cancel processing of the request
    println("main: Who has survived request cancellation?")
    delay(1000) // delay a second to see what happens
}


Review comment (suggestion):
```suggestion
    delay(1000) // delay the main thread for a second to see what happens
```

Code after:
// This file was automatically generated from coroutine-context-and-dispatchers.md by Knit tool. Do not edit.
package kotlinx.coroutines.guide.exampleContext06

import kotlinx.coroutines.*

fun main() = runBlocking<Unit> {
    // launch a coroutine to process some kind of incoming request
    val request = launch {
        // it spawns two other jobs
        launch(Job()) { 
            println("job1: I run in my own Job and execute independently!")
            delay(1000)
            println("job1: I am not affected by cancellation of the request")
        }
        // and the other inherits the parent context
        launch {
            delay(100)
            println("job2: I am a child of the request coroutine")
            delay(1000)
            println("job2: I will not execute this line if my parent request is cancelled")
        }
    }
    delay(500)
    request.cancel() // cancel processing of the request
    println("main: Who has survived request cancellation?")
    delay(1000) // delay the main thread for a second to see what happens
}
Repository: Kotlin/kotlinx.coroutines
File: integration-testing/src/debugDynamicAgentTest/kotlin/DynamicAttachDebugTest.kt (Kotlin)

Code before:
/*
 * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */
import org.junit.*
import kotlinx.coroutines.*
import kotlinx.coroutines.debug.*
import org.junit.Test
import java.io.*
import java.lang.IllegalStateException

class DynamicAttachDebugTest {

    @Test
    fun testAgentDumpsCoroutines() =
        DebugProbes.withDebugProbes {
            runBlocking {
                val baos = ByteArrayOutputStream()
                DebugProbes.dumpCoroutines(PrintStream(baos))
                // if the agent works, then dumps should contain something,
                // at least the fact that this test is running.
                Assert.assertTrue(baos.toString().contains("testAgentDumpsCoroutines"))
            }
        }

    @Test(expected = IllegalStateException::class)
    fun testAgentIsNotInstalled() {
        DebugProbes.dumpCoroutines(PrintStream(ByteArrayOutputStream()))

Review comment (suggestion):
```suggestion
 * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
```

Code after:
/*
 * Copyright 2016-2022 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */
import org.junit.*
import kotlinx.coroutines.*
import kotlinx.coroutines.debug.*
import org.junit.Test
import java.io.*
import java.lang.IllegalStateException

class DynamicAttachDebugTest {

    @Test
    fun testAgentDumpsCoroutines() =
        DebugProbes.withDebugProbes {
            runBlocking {
                val baos = ByteArrayOutputStream()
                DebugProbes.dumpCoroutines(PrintStream(baos))
                // if the agent works, then dumps should contain something,
                // at least the fact that this test is running.
                Assert.assertTrue(baos.toString().contains("testAgentDumpsCoroutines"))
            }
        }

    @Test(expected = IllegalStateException::class)
    fun testAgentIsNotInstalled() {
        DebugProbes.dumpCoroutines(PrintStream(ByteArrayOutputStream()))
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-test/build.gradle.kts (Kotlin)

Code before:
import org.jetbrains.kotlin.gradle.plugin.mpp.*

/*
 * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */

val experimentalAnnotations = listOf(
    "kotlin.Experimental",
    "kotlinx.coroutines.ExperimentalCoroutinesApi",
    "kotlinx.coroutines.InternalCoroutinesApi"
)

kotlin {
    sourceSets.all { configureMultiplatform() }

    targets.withType(KotlinNativeTargetWithTests::class.java).configureEach {
        val tests = binaries.getTest("DEBUG")
        tests.optimized = true
        tests.binaryOptions["memoryModel"] = "experimental"
    }
}


Review comment (bug):
Style:
```suggestion
        binaries.getTest("DEBUG").apply {
            optimized = true
            binaryOptions["memoryModel"] = "experimental"
        }
```
I don't think the name `tests` is informative enough here to preserve it.

Code after:
import org.jetbrains.kotlin.gradle.plugin.mpp.*

/*
 * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
 */

val experimentalAnnotations = listOf(
    "kotlin.Experimental",
    "kotlinx.coroutines.ExperimentalCoroutinesApi",
    "kotlinx.coroutines.InternalCoroutinesApi"
)

kotlin {
    sourceSets.all { configureMultiplatform() }

    targets.withType(KotlinNativeTargetWithTests::class.java).configureEach {
        binaries.getTest("DEBUG").apply {
            optimized = true
            binaryOptions["memoryModel"] = "experimental"
        }
    }
}
Repository: Kotlin/kotlinx.coroutines
File: kotlinx-coroutines-core/common/src/EventLoop.common.kt (Kotlin)

Code before:
        useCount += delta(unconfined)
        if (!unconfined) shared = true 
    }

    fun decrementUseCount(unconfined: Boolean = false) {
        useCount -= delta(unconfined)
        if (useCount > 0) return
        assert { useCount == 0L } // "Extra decrementUseCount"
        if (shared) {
            // shut it down and remove from ThreadLocalEventLoop
            shutdown()
        }
    }

    final override fun limitedParallelism(parallelism: Int): CoroutineDispatcher {
        parallelism.checkParallelism()
        return this
    }

    open fun shutdown() {}
}

/*
 * This is marked with `ThreadLocal` on purpose: the loop itself is supposed to be thread-local,
 * but `CommonThreadLocal` is not supported on K/N properly, so we emulate it with this annotation.
 */
@ThreadLocal
internal object ThreadLocalEventLoop {
    private val ref = CommonThreadLocal<EventLoop?>()

    internal val eventLoop: EventLoop
        get() = ref.get() ?: createEventLoop().also { ref.set(it) }

    internal fun currentOrNull(): EventLoop? =
        ref.get()

    internal fun resetEventLoop() {
        ref.set(null)
    }

    internal fun setEventLoop(eventLoop: EventLoop) {
        ref.set(eventLoop)
    }
}

private val DISPOSED_TASK = Symbol("REMOVED_TASK")

// results for scheduleImpl
private const val SCHEDULE_OK = 0
private const val SCHEDULE_COMPLETED = 1
private const val SCHEDULE_DISPOSED = 2

Review comment (refactor):
`CommonThreadLocal` is quite a leaky abstraction then. Maybe we should fix it instead? Here's my attempt to do it: https://gist.github.com/dkhalanskyjb/7cc2d6c752d97217c6c288304253ba11

Code after:
        if (!unconfined) shared = true 
    }

    fun decrementUseCount(unconfined: Boolean = false) {
        useCount -= delta(unconfined)
        if (useCount > 0) return
        assert { useCount == 0L } // "Extra decrementUseCount"
        if (shared) {
            // shut it down and remove from ThreadLocalEventLoop
            shutdown()
        }
    }

    final override fun limitedParallelism(parallelism: Int): CoroutineDispatcher {
        parallelism.checkParallelism()
        return this
    }

    open fun shutdown() {}
}

internal object ThreadLocalEventLoop {
    private val ref = commonThreadLocal<EventLoop?>(Symbol("ThreadLocalEventLoop"))

    internal val eventLoop: EventLoop
        get() = ref.get() ?: createEventLoop().also { ref.set(it) }

    internal fun currentOrNull(): EventLoop? =
        ref.get()

    internal fun resetEventLoop() {
        ref.set(null)
    }

    internal fun setEventLoop(eventLoop: EventLoop) {
        ref.set(eventLoop)
    }
}

private val DISPOSED_TASK = Symbol("REMOVED_TASK")

// results for scheduleImpl
private const val SCHEDULE_OK = 0
private const val SCHEDULE_COMPLETED = 1
private const val SCHEDULE_DISPOSED = 2

private const val MS_TO_NS = 1_000_000L
private const val MAX_MS = Long.MAX_VALUE / MS_TO_NS

/**
 * First-line overflow protection -- limit maximal delay.