mozilla / uniffi-rs

a multi-language bindings generator for rust
https://mozilla.github.io/uniffi-rs/
Mozilla Public License 2.0
2.56k stars 217 forks source link

Invalid code generation when updating to v0.25.0 #1804

Closed ianthetechie closed 9 months ago

ianthetechie commented 9 months ago

I hit what appears to be a code generation issue when updating to v0.25.0.

Background

I used to have a callback interface, but it looks like these are soft deprecated. My code didn't compile anyways as it was when updating, so I tried to switch to the [Trait] attribute instead. Here is the updated definition:

[Trait]
interface RouteRequestGenerator {
    [Throws=RoutingRequestGenerationError]
    RouteRequest generate_request(UserLocation user_location, sequence<GeographicCoordinates> waypoints);
};

Errors

I end up with a few errors after making the switch, but all have basically the same form.

error[E0308]: mismatched types
   --> /Users/ianthetechie/ferrostar/common/target/debug/build/ferrostar-core-2d66daf5c6114baa/out/ferrostar.uniffi.rs:433:1
    |
433 | #[::uniffi::export_for_udl]
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `RouteRequest`, found `Result<RouteRequest, ...>`
    |
    = note: expected enum `RouteRequest`
               found enum `Result<RouteRequest, RoutingRequestGenerationError>`
    = note: this error originates in the attribute macro `::uniffi::export_for_udl` (in Nightly builds, run with -Z macro-backtrace for more info)

There is some mismatch in the generation phase. This method is annotated with [Throws] so it should return a result. Here is the trait definition in Rust:

pub trait RouteRequestGenerator: Send + Sync + Debug {
    fn generate_request(
        &self,
        user_location: UserLocation,
        waypoints: Vec<GeographicCoordinates>,
    ) -> Result<RouteRequest, RoutingRequestGenerationError>;
}

The same is true for both callback traits but I think the other case is likely the exact same root cause.

If it's helpful, here is the commit showing exactly what changed while upgrading from v0.24.3 to v0.25.0: https://github.com/stadiamaps/ferrostar/commit/ef393cc2422b03486802d6f8f8a7b4b3ee8911ec.

And similarly, here are links to the trait definitions which didn't change.

ianthetechie commented 9 months ago

Update-ish: I'm actually really confused now about the status of callback interfaces.

https://mozilla.github.io/uniffi-rs/udl/callback_interfaces.html says they are soft deprecated and that you shouldn't use them. But it doesn't actually say what the alternative is. For some reason I thought just adding the [Trait] attribute was enough, but this doesn't seem to be correct.

There is code demonstrating the use of callback interfaces here in the UniFFI codebase, but the docs linked above say not to use them with proc macros. https://github.com/mozilla/uniffi-rs/blob/main/fixtures/proc-macro/src/callback_interface.rs

The proc macro docs don't have such a warning.

TL;DR in addition to the original issue of not being able to get my code working after the upgrade, is UniFFI dropping support for foreign implementations of traits? If so, that would be very sad, since this is an extremely useful feature.

mhammond commented 9 months ago

Update-ish: I'm actually really confused now about the status of callback interfaces.

https://mozilla.github.io/uniffi-rs/udl/callback_interfaces.html says they are soft deprecated and that you shouldn't use them. But it doesn't actually say what the alternative is.

I'll try and fix these docs, but just above it says "Callback interfaces are a special implementation of Rust traits implemented by foreign languages" - so the intent was to indicate "you should use the non-special implementation of Rust traits implemented by foreign languages". The primary difference between the 2 is the signature used by Rust - callbacks using just a Box<> limits their utility.

is UniFFI dropping support for foreign implementations of traits? If so, that would be very sad, since this is an extremely useful feature.

No - the general case of foreign implementations of traits is a very new feature - callbacks are a special case of them as it's very difficult to have a Rust implementation of traits backing callback interfaces exposed via UDL.

See https://github.com/mozilla/uniffi-rs/blob/main/examples/callbacks/src/lib.rs - SimCard is a new capability - the general ability to have foreign implementations of traits. CallAnswerer is the old-style callback interfaces where it's (IIUC) impossible to have a Rust implementation of that trait used in the UDL.

mhammond commented 9 months ago

In this specific case, UniFFI is generating:

#[::uniffi::export_for_udl]
pub trait r#RouteRequestGenerator {
    fn generate_request(
        &self,
        user_location: r#UserLocation,
        waypoints: std::vec::Vec<r#GeographicCoordinates>,
    ) -> r#RouteRequest;

}

So for reasons I don't understand, it doesn't seem to think that method returns a result even though it clearly is specified that it does in the UDL.

bendk commented 9 months ago

Sorry about this one, I think may have mislead you on Matrix.

The feature to allow foreign types to implement trait interfaces was merged just after the 0.25.0 release. I wanted to give it some time in main before releasing it.

I think the other error you're seeing is that callback interfaces can't be returned from Rust. This is one of the limitations that trait interfaces were created to overcome. Did that work with previous UniFFI versions for you?

I think your options are:

bendk commented 9 months ago

The saga continues... that branch was just using trait interfaces implemented in Rust, so there shouldn't have been an issue. I just opened https://github.com/mozilla/uniffi-rs/pull/1812 which should fix things. I think we're going to get an 0.25.1 release out soon that fixes this one.

ianthetechie commented 9 months ago

Thanks a bunch! It sounds like you found everything you needed to solve from my repo but if not give a shout :)And thanks a ton for what you guys are doing with this project. It’s freakin awesome and is getting even better with the proc macros. I’m actually working on a blog post series and two conference talks in which UniFFI will feature :) And I’m quite happy that as a result of my procrastination on the blogs, I get to rip out most of the UDL section ;)10/28/23 02:04, bendk @.***> 작성: The saga continues... that branch was just using trait interfaces implemented in Rust, so there shouldn't have been an issue. I just opened #1812 which should fix things. I think we're going to get an 0.25.1 release out soon that fixes this one.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

ianthetechie commented 9 months ago

Good news!

TL;DR for googlers, replace callback interfaces with the [Trait] attribute on a regular interface, and point at main or 46c06003261c9ce769afff735f8d29d4282f445c until v0.25.1 is released.

  1. Per #1812, I was able to pin to that commit (46c06003261c9ce769afff735f8d29d4282f445c) in our Cargo.toml and that seems to have fixed everything (still following the general Trait path), at least on iOS. I can't comment on Android since we haven't gotten as far in our implementation yet to be at the level of catching obscure regressions in UniFFI ;)
  2. The new proc macros are the freakin' bomb! I was seriously thinking to myself "gee this should be a macro" last week before checking the latest changes 😂 Now that I've got things sorted on that "prerelease" commit, I was able to nuke 100% of our boilerplate UDL, and amusingly that even made some further idiomatic improvements like replacing some [UInt8] types in the Swift interfaces with Data (which is exactly what it should be).
  3. Extremely minor, but I found a few additional docs improvements, but will open separate issues for those.

Again, many thanks to both of you for such quick replies. That really made my weekend a lot more enjoyable since weekends are my main time for working on this project.

I'll mark this issue as closed now since it seems to be working in the latest code.