Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.9.0-RC: kotlinx-coroutines-core/concurrent/src/... contains common (expect) and platform (actual) files #4163

Open
nreid260 opened this issue Jun 29, 2024 · 4 comments

Comments

@nreid260
Copy link

nreid260 commented Jun 29, 2024

What broke?

Our current build setup globs all of kotlinx-coroutines-core/common/src/... and kotlinx-coroutines-core/concurrent/src/... to pass as -Xcommon-srcs. When we do this with -language-version=2.0, which is stricter about expect/actual declarations, there's a EXPECT_AND_ACTUAL_IN_THE_SAME_MODULE error for LockFreeLinkedListNode.

The problem is that there are some files from kotlinx-coroutines-core/concurrent/src/... that must be -Xcommon-srcs (expects) and some that must not be (actuals). These files shouldn't be mixed together.

Ideally, all expect files would be under a single root, such askotlinx-coroutines-core/common/src/....

PS: What naming convention is Builders.concurrent.kt? It looks like someone wanted Builders.common.kt.

Did I check that setting the version to the latest stable release fixes the problem?

Yes.

@nreid260 nreid260 added the bug label Jun 29, 2024
@globsterg
Copy link
Contributor

The problem is that there are some files from kotlinx-coroutines-core/concurrent/src/... that must be -Xcommon-srcs (expects) and some that must not be (actuals). These files shouldn't be mixed together.

Could you link to some style guide that mandates this? I could not find this requirement anywhere. When I have deep hierarchies of source sets, I freely mix expect and actual together in the intermediate source sets. Am I wrong to do that?

PS: What naming convention is Builders.concurrent.kt? It looks like someone wanted Builders.common.kt.

Yeah, seems strange. The authors probably wanted $fileName.$sourceSetName.kt, but ./kotlinx-coroutines-core/concurrent/src/MultithreadedDispatchers.common.kt violates this scheme. Would also be interested to see any style guide that explains how these names are chosen (I'm generating Kotlin code and would very much like for it to be idiomatic).

@nreid260
Copy link
Author

nreid260 commented Jul 1, 2024

An alternative solution is simply to remove the actual keyword from kotlinx-coroutines-core/concurrent/src/internal/LockFreeLinkedList.kt, which is probably what's intended. The code in that is already platform-agnostic. There's no need for expect/actual.

@dkhalanskyjb
Copy link
Collaborator

The problem is that there are some files from kotlinx-coroutines-core/concurrent/src/... that must be -Xcommon-srcs (expects) and some that must not be (actuals). These files shouldn't be mixed together.

It's natural to establish some expect declarations for different platforms to fulfill, but also to implement actual definitions for the whole group of platforms.

there's a EXPECT_AND_ACTUAL_IN_THE_SAME_MODULE error for LockFreeLinkedListNode.

Maybe that's some compiler bug. The stated reason for this error is to forbid implementing something in the same module where the expect was declared: https://youtrack.jetbrains.com/issue/KT-55177/Deprecate-declaration-of-expect-and-actual-counterparts-of-same-class-in-one-module . We don't do that: LockFreeLinkedListNode is declared as expect in the common module and then is implemented in the concurrent and the jsAndWasmShared modules separately.

The code in that is already platform-agnostic. There's no need for expect/actual.

JS and Wasm have a separate implementation.

@mankyd
Copy link

mankyd commented Sep 27, 2024

I am running into this problem as well.

My build system uses kotlinc (i.e. not gradle) and takes in build.xml files via the -Xbuild-file flag. I have been unable to create a build.xml file that can compile this kotlinx.coroutines.

My build file looks something like:

<modules>
  <module name=”kotlinx_coroutines” type=”java-production” outputDir=”kotlinx_coroutines/kotlinc/classes”>
    <classpath path=”things.jar” />
    <!-- repeats various dependencies -->

    <sources path=”external/kotlinx.coroutines/kotlinx-coroutines-core/jvm/src/AbstractTimeSource.kt” />
    <!-- repeats for jvm sources -->

    <sources path=”external/kotlinx.coroutines/kotlinx-coroutines-core/common/src/Annotations.kt” />
    <commonSources path=”external/kotlinx.coroutines/kotlinx-coroutines-core/common/src/Annotations.kt” />
    <!-- repeats for common sources -->

    <sources path=”external/kotlinx.coroutines/kotlinx-coroutines-core/concurrent/src/Builders.concurrent.kt” />
    <commonSources path=”external/kotlinx.coroutines/kotlinx-coroutines-core/concurrent/src/Builders.concurrent.kt” />
    <!-- repeats for concurrent sources -->

  </module>
</modules>

The error:

external/kotlinx.coroutines/kotlinx-coroutines-core/concurrent/src/internal/LockFreeLinkedList.kt:32:26: error: lockFreeLinkedListNode: expect and corresponding actual are declared in the same module
public actual open class LockFreeLinkedListNode {
                         ^^^^^^^^^^^^^^^^^^^^^^
external/kotlinx.coroutines/kotlinx-coroutines-core/concurrent/src/internal/LockFreeLinkedList.kt:38:9: error: unresolved reference '_removedRef'.
        _removedRef.value ?: Removed(this).also { _removedRef.lazySet(it) }
...

I have tried moving various things around in the build file, but to no avail. Is it possible to build this using kotilnc 2.0.20 and a build.xml file?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants