Move `FfiConverter` definition into the scaffolding code

[bendk]

This is something we've discussed a few times over the year I've been working on UniFFI, I think that maybe it's time to do it. The reason is to better support a macro-based interface definitions. Here's a rundown of the issue:

The FfiConverter trait allows us to generate the scaffolding code without knowledge of the entire ComponentInterface, but it gets tricky with user types.

We want to allow the exported interface to be split up over multiple Rust modules. We also want the proc-macros to be able to generate the scaffolding code. This is tricky because the proc-macro can't "see" the code in other modules when it generates the scaffolding. However, we can leverage the FfiConverter trait to make this work.

For example, suppose the definitions of Line and Point in the geometry crate were split out into the geometry.types module. Then in the main module, we needed to generate scaffolding functions that input/output those types.

Based on this code:

use types::Line;

#[uniffi::export]
fn gradient(ln: Line) -> f64 { ... }

We can (almost) generate a scaffolding function like this:

pub extern "C" fn geometry_eb69_gradient(
  ln: <Line as FfiConverter>::FfiType,
)  -> <f64 as uniffi::FfiConverter>::FfiType {
  <f64 as uniffi::FfiConverter>::lower(
      gradient(<Line as FfiConverter>::try_lift(ln))
}

It's pretty cool how we can generate this function without knowing how Lines are lowered or lifted. However, right now this code will fail because we don't implement FfiConverter for Line or any other user types.

For structs, records, objects we don't implement FfiConverter directly on the type itself. The reason we do this is FfiConverter is defined in the uniffi crate, the implementation lives in the library crate that we're generating scaffolding for, and some of our types live in a 3rd crate (for example SqlInterruptHandle). In these cases, the Rust orphan rule prevents the library crate from implementing FfiConverter on the type since both the type and trait are defined in other crates.

There are a few way to handle this, but my proposal is to change this so FfiConverter is defined by the generated scaffolding code, in the library crate.

How would FfiConverter work if it was defined in the scaffolding code?

If FfiConverter was defined in the scaffolding code, then we couldn't reference it from the uniffi crate. I think most things would continue to work fine though.

• We used to have generic functions that used FfiConverter as a trait bound in rustcalls.rs, but we don't anymore. So I don't think that would be an issue.
• We have FfiConverter implementations for builtin types in the uniffi crate. I think we could keep this shared code and just have it implement the BuiltinFfiConverter trait. This would be exactly the same as FfiConverter except it's defined in uniffi and only used for builtin types.
• A related issue would happen with external types. If you use an external type, then that type will implement the FfiConverter trait defined in it's crate. So we would be working with 2 traits named FfiConverter that are different. It's weird, but I can't see any serious issues with this.
• We wouldn't be able to use RustBufferFfiConverter in the scaffolding code, but that's just there for convenience. To work around this, we could either just implement FfiConverter directly by defining the extra functions or write a macro that does something similar.
• I think the most serious cost is the long-term commitment to never use FfiConverter in the uniffi crate itself. We could never add a function that was generic over FfiConverter like the old rustcalls code did. However, it seems like we could always shift things the same way we did the rustcalls code, have the generated scaffolding code do a bit more work and so we don't need to rely on the FfiConverter trait.

How would scaffolding generation work with this system?

It would look something like this:

pub extern "C" fn geometry_eb69_gradient(
  ln: <Line as FfiConverter>::FfiType,
)  -> <f64 as FfiConverter>::FfiType {
  <f64 as FfiConverter>::lower(
      gradient(<Line as FfiConverter>::try_lift(ln))
}

We'd need to do a couple things to ensure this would work:

• Implement FfiConverter for all types:
  • For user types, we can implement FfiConverter directly in the scaffolding code just like we do now.
  • For builtin types, we can trivially create a blanket impl<T: uniffi::BuiltinFfiConverter> FfiConverter for T impl
  • For external types we could trivially implement FfiConverter for the library using the FfiConverter implementation from the external crate. In order for this to work we'd need to know a type was external. We could require users to add an annotation like #[uniffi::external] to a use statement for the type.
• Define FfiConverter in the root module for the crate. This is needed to ensure that it's available in all modules without needing to import it. We also need this to be in a well-known location so that we can use the FfiConverter trait from external crates. I think this means that we would require users to call some sort of global macro in their lib.rs file. I think we want this for other reasons anyways, like defining up a static UniFFI version string to check for versioning mismatches.

┆Issue is synchronized with this Jira Task ┆Issue Number: UNIFFI-165

[jplatte]

Actually, I think we were talking a bit past each other 😅

I don't see how separate traits could work because the proc-macro can't know which crate a given named type comes from unless it's written as an absolute path.

In any case, this doesn't block the proc-macro work since it's easy to write out <ParamType as uniffi::BuiltinFfiConverter> by default and allow customization via something like #[uniffi::ffi_converter(CustomFfiConverter)].

What I originally wanted to suggest and IMO still makes sense ultimately is that this:

pub unsafe trait FfiConverter: Sized {
    type RustType;
    type FfiType;

    fn lower(obj: Self::RustType) -> Self::FfiType;
    // ...
}

struct MyFfiConverter;

impl FfiConverter for MyFfiConverter {
    type RustType = uuid::Uuid;
    type FfiType = String;

    fn lower(obj: Self::RustType) -> Self::FfiType { obj.to_string() }
    // ...
}

could be replaced by this:

pub unsafe trait FfiConvertible: Sized {
    type FfiType;

    fn lower(obj: Self) -> Self::FfiType;
    // ...
}

// repr(transparent) is not strictly necessary but allow allocation-free
// conversion of Uuid to/from MyUuid inside containers like Box<_>, Arc<_>,
// Vec<_> and such by using into_raw, from_raw and similar functions
#[repr(transparent)]
struct MyUuid(uuid::Uuid);

impl FfiConverter for MyFfiConverter {
    type FfiType = String;

    fn lower(obj: Self) -> Self::FfiType { obj.0.to_string() }
    // ...
}

[bendk]

I think I actually confused myself. The issue you bring up is very real, but I think there's a solution to it. I just updated the proposal to reflect that.

There's definitely other ways to handle this and I think your proposal is the main alternative. I think that would work great for:

• External types in crates we control. Since we control the crate, we could add a uniffi annotation to that type that implemented FfiConverter.
• External types in crates we don't control where just call methods. In this case we could have a Deref impl and things would be fairly ergonomic.

The one issue is external types in crates we don't control, where we want to access the data. Like if you wanted to use num_complex::Complex. Then I think you would have to write things like num.0.re a lot which is not so nice.

That said, I doubt those types are used very much right now so maybe it's fine to require users to wrap them in a newtype. Now that I write out the proposal I realize there's a good deal of complexity there. Maybe it's better to avoid that complexity than to continue to support external types in the same way.

[bendk]

After thinking about this some more, I think there's a better way to handle this:

• Keep the FfiConverter trait the same
• Require proc-macro users to call a global UniFFI macro at the top-level of their crate (something like uniffi_setup!()). This would:
  • Define struct UniFFIWrapper<T>(T)
  • Define a blanket impl like impl<T: FFIConverter> FFIConverter for UniFFIWrapper<T>. I'm pretty sure this doesn't overlap with the current FFIConverterRustBuffer blanket impl and even if it does I think there's a workaround.
• For local types implement FFIConverter on that type. Because of the blanket impl above, this means UniFFIWrapper<T> also implements FFIConverter<RustType=T>
• This also works for foreign types in crates we control. Wrap the type in a proc-macro that implements FFIConverter on it and we get a FFIConverter<RustType=ForeignType> for free.
• For foreign types in crates we don't control, duplicate the type definition in the UniFFI crate, wrapped in a proc macro. That proc macro implements FFIConverter<RustType=ForeignType> on UniFFIWrapper<ForeignType>
• All of this means that for any type, <UniFFIWrapper<T> as FFIConverter> can be used by the scaffolding code to handle the lifting/lowering.

I think this gives the same advantages without some of the drawbacks:

• No need to annotate the use statement for foreign types in crates we control
• The uniffi crate can use the FFIConverter trait if it wants
• Can continue to use RustBufferFfiConverter just like before

[jplatte]

To be honest, that seems very complex and I don't really understand which part of it addresses the problems I've had FfiConverter in the proc-macro code.

I would like to try the simplest approach first, i.e. what I posted over there, w/o extra uniffi_setup!() or anything like that. It shouldn't take very long, the interesting bit will be how it integrates with existing more complex uniffi usage (but I think that will also be solveable in an elegant way).

[bendk]

Go for it. I think you're idea is a good starting point and I'm still not convinced that my proposal is worth the added complexity.

[bendk]

Maybe I should clarify what I mean by "Go for it".

• I'm pretty sure we currently implement FFIConverter on the type itself for all types UniFFI supports. If not, I think you should feel free to open a PR against the uniffi code.
• I think the change would be to also implement FFIConverter on the type itself in the scaffolding code for user types. I think this addresses your issues, since for any type you can do <T as FFIConverter> to access the impl.
• For now at least, can you keep the trait definition the same? Maybe we'll eliminate the RustType associated type, but I don't think we need to do it yet.

[jplatte]

I was thinking of removing RustType in favor of Self but you're right, that's not so important for now.

[jplatte]

Hm, actually the code currently generated for [Error] UDL works even if the error type is defined in another crate from the one that includes the scaffolding code. Maybe the better way forward is to require #[uniffi::export]ed functions to use an error type that's annotated with a proc-macro too. This makes mixing UDL and proc-macros harder than it would be ideally, but seems like the cleaner path forward.