Remote types: interfaces and proc macros ADR

[bendk]

@mhammond raised this question when we were discussing https://github.com/mozilla/uniffi-rs/pull/2087 and I think it's a very good one. Let's discuss and document our decision here.

@jplatte @Sajjon flagging you here since you were involved in that discussion. If you have opinions, I'd love to hear them.

[bendk]

When I was writing the code for #2087, I started with option 2 but switched to option 1a. The main issue was that I had a real hard time trying to document and explain the differences. I had a hard time just saying, "We use anyhow::Error in Rust, but AnyhowError on the foreign side. to_string here, but message over there." and not explaining more. But I also didn't want to get into the orphan rule and why there was the distinction either. Somehow option 1a seemed easier to explain. I think option 1 is an improvement over that, since it lessens the main drawback: users may see a semi-valid item and think that code is actually going to be present in the final instance. That said, that remote_extend macro does feel a magical to me.

[Sajjon]

Thanks for pinging! I will review tonight (Stockholm, Sweden, i.e. in a couple of hours) or tomorrow!

[Sajjon]

@bendk I made a tiny PR making the code in the ADR easier to read with highlighting: https://github.com/bendk/uniffi-rs/pull/4

[Sajjon]

@bendk Nice ADR! I really like Option 1 (and 1a) - I think Option 2 is quite "expensive" in in boilerplate cost. I have recently wrapped a fairly big external crate in its entirety, resulting in 78 From implementations and 70 type implementations, making up many thousand lines of code, quite similar to Option 2.

So Option 1 seem like a huge advantages over Option 2. It really boils down to "how the solution scales" IMO, being able to skip duplication of types with from / into implementations.

I like Option 1a best!

(My answer was mostly focused on Records / Enums, not so much on interfaces since I have not been using remote interfaces so much with UniFFI so far.)

[jplatte]

I also like 1a best, but there's one bit about it that I really don't like, which is the empty function bodies. It would require a little bit of hacking around with the parsing, but I think it shouldn't be too hard to make a custom parser for #[uniffi::remote]s impl input such that it can parse both functions with a body (which would then end up in an extension trait and be exported) and functions with a semicolon after the declaration (which would just export an existing method).

It would be perfectly fine to initially only support the form with a semicolon, such that users would have to write the function signature (or thrice, if not using a macro like extend::ext) in the case of a third-party method.

Future possibility: There could also be a way of declaring a method for FFI only, i.e. a method that can't be invoked as a Rust method, but can be invoked from foreign code as a method. I think this would be useful for "overriding" the signature of an existing method for usage from foreign languages, where the impl would then likely forward to the Rust method of the same name. This would look weird in source, but that's already a thing in regular Rust code¹ and would be somewhat of a niche use case anyways I imagine.

¹ when you have a trait method that's implemented in terms of an inherent method of the same name

[jplatte]

One more thing though, how would we deal with destructors? Right now, #[derive(uniffi::Object)] exists to generate the d'tor scaffolding. I think it would be good to have something similar for remote objects, maybe just a #[uniffi::remote_object] macro that can be used on use statements or type aliases. The alernative of generating the d'tor scaffolding on #[uniffi::remote] impl blocks would make it impossible to have multiple such impl blocks for one type. Probably not a serious limitation, but a weird inconsistency IMHO.

[mhammond]

What I like about option 1 is that:

type LogLevel = log::Level;

uniffi::remote!(
    pub enum LogLevel {
        Error = 1,
    }
);

looks "special" - because you aren't actually defining the enum, you are feeding uniffi info about it

what I don't really like about:

type LogLevel = log::Level;

#[uniffi::remote]
pub enum LogLevel {
    Error = 1,
}

is that because it's more natural it's more head-scratching for the casual reader.

Ditto the empty function bodies - trying to look natural actually looks odd. But:

I also like 1a best, but there's one bit about it that I really don't like, which is the empty function bodies. It would require a little bit of hacking around with the parsing, but [it could be the actual impl]}

would be fantastic - and would argue more for 1a, for interfaces at least.

[mhammond]

it seems unlikely we'd ever try and support record objects, right? This would be used by only enums and interfaces?

[bendk]

I also like 1a best, but there's one bit about it that I really don't like, which is the empty function bodies. It would require a little bit of hacking around with the parsing, but [it could be the actual impl]}

This suggestion also seems great to me.

it seems unlikely we'd ever try and support record objects, right? This would be used by only enums and interfaces?

I was thinking we would, just because it seems easy and it's currently supported for UDL. I can't think of an instance of this being used in the wild though, so I'd be open to not supporting it initially.

[mhammond]

One more thing though, how would we deal with destructors? Right now, #[derive(uniffi::Object)] exists to generate the d'tor scaffolding.

I think that's just generating the ffi for the the generic Arc<> release/clones? We've some tests for UDL - interfaces are explicitly wrapped in Arcs when used over the exposed api (in ext-types/lib).

I think it would be good to have something similar for remote objects, maybe just a #[uniffi::remote_object] macro that can be used on use statements or type aliases. The alernative of generating the d'tor scaffolding on #[uniffi::remote] impl blocks would make it impossible to have multiple such impl blocks for one type. Probably not a serious limitation, but a weird inconsistency IMHO.

I don't quite get what you mean here - do you mind spelling that out a bit more for me?

[bendk]

I'm undecided on 1 vs 1a, but I really like @jplatte suggestion of a impl block where you can either specify the method block or not. I added that as 1b and it's currently my preferred option.

I guess there could also be a 1c where the same feature is spelled out with a function-style macro, but there doesn't seem to be much enthusiasm for that option so I didn't spell it out.

[bendk]

Pushing one more update:

• I swapped 1 and 1a since it seems like people like 1 better
• I went all-in on @jplatte's suggestion. Not only should we allow users to define method bodies, we also go with the FFI-only method proposal. Rather than generate extension trait, lets put it directly in the generated scaffolding. This feels much nicer to me, since in almost all cases users are only going to want to call these methods from the foreign language. For cases where they do want to call them from Rust, then they can define their own extension trait without any UniFFI magic.
• I moved the FFI method vs extension trait question to a discussion section at the bottom. I want to keep the number of sub-options low and have come to the point where I think there's one Right solution and the other one is just worse. Please push back on this if you feel differently though.

[bendk]

Looks like there's general agreement for 1 (formally 1a). I'm thinking that we can accept and merge this one soon. If you have concerns, please post them in the next day or two.

@Sajjon @jplatte I think you both expressed support for this, can I put you on as a decider? If so, just thumbs up this comment.