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

Adding support for Rust async trait methods #1981

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

bendk
Copy link
Contributor

@bendk bendk commented Feb 6, 2024

This was pretty easy, it mostly meant not hard coding that we don't support async methods and which order to apply #[uniffi::export] vs #[async_trait::async_trait].

The next step is foreign-implemented traits.

@bendk bendk requested a review from mhammond February 6, 2024 22:25
@bendk bendk requested a review from a team as a code owner February 6, 2024 22:25
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.

I love how tiny the actual change is! Also, this needs a CHANGELOG entry.

};
```

When using proc-macros, make sure to put `#[uniffi::export]` outside the `#[async_trait]` attribute:
Copy link
Member

Choose a reason for hiding this comment

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

Is it strictly necessary for it to be "outside the #[async_trait] attribute"? I thought it would work if those 2 lines were swapped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure exactly why, but it fails because of the lifetimes async_trait adds:

    |                  - lifetime `'async_trait` is missing in item created through this procedural macro
332 | pub trait SayAfterTrait: Send + Sync {
333 |     async fn say_after(&self, ms: u16, who: String) -> String;
    |     ^^^^^ undeclared lifetime

This was pretty easy, it mostly meant not hard coding that we don't
support async methods and which order to apply `#[uniffi::export]` vs
`#[async_trait::async_trait]`.

The next step is foreign-implemented traits.
@bendk bendk force-pushed the async-rust-trait-interfaces branch from 7aa00f2 to 81893ca Compare February 7, 2024 21:51
@bendk bendk merged commit 49becea into mozilla:main Feb 7, 2024
0 of 4 checks passed
@shaiguelman
Copy link

Do you have an estimate of when the next release will be? My team is strongly dependent on this change

@bendk
Copy link
Contributor Author

bendk commented Feb 14, 2024

No estimate, but you can try to use the git commit in to get the feature. Other teams have had success with that.

@shaiguelman
Copy link

Thanks, that did work

mgeisler added a commit to mgeisler/mls-rs that referenced this pull request Mar 22, 2024
UniFFI added support for async traits in mozilla/uniffi-rs#1981, but
this is not yet released. This commit temporarily introduces a Git
dependency on UniFFI to let us test the async functionality which was
removed in awslabs#86.

An unrelated UniFFI change (mozilla/uniffi-rs#1840) made our existing
test fail.
mgeisler added a commit to mgeisler/mls-rs that referenced this pull request Mar 22, 2024
UniFFI added support for async traits in mozilla/uniffi-rs#1981, but
this is not yet released. This commit temporarily introduces a Git
dependency on UniFFI to let us test the async functionality which was
removed in awslabs#86.

An unrelated UniFFI change (mozilla/uniffi-rs#1840) made our existing
test fail.
mgeisler added a commit to mgeisler/mls-rs that referenced this pull request Mar 23, 2024
UniFFI added support for async traits in mozilla/uniffi-rs#1981, but
this is not yet released. This commit temporarily introduces a Git
dependency on UniFFI to let us test the async functionality which was
removed in awslabs#86.

An unrelated UniFFI change (mozilla/uniffi-rs#1840) made our existing
test fail.
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.

3 participants