Skip to content

Comments

Kotlin: define constructors that throw for async functions#2813

Open
bendk wants to merge 1 commit intomozilla:mainfrom
bendk:push-kwylwvxvtlxy
Open

Kotlin: define constructors that throw for async functions#2813
bendk wants to merge 1 commit intomozilla:mainfrom
bendk:push-kwylwvxvtlxy

Conversation

@bendk
Copy link
Contributor

@bendk bendk commented Feb 6, 2026

These constructors always just throw an exception, since Kotlin doesn't supprot async constructors. This is less confusing for users than just skipping the codegen (#2804).

@bendk bendk requested a review from a team as a code owner February 6, 2026 14:55
@bendk bendk requested review from badboy and removed request for a team February 6, 2026 14:55
These constructors always just throw an exception, since Kotlin doesn't
supprot async constructors.  This is less confusing for users than just skipping the
codegen (mozilla#2804).
@bendk bendk requested a review from a team February 6, 2026 14:55
@paxbun
Copy link
Collaborator

paxbun commented Feb 6, 2026

How about adding @Deprecated or opt-in annotations to such constructors? With this, we can notify users at compile time that async constructors are not supported in Kotlin as well.

@bendk
Copy link
Contributor Author

bendk commented Feb 6, 2026

In #2804 we discussed making the codegen fail at build time, however I went with this approach for a couple reasons:

  • It matches how we handle things in Python
  • I could see a use-case for a library that defined an async constructor knowing that it only worked on some bindings and providing an alternative method for other bindings. This is how fixtures/futures works actually.

I'm not totally sure this is the right way though, a build-time failure would be nicer in many cases compared to generating code that's just going to throw when you use it. However, that second bullet feels important to me. Maybe we could extend the language-specific config code that allows function/method renaming to also allow skipping codegen for functions/methods. That would give those users a decent path forward: they just need to add the constructor to that skip configuration.

@paxbun
Copy link
Collaborator

paxbun commented Feb 6, 2026

I'm not sure I understood the comment correctly, but @Deprecated will just warn the users, not cause compilation errors. If the users know this async constructor behavior, they can explicitly suppress the deprecation using the @Suppress annotation.

@bendk
Copy link
Contributor Author

bendk commented Feb 6, 2026

My comment wasn't a response to yours, we just happened to post at the same time. It was more me thinking out-loud on other ways to handle this.

Adding @Deprecated would be okay with me. I think that might be an improvement over the current code. However, I'm still wondering if a build error at codegen time would be the best behavior.

@paxbun
Copy link
Collaborator

paxbun commented Feb 6, 2026

I think this will be confusing to many users, but we can simulate async constructors in Kotlin by adding suspend operator fun invoke(...) to the companion object. I've never seen an actual production-level code that does this, though.

class Foo private constructor(public val v: String) {
  override fun toString() = "Foo($v)"
  companion object {
    suspend operator fun invoke(v: Int): Foo {
      return Foo(v.toString())
    }
  }
}

fun main() = runBlocking {
  println(Foo(15)) // invoking Foo.Companion.invoke(15)
}

@paxbun
Copy link
Collaborator

paxbun commented Feb 6, 2026

https://github.com/search?q=%22suspend+operator+fun+invoke%22+%22companion+object%22+language%3AKotlin&type=code

image

There seem to be several libraries that actually do this. Maybe it would be okay to implement async constructors like this.

@pronebird
Copy link

pronebird commented Feb 6, 2026

If I understand it right this will result in runtime error. I am not particularly fond of this solution since CI checks will pass and give a false sense of safety.

Rust supports conditional compilation so it's possible to handle differences for android/ios by providing alternative constructors. As such build error isn't a problem but would help catch issues early on during CI pass

bendk added a commit to bendk/uniffi-rs that referenced this pull request Feb 13, 2026
The motivation for this is avoiding async constructors
(mozilla#2813).

We'd like things to fail at build time if you have an async constructor
and are generating bindings for a language that doesn't support them.
However, this prevents a valid use case: I want the async constructor on
languages that do support them, but not on the others.

This change allows us to support that.  Users would need to exclude the
constructor on languages that don't support it.

Actually, now that think about it I don't think any built-in language
supports async constructors, but maybe external binding generators could
or this could be useful for some other feature.
bendk added a commit to bendk/uniffi-rs that referenced this pull request Feb 13, 2026
The motivation for this is avoiding async constructors
(mozilla#2813).

We'd like things to fail at build time if you have an async constructor
and are generating bindings for a language that doesn't support them.
However, this prevents a valid use case: I want the async constructor on
languages that do support them, but not on the others.

This change allows us to support that.  Users would need to exclude the
constructor on languages that don't support it.

Actually, now that think about it I don't think any built-in language
supports async constructors, but maybe external binding generators could
or this could be useful for some other feature.
bendk added a commit to bendk/uniffi-rs that referenced this pull request Feb 13, 2026
The motivation for this is avoiding async constructors
(mozilla#2813).

We'd like things to fail at build time if you have an async constructor
and are generating bindings for a language that doesn't support them.
However, this prevents a valid use case: I want the async constructor on
languages that do support them, but not on the others.

This change allows us to support that.  Users would need to exclude the
constructor on languages that don't support it.

Actually, now that think about it I don't think any built-in language
supports async constructors, but maybe external binding generators could
or this could be useful for some other feature.
bendk added a commit to bendk/uniffi-rs that referenced this pull request Feb 13, 2026
The motivation for this is avoiding async constructors
(mozilla#2813).

We'd like things to fail at build time if you have an async constructor
and are generating bindings for a language that doesn't support them.
However, this prevents a valid use case: I want the async constructor on
languages that do support them but not on ones that don't.

This change allows us to support that by allowing users to exclude the
constructor on languages that don't support it.

Actually, now that think about it I don't think any built-in language
supports async constructors, but maybe external binding generators could
or this could be useful for some other feature.
@bendk
Copy link
Contributor Author

bendk commented Feb 13, 2026

If I understand it right this will result in runtime error. I am not particularly fond of this solution since CI checks will pass and give a false sense of safety.

I agree with this and made #2822 as a way to move us towards build-time errors.

Copy link
Member

@mhammond mhammond left a comment

Choose a reason for hiding this comment

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

IIUC, you don't want to do this now, but would rather throw at bindgen time?

constructor({% call kt::arg_list(cons, true) -%}) {
throw InternalException("async primary constructors are not supported on Kotlin");
}
// Note no constructor generated for this object as it is async.
Copy link
Member

Choose a reason for hiding this comment

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

this comment is now wrong?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants