Skip to content

Introduce workaround (in fact, fix the previously incorrect expect-ac… #3850

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 20, 2023
Merged

Introduce workaround (in fact, fix the previously incorrect expect-ac… #3850

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion kotlinx-coroutines-core/common/src/Exceptions.common.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ public class CompletionHandlerException(message: String, cause: Throwable) : Run

public expect open class CancellationException(message: String?) : IllegalStateException

@Suppress("FunctionName", "NO_ACTUAL_FOR_EXPECT")
public expect fun CancellationException(message: String?, cause: Throwable?) : CancellationException

internal expect class JobCancellationException(
Expand Down
5 changes: 5 additions & 0 deletions kotlinx-coroutines-core/js/src/Exceptions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ package kotlinx.coroutines
*/
public actual typealias CancellationException = kotlin.coroutines.cancellation.CancellationException

@Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE")
@kotlin.internal.LowPriorityInOverloadResolution
public actual fun CancellationException(message: String?, cause: Throwable?): CancellationException =
CancellationException(message, cause)

/**
* Thrown by cancellable suspending functions if the [Job] of the coroutine is cancelled or completed
* without cause, or with a cause or exception that is not [CancellationException]
Expand Down
1 change: 0 additions & 1 deletion kotlinx-coroutines-core/jvm/src/Exceptions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ public actual typealias CancellationException = java.util.concurrent.Cancellatio
/**
* Creates a cancellation exception with a specified message and [cause].
*/
@Suppress("FunctionName")
public actual fun CancellationException(message: String?, cause: Throwable?) : CancellationException =
CancellationException(message).apply { initCause(cause) }

Expand Down
5 changes: 5 additions & 0 deletions kotlinx-coroutines-core/native/src/Exceptions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ import kotlinx.coroutines.internal.SuppressSupportingThrowableImpl
*/
public actual typealias CancellationException = kotlin.coroutines.cancellation.CancellationException

@Suppress("INVISIBLE_MEMBER", "INVISIBLE_REFERENCE")
@kotlin.internal.LowPriorityInOverloadResolution
public actual fun CancellationException(message: String?, cause: Throwable?): CancellationException =
CancellationException(message, cause)
Comment on lines +19 to +21
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this LowPriorityInOverloadResolution just to make this definition work, or is it also so that platform-specific code chooses the other overload?

  • If it's for the definition to work, won't a fully-qualified name work better? Like = kotlin.coroutines.cancellation.CancellationException(message, cause).
  • If it's for choosing the other overload, maybe we can ensure this also happens on K2 by marking this function as inline? Is this possible?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make the declaration work (otherwise, it's conflicting overloads on the declaration level) and, later, the choice of the overload from our/users' code.

maybe we can ensure this also happens on K2 by marking this function as inline?

It's a bit unclear to me what you've meant, could you please elaborate on that?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit unclear to me what you've meant, could you please elaborate on that?

If I remember correctly, with K2, in common code, the overload that gets chosen in the end is the common one, so in this case, the expect function. If that's undesirable, can't we mark this function as inline instead so its calls always get converted to the ones we want?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks.

the overload that gets chosen in the end is the common one

Unfortunately, it will be the case only for the code in kotlinx-coroutines-core sourceset and not for our users, so I don't think it worth it


/**
* Thrown by cancellable suspending functions if the [Job] of the coroutine is cancelled or completed
* without cause, or with a cause or exception that is not [CancellationException]
Expand Down