Support references in foreign traits somehow

[mhammond]

Discussed a little with @linabutler and @bendk about this papercut. It exists for functions too, but seems more of a PITA for traits.

Eg, you can't use this as a foreign trait

55 | #[uniffi::export(with_foreign)]
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   | |
   | expected `&str`, found `String`
   | help: change the parameter type to match the trait: `&str`

which seems unfortunate, even though it's tricky. I can't see an issue for this, so here's one!

[linabutler]

I promised I'd file an issue about this...and didn't, oops! 😬 Thanks for starting the discussion!

References in traits (and functions) feel tricky: they're definitely more ergonomic for Rust callers, but they hide a "surprise copy" for foreign callers. I think most folks would expect that an &T argument would avoid a copy, but the scaffolding actually creates an owned T, passes &T as an argument, and then destroys T.

(If your function or trait method wanted to conditionally take ownership of the &T, it would need to do another copy, leading to a weird incentive for such a function to take an owned T instead of &T).

I wonder if we should think about:

1. Disallowing &T arguments in UniFFI-exported functions and traits, but...
2. Allowing Cow<'_, T> arguments. (I also really liked @bendk's suggestion to allow impl Into<Cow<'_, T>>).

That way, the scaffolding could always pass a Cow::Owned(T), and a Rust caller could pass a Cow::Borrowed(T) or a Cow::Owned(T). I think this would work for foreign traits, too: a trait method can take a Cow<'_, T>, and the scaffolding code that calls into the actual foreign implementation can take ownership of the Cow via into_owned().

Cow doesn't have a blanket From<&'a T> implementation, but it does implement From for owned and borrowed strings, paths, and slices, so a Rust caller would be able to write, say, func("foo".into()) instead of func(Cow::Borrowed("foo")).

I'm not sure that we can fix this "for real", and make cross-language references work properly 😔 Foreign objects—especially in the languages we're targeting—can't uphold Rust's "aliasing XOR mutability" guarantee, in either direction (C / C++ could, maybe, if we were very careful?)...and if we're passing a dictionary or a list, we need to serialize it, anyway.

WDYT?

[mhammond]

References in traits (and functions) feel tricky: they're definitely more ergonomic for Rust callers, but they hide a "surprise copy" for foreign callers. I think most folks would expect that an &T argument would avoid a copy, but the scaffolding actually creates an owned T, passes &T as an argument, and then destroys T.

I see your point, but I'm not sure I entirely agree with the severity of the status quo.

Firstly, in this specific scenario, I don't really think it necessarily adds any new surprises, at least for foreign callers of these traits. You could potentially argue that foreign implementations of these traits might expect a reference there too, but I'm not particularly convinced that's a bigger surprise than they already have - ie, I don't think that trivial patch I noted above introduces more surprises than the non-foreign trait example has. Therefore, while we continue to support that capability today for non-foreign traits, I can't see a good argument for also not supporting it for foreign traits (unless, of course, we agree to remove that existing capability in the short term, which I'm not really sure we are in a position to do)

Secondly, in the general case, I'm also not particularly convinced we are surprising many users. I haven't really seen evidence of many users believing that a rust function taking an argument by reference will somehow allow foreign code to just pass references. And even if some are, I don't think it's so confusing that better documentation can't help there. While I'd agree that not supporting it would make sense if there was no concrete value in the support, I think supporting these args do have value because you can use more natural (and possibly your existing) Rust code (eg, our existing BridgedEngine trait, which I'm still hoping to use as a foreign trait, does use byref args). In some ways I see this not too dissimilar to being able to use &self. In other words, I'm asserting that the small surprise to some users is justified by the added convenience for the majority.

[mhammond]

But we can have both, right? We should be able to sniff out a Cow and carry a flag in the metadata to the bindings?

[bendk]

I feel very on the fence about this one, but I think I'm leaning towards "it's okay". Like Mark says, it seems fine to say that passing things across the FFI will likely require copying, even if you have a reference. You could even say the same thing happens with owned values. If I pass a Record value, UniFFI is going to make 2 copies that wouldn't happen with a normal Rust call -- one to serialize and one to deserialize.