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

External trait interface isn't compiled (UDL mode) #1831

Closed
Voronar opened this issue Nov 3, 2023 · 9 comments · Fixed by #1867
Closed

External trait interface isn't compiled (UDL mode) #1831

Voronar opened this issue Nov 3, 2023 · 9 comments · Fixed by #1867

Comments

@Voronar
Copy link

Voronar commented Nov 3, 2023

I'm trying to reuse uniffi trait interface (https://mozilla.github.io/uniffi-rs/foreign_traits.html) from an external crate living in my own workspace.

main_lib:

use std::sync::Arc;
use shared_lib::*;

pub fn use_common_trait(common_trait: Arc<dyn CommonTrait>) {
    common_trait.foo();
}

uniffi::include_scaffolding!("main_lib");

main_lib UDL:

namespace main_lib {
  void use_common_trait(CommonTrait common_trait);
};

[ExternalInterface="shared_lib"]
typedef extern CommonTrait;

shared_lib:

pub trait CommonTrait: Send + Sync {
    fn foo(&self);
}

uniffi::include_scaffolding!("shared_lib");

shared_lib UDL:

namespace shared_lib {};

[Trait]
interface CommonTrait {
    void foo();
};

Error:

error[E0782]: trait objects must include the `dyn` keyword
  --> uniffi_external_trait_interface/target/debug/build/main_lib-5098ef40e334e5d0/out/main_lib.uniffi.rs:34:38
   |
34 |     r#common_trait: ::std::sync::Arc<r#CommonTrait>,
   |                                      ^^^^^^^^^^^^^
   |
help: add `dyn` keyword before this trait
   |
34 |     r#common_trait: ::std::sync::Arc<dyn r#CommonTrait>,
   |                                      +++

error[E0782]: trait objects must include the `dyn` keyword
  --> uniffi_external_trait_interface/target/debug/build/main_lib-5098ef40e334e5d0/out/main_lib.uniffi.rs:53:38
   |
53 | ::uniffi::ffi_converter_arc_forward!(r#CommonTrait, ::r#shared_lib::UniFfiTag, crate::UniFfiTag);
   |                                      ^^^^^^^^^^^^^
   |
help: add `dyn` keyword before this trait
   |
53 | ::uniffi::ffi_converter_arc_forward!(dyn r#CommonTrait, ::r#shared_lib::UniFfiTag, crate::UniFfiTag);

Reproducible example: https://github.com/Voronar/uniffi_external_trait_interface
uniffi version: 0.25

@Voronar Voronar changed the title External trait interface doesn't compile External trait interface isn't compiled Nov 3, 2023
@Voronar Voronar changed the title External trait interface isn't compiled External trait interface isn't compiled (UDL mode) Nov 3, 2023
@badboy
Copy link
Member

badboy commented Nov 3, 2023

Can you provide the actual error you're running into?

@Voronar
Copy link
Author

Voronar commented Nov 3, 2023

As we see the dyn somehow isn't generated during codegen phase in case of external trait interface type.

@Voronar
Copy link
Author

Voronar commented Nov 3, 2023

Maybe the problem lies here https://github.com/mozilla/uniffi-rs/blob/main/uniffi_bindgen/src/scaffolding/mod.rs#L66. In case of the external type it generates just Arc<T> without dyn modifier.

@mhammond
Copy link
Member

mhammond commented Nov 3, 2023

yeah - the problem here is that the trait is "external". I don't think an external type even knows if it is a trait or not - it probably needs to though.

@mhammond
Copy link
Member

mhammond commented Nov 4, 2023

this should actually work ok using procmacros I think. There are a few options for UDL though, but all have some work.

@Voronar
Copy link
Author

Voronar commented Nov 6, 2023

It compiles with proc-macros, but crashes at runtime for example with Kotlin script test:

thread '<unnamed>' panicked at /Users/user/.cargo/git/checkouts/uniffi-rs-6f89edd2a1ffa4bd/659d5d6/uniffi_core/src/ffi/foreigncallbacks.rs:102:1:
Bug: callback not set.  This is likely a uniffi bug.

Reproducible example: https://github.com/Voronar/uniffi_external_trait_interface/tree/external_trait_proc_macro_kotlin_script_crash

@mhammond
Copy link
Member

The problem here is still UDL - main_lib's UDL can't reference the trait - so if you remove the trait and function from the UDL and use #[uniffi::export] on the function in lib.rs things seem to work. It would be possible to make UDL work here but it would probably require a contributor to implement it.

@mhammond
Copy link
Member

See also #1854 and #1855

@Voronar
Copy link
Author

Voronar commented Nov 20, 2023

The problem here is still UDL...

I made reproducible example (mentioned in my previous message) without any UDL and it crashes at runtime (at least on my machine).

Reproducible example: https://github.com/Voronar/uniffi_external_trait_interface/tree/external_trait_proc_macro_kotlin_script_crash

bendk pushed a commit to bendk/uniffi-rs that referenced this issue Nov 28, 2023
Fixes mozilla#1831

Also fixes naming problem bindings with external types with
unusual names.
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 a pull request may close this issue.

3 participants