Use handles for better trait interfaces passing

Check if the trait interface handle originates from the other side of
the FFI.  If it does, clone the handle, rather than returning a handle
to the wrapped object.  Before, each time the trait object was passed
across the FFI, we wrapped it another time

On the foreign side, this can be accomplished by a type check.  On the
Rust side, this requires an extra, `#[doc(hidden)]` method on the trait.
This means that UDL-defined trait interfaces need to be wrapped with an
attribute macro that inserts that method.

Reworked the changelog so that breaking changes for external bindings is
its own section.

Removed the reexport-scaffolding-macro fixture which often requires
changes when the FFI changes.  I don't think it's giving us enough value
at this point to justify continuing to update it.

Author: bendk

CHANGELOG.md

@@ -16,13 +16,22 @@
 
 ### What's changed?
 
+- Trait interfaces defined in UDL need to be wrapped with `#[uniffi::trait_interface]`.
+- Trait interfaces performance has been improved.  If a trait interface handle is passed across the
+  FFI multiple times, UniFFI will only wrap the object once rather than each time it's passed.
+
+### Breaking changes for external bindings:
+
 - The `rust_future_continuation_callback_set` FFI function was removed.  `rust_future_poll` now
-  inputs the callback pointer.  External bindings authors will need to update their code.
-- The object handle FFI has changed.  External bindings generators will need to update their code to
-  use the new handle system:
-    * A single `FfiType::Handle` is used for all object handles.
-    * `FfiType::Handle` is always a 64-bit int.
-    * Foreign handles must always set the lowest bit of that int.
+  inputs the callback pointer.
+- The object handle FFI has changed.
+  * A single `FfiType::Handle` is used for all object handles.
+  * `FfiType::Handle` is always a 64-bit int.
+  * Foreign handles must always set the lowest bit of that int.
+  * Handles are now cloned before they are returned:
+    * Clone Rust handles before lowering them
+    * If you implement trait interfaces: implement the `IDX_CALLBACK_CLONE` special method for
+      callback interfaces so that Rust can clone your handles and consume the handle in `lift`.
 
 ## v0.25.0 (backend crates: v0.25.0) - (_2023-10-18_)
 

Cargo.lock

@@ -37,7 +37,6 @@ members = [
   "fixtures/keywords/swift",
   "fixtures/metadata",
   "fixtures/proc-macro",
-  "fixtures/reexport-scaffolding-macro",
   "fixtures/regressions/enum-without-i32-helpers",
   "fixtures/regressions/fully-qualified-types",
   "fixtures/regressions/kotlin-experimental-unsigned-types",

Cargo.toml

@@ -22,6 +22,7 @@ a [compatible error type](./udl/errors.md) - see below for more on error handlin
 For example:
 
 ```rust,no_run
+#[uniffi::trait_interface]
 pub trait Keychain: Send + Sync + Debug {
   fn get(&self, key: String) -> Result<Option<String>, KeyChainError>;
   fn put(&self, key: String, value: String) -> Result<(), KeyChainError>;

docs/manual/src/foreign_traits.md

@@ -84,6 +84,8 @@ impl MyObject {
 // Corresponding UDL:
 // [Trait]
 // interface MyTrait {};
+//
+// Note: `[uniffi::trait_interface]` is not needed when the trait is exported via a proc-macro.
 #[uniffi::export]
 trait MyTrait {
     // ...

docs/manual/src/proc_macro/index.md

@@ -87,9 +87,10 @@ interface Button {
 
 ```
 
-With the following Rust implementation:
+The Rust implementation needs to be wrapped in `#[uniffi::trait_interface]`:
 
 ```rust
+#[uniffi::trait_interface]
 pub trait Button: Send + Sync {
     fn name(&self) -> String;
 }

docs/manual/src/udl/interfaces.md

@@ -21,6 +21,7 @@ impl From<uniffi::UnexpectedUniFFICallbackError> for TelephoneError {
 }
 
 // SIM cards.
+#[uniffi::trait_interface]
 pub trait SimCard: Send + Sync {
     fn name(&self) -> String;
 }

examples/callbacks/src/lib.rs

@@ -13,6 +13,7 @@ fn press(button: Arc<dyn Button>) -> Arc<dyn Button> {
     button
 }
 
+#[uniffi::trait_interface]
 pub trait Button: Send + Sync {
     fn name(&self) -> String;
 }

examples/traits/src/lib.rs

@@ -10,6 +10,7 @@ pub fn get_traits() -> Vec<Arc<dyn NodeTrait>> {
     vec![Arc::new(Trait1::default()), Arc::new(Trait2::default())]
 }
 
+#[uniffi::trait_interface]
 pub trait NodeTrait: Send + Sync + std::fmt::Debug {
     fn name(&self) -> String;
 
@@ -35,6 +36,7 @@ pub fn ancestor_names(node: Arc<dyn NodeTrait>) -> Vec<String> {
 /// Test trait
 ///
 /// The goal here is to test all possible arg, return, and error types.
+#[uniffi::trait_interface]
 pub trait Getters: Send + Sync {
     fn get_bool(&self, v: bool, arg2: bool) -> bool;
     fn get_string(&self, v: String, arg2: bool) -> Result<String, CoverallError>;

fixtures/coverall/src/traits.rs

@@ -348,12 +348,10 @@ getTraits().let { traits ->
     assert(traits[1].name() == "node-2")
     assert(traits[1].strongCount() == 2UL)
 
-    // Note: this doesn't increase the Rust strong count, since we wrap the Rust impl with a
-    // Swift impl before passing it to `setParent()`
     traits[0].setParent(traits[1])
     assert(ancestorNames(traits[0]) == listOf("node-2"))
     assert(ancestorNames(traits[1]).isEmpty())
-    assert(traits[1].strongCount() == 2UL)
+    assert(traits[1].strongCount() == 3UL)
     assert(traits[0].getParent()!!.name() == "node-2")
 
     val ktNode = KotlinNode()
@@ -362,6 +360,10 @@ getTraits().let { traits ->
     assert(ancestorNames(traits[1]) == listOf("node-kt"))
     assert(ancestorNames(ktNode) == listOf<String>())
 
+    // If we get the node back from Rust, we should get the original object, not an object that's
+    // been wrapped again.
+    assert(traits[1].getParent() === ktNode)
+
     traits[1].setParent(null)
     ktNode.setParent(traits[0])
     assert(ancestorNames(ktNode) == listOf("node-1", "node-2"))