From 1010792759c07f4d4a1c52bf1ddc6731740b9b2a Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Thu, 14 Dec 2023 11:58:34 -0500 Subject: [PATCH] Make foreign trait implementations opt-in (#1827) This changes the default behavior for exported trait interfaces to not generate a foreign callback interface implementation for them. Users can opt-in to this using the `WithForeign` attribute for UDL and the `with_foreign` for proc-macros. The main reason for this change is that callback interfaces and trait interfaces aren't completely compatible. For example, `ByRef` is not supported for callback interfaces, but it is supported for trait interfaces that only have Rust implementations. --- CHANGELOG.md | 2 + docs/manual/src/proc_macro/index.md | 7 ++ docs/manual/src/udl/ext_types.md | 2 +- docs/manual/src/udl/interfaces.md | 11 ++- examples/callbacks/src/callbacks.udl | 2 +- examples/traits/src/traits.udl | 2 +- fixtures/coverall/src/coverall.udl | 18 ++++- fixtures/coverall/src/lib.rs | 4 +- fixtures/coverall/src/traits.rs | 23 ++++++ .../coverall/tests/bindings/test_coverall.kts | 6 ++ .../coverall/tests/bindings/test_coverall.py | 5 ++ .../tests/bindings/test_coverall.swift | 7 ++ fixtures/ext-types/uniffi-one/src/lib.rs | 2 +- .../ext-types/uniffi-one/src/uniffi-one.udl | 2 +- fixtures/metadata/src/tests.rs | 80 +++++++++++++++++-- fixtures/proc-macro/src/lib.rs | 37 ++++++++- fixtures/proc-macro/src/proc-macro.udl | 4 + .../tests/bindings/test_proc_macro.kts | 10 ++- .../tests/bindings/test_proc_macro.py | 12 ++- .../tests/bindings/test_proc_macro.swift | 10 ++- .../src/bindings/kotlin/gen_kotlin/mod.rs | 23 +++--- .../src/bindings/kotlin/gen_kotlin/object.rs | 7 +- .../kotlin/templates/ObjectTemplate.kt | 13 ++- .../src/bindings/python/gen_python/mod.rs | 19 +++-- .../python/templates/ObjectTemplate.py | 16 ++-- .../src/bindings/swift/gen_swift/mod.rs | 21 +++-- .../src/bindings/swift/gen_swift/object.rs | 7 +- .../swift/templates/ObjectTemplate.swift | 13 ++- uniffi_bindgen/src/interface/mod.rs | 2 +- uniffi_bindgen/src/interface/object.rs | 8 +- .../scaffolding/templates/ObjectTemplate.rs | 9 +-- uniffi_core/src/metadata.rs | 5 +- uniffi_macros/src/export.rs | 14 +++- uniffi_macros/src/export/attributes.rs | 18 ++++- uniffi_macros/src/export/item.rs | 18 ++++- uniffi_macros/src/export/trait_interface.rs | 56 ++++++++++--- uniffi_macros/src/object.rs | 16 ++-- uniffi_macros/src/util.rs | 1 + uniffi_meta/src/metadata.rs | 7 +- uniffi_meta/src/reader.rs | 20 ++++- uniffi_meta/src/types.rs | 20 ++--- uniffi_udl/src/attributes.rs | 39 +++++++-- uniffi_udl/src/converters/interface.rs | 2 +- uniffi_udl/src/finder.rs | 7 +- 44 files changed, 451 insertions(+), 156 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 04da00227d..bea4cc7826 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ - Rust traits `Display`, `Hash` and `Eq` exposed to Kotlin and Swift [#1817](https://github.com/mozilla/uniffi-rs/pull/1817) - Foreign types can now implement trait interfaces [#1791](https://github.com/mozilla/uniffi-rs/pull/1791) + - UDL: use the `[WithForeign]` attribute + - proc-macros: use the `#[uniffi::export(with_foreign)]` attribute - Generated Python code is able to specify a package name for the module [#1784](https://github.com/mozilla/uniffi-rs/pull/1784) - UDL can describe async function [#1834](https://github.com/mozilla/uniffi-rs/pull/1834) - UDL files can reference types defined in procmacros in this crate - see diff --git a/docs/manual/src/proc_macro/index.md b/docs/manual/src/proc_macro/index.md index 03c9af9e9e..b9eafcecb8 100644 --- a/docs/manual/src/proc_macro/index.md +++ b/docs/manual/src/proc_macro/index.md @@ -89,6 +89,13 @@ trait MyTrait { // ... } +// Corresponding UDL: +// [Trait, WithForeign] +// interface MyTrait {}; +#[uniffi::export(with_foreign)] +trait MyTrait { + // ... +} ``` diff --git a/docs/manual/src/udl/ext_types.md b/docs/manual/src/udl/ext_types.md index 593508ae5b..4ad4139c5e 100644 --- a/docs/manual/src/udl/ext_types.md +++ b/docs/manual/src/udl/ext_types.md @@ -37,6 +37,6 @@ namespace app { ``` Supported values: -* "enum", "trait", "callback" +* "enum", "trait", "callback", "trait_with_foreign" * For records, either "record" or "dictionary" * For objects, either "object" or "interface" diff --git a/docs/manual/src/udl/interfaces.md b/docs/manual/src/udl/interfaces.md index 020befb981..7f040f4e58 100644 --- a/docs/manual/src/udl/interfaces.md +++ b/docs/manual/src/udl/interfaces.md @@ -124,7 +124,14 @@ fn press(button: Arc) -> Arc { ... } ### Foreign implementations -Traits can also be implemented on the foreign side passed into Rust, for example: +Use the `WithForeign` attribute to allow traits to also be implemented on the foreign side passed into Rust, for example: + +```idl +[Trait, WithForeign] +interface Button { + string name(); +}; +``` ```python class PyButton(uniffi_module.Button): @@ -134,7 +141,7 @@ class PyButton(uniffi_module.Button): uniffi_module.press(PyButton()) ``` -Note: This is currently supported on Python, Kotlin, and Swift. +Note: This is currently only supported on Python, Kotlin, and Swift. ### Traits construction diff --git a/examples/callbacks/src/callbacks.udl b/examples/callbacks/src/callbacks.udl index 00cc1e62b3..1ce79fda67 100644 --- a/examples/callbacks/src/callbacks.udl +++ b/examples/callbacks/src/callbacks.udl @@ -3,7 +3,7 @@ namespace callbacks { }; // This trait is implemented in Rust and in foreign bindings. -[Trait] +[Trait, WithForeign] interface SimCard { string name(); // The name of the carrier/provider. }; diff --git a/examples/traits/src/traits.udl b/examples/traits/src/traits.udl index b9583f5703..6b75916d35 100644 --- a/examples/traits/src/traits.udl +++ b/examples/traits/src/traits.udl @@ -6,7 +6,7 @@ namespace traits { }; // This is a trait in Rust. -[Trait] +[Trait, WithForeign] interface Button { string name(); }; diff --git a/fixtures/coverall/src/coverall.udl b/fixtures/coverall/src/coverall.udl index 9eabc55b05..a51f8f36aa 100644 --- a/fixtures/coverall/src/coverall.udl +++ b/fixtures/coverall/src/coverall.udl @@ -23,6 +23,8 @@ namespace coverall { sequence ancestor_names(NodeTrait node); + sequence get_string_util_traits(); + ReturnOnlyDict output_return_only_dict(); ReturnOnlyEnum output_return_only_enum(); @@ -220,10 +222,10 @@ interface ThreadsafeCounter { i32 increment_if_busy(); }; -// Test trait #1 +// Test trait interface #1 // // The goal here is to test all possible arg, return, and error types. -[Trait] +[Trait, WithForeign] interface Getters { boolean get_bool(boolean v, boolean arg2); [Throws=CoverallError] @@ -235,10 +237,10 @@ interface Getters { Coveralls round_trip_object(Coveralls coveralls); }; -// Test trait #2 +// Test trait interface #2 // // The goal here is test passing objects back and forth between Rust and the foreign side -[Trait] +[Trait, WithForeign] interface NodeTrait { string name(); // The name of the this node @@ -254,6 +256,14 @@ interface NodeTrait { u64 strong_count(); }; +// Test trait interface #3 +// +// The goal here is test Rust-only trait interfaces +[Trait] +interface StringUtil { + string concat([ByRef]string a, [ByRef]string b); +}; + // Forward/backward declarations are fine in UDL. // Running the Python tests & type checks will ensure this works, // no function calls needed diff --git a/fixtures/coverall/src/lib.rs b/fixtures/coverall/src/lib.rs index 301513de04..d7ba238568 100644 --- a/fixtures/coverall/src/lib.rs +++ b/fixtures/coverall/src/lib.rs @@ -11,8 +11,8 @@ use once_cell::sync::Lazy; mod traits; pub use traits::{ - ancestor_names, get_traits, make_rust_getters, test_getters, test_round_trip_through_foreign, - test_round_trip_through_rust, Getters, NodeTrait, + ancestor_names, get_string_util_traits, get_traits, make_rust_getters, test_getters, + test_round_trip_through_foreign, test_round_trip_through_rust, Getters, NodeTrait, StringUtil, }; static NUM_ALIVE: Lazy> = Lazy::new(|| RwLock::new(0)); diff --git a/fixtures/coverall/src/traits.rs b/fixtures/coverall/src/traits.rs index 5ba65694ec..15be1eeebe 100644 --- a/fixtures/coverall/src/traits.rs +++ b/fixtures/coverall/src/traits.rs @@ -200,3 +200,26 @@ impl NodeTrait for Trait2 { (*self.parent.lock().unwrap()).as_ref().map(Arc::clone) } } + +pub trait StringUtil: Send + Sync { + fn concat(&self, a: &str, b: &str) -> String; +} + +pub struct StringUtilImpl1; +pub struct StringUtilImpl2; + +pub fn get_string_util_traits() -> Vec> { + vec![Arc::new(StringUtilImpl1), Arc::new(StringUtilImpl2)] +} + +impl StringUtil for StringUtilImpl1 { + fn concat(&self, a: &str, b: &str) -> String { + format!("{a}{b}") + } +} + +impl StringUtil for StringUtilImpl2 { + fn concat(&self, a: &str, b: &str) -> String { + format!("{a}{b}") + } +} diff --git a/fixtures/coverall/tests/bindings/test_coverall.kts b/fixtures/coverall/tests/bindings/test_coverall.kts index eff626bad9..71830112ec 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.kts +++ b/fixtures/coverall/tests/bindings/test_coverall.kts @@ -406,6 +406,12 @@ makeRustGetters().let { rustGetters -> testRoundTripThroughForeign(KotlinGetters()) } +// Test StringUtil +getStringUtilTraits().let { traits -> + assert(traits[0].concat("cow", "boy") == "cowboy") + assert(traits[1].concat("cow", "boy") == "cowboy") +} + // This tests that the UniFFI-generated scaffolding doesn't introduce any unexpected locking. // We have one thread busy-wait for a some period of time, while a second thread repeatedly // increments the counter and then checks if the object is still busy. The second thread should diff --git a/fixtures/coverall/tests/bindings/test_coverall.py b/fixtures/coverall/tests/bindings/test_coverall.py index 5b14c02467..34f83c27c3 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.py +++ b/fixtures/coverall/tests/bindings/test_coverall.py @@ -443,5 +443,10 @@ def test_round_tripping(self): test_round_trip_through_foreign(PyGetters()) + def test_rust_only_traits(self): + traits = get_string_util_traits() + self.assertEqual(traits[0].concat("cow", "boy"), "cowboy") + self.assertEqual(traits[1].concat("cow", "boy"), "cowboy") + if __name__=='__main__': unittest.main() diff --git a/fixtures/coverall/tests/bindings/test_coverall.swift b/fixtures/coverall/tests/bindings/test_coverall.swift index fb1e32b102..1e52e1ba80 100644 --- a/fixtures/coverall/tests/bindings/test_coverall.swift +++ b/fixtures/coverall/tests/bindings/test_coverall.swift @@ -457,3 +457,10 @@ do { testRoundTripThroughForeign(getters: SwiftGetters()) } + +// Test rust-only traits +do { + let stringUtils = getStringUtilTraits() + assert(stringUtils[0].concat(a: "cow", b: "boy") == "cowboy") + assert(stringUtils[1].concat(a: "cow", b: "boy") == "cowboy") +} diff --git a/fixtures/ext-types/uniffi-one/src/lib.rs b/fixtures/ext-types/uniffi-one/src/lib.rs index 7a53adc463..90ecc191e8 100644 --- a/fixtures/ext-types/uniffi-one/src/lib.rs +++ b/fixtures/ext-types/uniffi-one/src/lib.rs @@ -39,7 +39,7 @@ async fn get_uniffi_one_async() -> UniffiOneEnum { UniffiOneEnum::One } -#[uniffi::export] +#[uniffi::export(with_foreign)] pub trait UniffiOneTrait: Send + Sync { fn hello(&self) -> String; } diff --git a/fixtures/ext-types/uniffi-one/src/uniffi-one.udl b/fixtures/ext-types/uniffi-one/src/uniffi-one.udl index 5e23ae2525..a0245922ca 100644 --- a/fixtures/ext-types/uniffi-one/src/uniffi-one.udl +++ b/fixtures/ext-types/uniffi-one/src/uniffi-one.udl @@ -15,7 +15,7 @@ interface UniffiOneInterface { i32 increment(); }; -[Trait] +[Trait, WithForeign] interface UniffiOneUDLTrait { string hello(); }; diff --git a/fixtures/metadata/src/tests.rs b/fixtures/metadata/src/tests.rs index f4d8ae244f..27f6487821 100644 --- a/fixtures/metadata/src/tests.rs +++ b/fixtures/metadata/src/tests.rs @@ -471,6 +471,15 @@ mod test_function_metadata { } } + #[uniffi::export(with_foreign)] + pub trait TraitWithForeign: Send + Sync { + fn test_method(&self, a: String, b: u32) -> String; + } + + #[allow(unused)] + #[uniffi::export] + fn input_trait_with_foreign(val: Arc) {} + #[test] fn test_function() { check_metadata( @@ -680,20 +689,48 @@ mod test_function_metadata { } #[test] - fn test_trait_result() { + fn test_trait_metadata() { + check_metadata( + &UNIFFI_META_UNIFFI_FIXTURE_METADATA_INTERFACE_CALCULATORDISPLAY, + ObjectMetadata { + module_path: "uniffi_fixture_metadata".into(), + name: "CalculatorDisplay".into(), + imp: ObjectImpl::Trait, + docstring: None, + }, + ); + } + + #[test] + fn test_trait_with_foreign_metadata() { + check_metadata( + &UNIFFI_META_UNIFFI_FIXTURE_METADATA_INTERFACE_TRAITWITHFOREIGN, + ObjectMetadata { + module_path: "uniffi_fixture_metadata".into(), + name: "TraitWithForeign".into(), + imp: ObjectImpl::CallbackTrait, + docstring: None, + }, + ); + } + + #[test] + fn test_trait_type_data() { check_metadata( &UNIFFI_META_UNIFFI_FIXTURE_METADATA_METHOD_CALCULATOR_GET_DISPLAY, MethodMetadata { - module_path: "uniffi_fixture_metadata".into(), - self_name: "Calculator".into(), - name: "get_display".into(), - is_async: false, - inputs: vec![], + // The main point of this test is to check the `Type` value for a trait interface return_type: Some(Type::Object { module_path: "uniffi_fixture_metadata".into(), name: "CalculatorDisplay".into(), imp: ObjectImpl::Trait, }), + // We might as well test other fields too though + module_path: "uniffi_fixture_metadata".into(), + self_name: "Calculator".into(), + name: "get_display".into(), + is_async: false, + inputs: vec![], throws: None, takes_self_by_arc: false, checksum: Some( @@ -728,6 +765,37 @@ mod test_function_metadata { ); } + #[test] + fn test_trait_with_foreign_type_data() { + check_metadata( + &UNIFFI_META_UNIFFI_FIXTURE_METADATA_FUNC_INPUT_TRAIT_WITH_FOREIGN, + FnMetadata { + inputs: vec![ + // The main point of this test is to check the `Type` value for a trait interface + FnParamMetadata::simple( + "val", + Type::Object { + module_path: "uniffi_fixture_metadata".into(), + name: "TraitWithForeign".into(), + imp: ObjectImpl::CallbackTrait, + }, + ), + ], + // We might as well test other fields too though + return_type: None, + module_path: "uniffi_fixture_metadata".into(), + name: "input_trait_with_foreign".into(), + is_async: false, + throws: None, + checksum: Some( + UNIFFI_META_CONST_UNIFFI_FIXTURE_METADATA_FUNC_INPUT_TRAIT_WITH_FOREIGN + .checksum(), + ), + docstring: None, + }, + ); + } + #[test] fn test_callback_interface() { check_metadata( diff --git a/fixtures/proc-macro/src/lib.rs b/fixtures/proc-macro/src/lib.rs index d1f2991787..4e393af201 100644 --- a/fixtures/proc-macro/src/lib.rs +++ b/fixtures/proc-macro/src/lib.rs @@ -56,14 +56,30 @@ impl Unused { #[uniffi::export] pub trait Trait: Send + Sync { - fn name(&self) -> String; + // Test the absence of `with_foreign` by inputting reference arguments, which is + // incompatible with callback interfaces + #[allow(clippy::ptr_arg)] + fn concat_strings(&self, a: &str, b: &str) -> String; } struct TraitImpl {} impl Trait for TraitImpl { + fn concat_strings(&self, a: &str, b: &str) -> String { + format!("{a}{b}") + } +} + +#[uniffi::export(with_foreign)] +pub trait TraitWithForeign: Send + Sync { + fn name(&self) -> String; +} + +struct RustTraitImpl {} + +impl TraitWithForeign for RustTraitImpl { fn name(&self) -> String { - "TraitImpl".to_string() + "RustTraitImpl".to_string() } } @@ -97,6 +113,13 @@ impl Object { inc.unwrap_or_else(|| Arc::new(TraitImpl {})) } + fn get_trait_with_foreign( + &self, + inc: Option>, + ) -> Arc { + inc.unwrap_or_else(|| Arc::new(RustTraitImpl {})) + } + fn take_error(&self, e: BasicError) -> u32 { assert!(matches!(e, BasicError::InvalidInput)); 42 @@ -104,8 +127,8 @@ impl Object { } #[uniffi::export] -fn get_trait_name_by_ref(t: &dyn Trait) -> String { - t.name() +fn concat_strings_by_ref(t: &dyn Trait, a: &str, b: &str) -> String { + t.concat_strings(a, b) } #[uniffi::export] @@ -260,6 +283,12 @@ fn get_trait(o: Option>) -> Arc { o.unwrap_or_else(|| Arc::new(TraitImpl {})) } +fn get_trait_with_foreign( + o: Option>, +) -> Arc { + o.unwrap_or_else(|| Arc::new(RustTraitImpl {})) +} + #[derive(Default)] struct Externals { one: Option, diff --git a/fixtures/proc-macro/src/proc-macro.udl b/fixtures/proc-macro/src/proc-macro.udl index 843b2e5df8..9896f2e633 100644 --- a/fixtures/proc-macro/src/proc-macro.udl +++ b/fixtures/proc-macro/src/proc-macro.udl @@ -16,6 +16,9 @@ typedef extern Object; [Rust="trait"] typedef extern Trait; +[Rust="trait_with_foreign"] +typedef extern TraitWithForeign; + // Then stuff defined here but referencing the imported types. dictionary Externals { One? one; @@ -28,5 +31,6 @@ namespace proc_macro { MaybeBool get_bool(MaybeBool? b); Object get_object(Object? o); Trait get_trait(Trait? t); + TraitWithForeign get_trait_with_foreign(TraitWithForeign? t); Externals get_externals(Externals? e); }; diff --git a/fixtures/proc-macro/tests/bindings/test_proc_macro.kts b/fixtures/proc-macro/tests/bindings/test_proc_macro.kts index b1acd1c99e..cd30ce26bf 100644 --- a/fixtures/proc-macro/tests/bindings/test_proc_macro.kts +++ b/fixtures/proc-macro/tests/bindings/test_proc_macro.kts @@ -44,9 +44,13 @@ try { } val traitImpl = obj.getTrait(null) -assert(traitImpl.name() == "TraitImpl") -assert(obj.getTrait(traitImpl).name() == "TraitImpl") -assert(getTraitNameByRef(traitImpl) == "TraitImpl") +assert(traitImpl.concatStrings("foo", "bar") == "foobar") +assert(obj.getTrait(traitImpl).concatStrings("foo", "bar") == "foobar") +assert(concatStringsByRef(traitImpl, "foo", "bar") == "foobar") + +val traitImpl2 = obj.getTraitWithForeign(null) +assert(traitImpl2.name() == "RustTraitImpl") +assert(obj.getTraitWithForeign(traitImpl2).name() == "RustTraitImpl") class KtTestCallbackInterface : TestCallbackInterface { diff --git a/fixtures/proc-macro/tests/bindings/test_proc_macro.py b/fixtures/proc-macro/tests/bindings/test_proc_macro.py index 3028c21a1d..03d321642f 100644 --- a/fixtures/proc-macro/tests/bindings/test_proc_macro.py +++ b/fixtures/proc-macro/tests/bindings/test_proc_macro.py @@ -21,9 +21,13 @@ assert obj.is_other_heavy(obj2) == MaybeBool.UNCERTAIN trait_impl = obj.get_trait(None) -assert trait_impl.name() == "TraitImpl" -assert obj.get_trait(trait_impl).name() == "TraitImpl" -assert get_trait_name_by_ref(trait_impl) == "TraitImpl" +assert trait_impl.concat_strings("foo", "bar") == "foobar" +assert obj.get_trait(trait_impl).concat_strings("foo", "bar") == "foobar" +assert concat_strings_by_ref(trait_impl, "foo", "bar") == "foobar" + +trait_impl2 = obj.get_trait_with_foreign(None) +assert trait_impl2.name() == "RustTraitImpl" +assert obj.get_trait_with_foreign(trait_impl2).name() == "RustTraitImpl" assert enum_identity(MaybeBool.TRUE) == MaybeBool.TRUE @@ -90,7 +94,7 @@ def callback_handler(self, h): assert get_one(None).inner == 0 assert get_bool(None) == MaybeBool.UNCERTAIN assert get_object(None).is_heavy() == MaybeBool.UNCERTAIN -assert get_trait(None).name() == "TraitImpl" +assert get_trait_with_foreign(None).name() == "RustTraitImpl" assert get_externals(None).one is None # values for enums without an explicit value are their index. diff --git a/fixtures/proc-macro/tests/bindings/test_proc_macro.swift b/fixtures/proc-macro/tests/bindings/test_proc_macro.swift index b8029c69f0..3613d8303e 100644 --- a/fixtures/proc-macro/tests/bindings/test_proc_macro.swift +++ b/fixtures/proc-macro/tests/bindings/test_proc_macro.swift @@ -22,9 +22,13 @@ let obj2 = Object() assert(obj.isOtherHeavy(other: obj2) == .uncertain) let traitImpl = obj.getTrait(inc: nil) -assert(traitImpl.name() == "TraitImpl") -assert(obj.getTrait(inc: traitImpl).name() == "TraitImpl") -assert(getTraitNameByRef(t: traitImpl) == "TraitImpl") +assert(traitImpl.concatStrings(a: "foo", b: "bar") == "foobar") +assert(obj.getTrait(inc: traitImpl).concatStrings(a: "foo", b: "bar") == "foobar") +assert(concatStringsByRef(t: traitImpl, a: "foo", b: "bar") == "foobar") + +let traitImpl2 = obj.getTraitWithForeign(inc: nil) +assert(traitImpl2.name() == "RustTraitImpl") +assert(obj.getTraitWithForeign(inc: traitImpl2).name() == "RustTraitImpl") assert(enumIdentity(value: .true) == .true) diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs index 7e5940d80c..6883731730 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/mod.rs @@ -342,22 +342,19 @@ impl KotlinCodeOracle { /// Get the name of the interface and class name for an object. /// - /// This depends on the `ObjectImpl`: + /// If we support callback interfaces, the interface name is the object name, and the class name is derived from that. + /// Otherwise, the class name is the object name and the interface name is derived from that. /// - /// For struct impls, the class name is the object name and the interface name is derived from that. - /// For trait impls, the interface name is the object name, and the class name is derived from that. - /// - /// This split is needed because of the `FfiConverter` interface. For struct impls, `lower` - /// can only lower the concrete class. For trait impls, `lower` can lower anything that - /// implement the interface. + /// This split determines what types `FfiConverter.lower()` inputs. If we support callback + /// interfaces, `lower` must lower anything that implements the interface. If not, then lower + /// only lowers the concrete class. fn object_names(&self, ci: &ComponentInterface, obj: &Object) -> (String, String) { let class_name = self.class_name(ci, obj.name()); - match obj.imp() { - ObjectImpl::Struct => (format!("{class_name}Interface"), class_name), - ObjectImpl::Trait => { - let interface_name = format!("{class_name}Impl"); - (class_name, interface_name) - } + if obj.has_callback_interface() { + let impl_name = format!("{class_name}Impl"); + (class_name, impl_name) + } else { + (format!("{class_name}Interface"), class_name) } } } diff --git a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/object.rs b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/object.rs index 5c0b5d5535..5a4305d14a 100644 --- a/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/object.rs +++ b/uniffi_bindgen/src/bindings/kotlin/gen_kotlin/object.rs @@ -27,9 +27,8 @@ impl CodeType for ObjectCodeType { } fn initialization_fn(&self) -> Option { - match &self.imp { - ObjectImpl::Struct => None, - ObjectImpl::Trait => Some(format!("uniffiCallbackInterface{}.register", self.name)), - } + self.imp + .has_callback_interface() + .then(|| format!("uniffiCallbackInterface{}.register", self.name)) } } diff --git a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt index 797d595f11..f190569fc5 100644 --- a/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt +++ b/uniffi_bindgen/src/bindings/kotlin/templates/ObjectTemplate.kt @@ -156,7 +156,7 @@ open class {{ impl_class_name }} : FFIObject, {{ interface_name }} { {% endif %} } -{%- if obj.is_trait_interface() %} +{%- if obj.has_callback_interface() %} {%- let callback_handler_class = format!("UniffiCallbackInterface{}", name) %} {%- let callback_handler_obj = format!("uniffiCallbackInterface{}", name) %} {%- let ffi_init_callback = obj.ffi_init_callback() %} @@ -164,17 +164,16 @@ open class {{ impl_class_name }} : FFIObject, {{ interface_name }} { {%- endif %} public object {{ obj|ffi_converter_name }}: FfiConverter<{{ type_name }}, Pointer> { - {%- if obj.is_trait_interface() %} + {%- if obj.has_callback_interface() %} internal val handleMap = ConcurrentHandleMap<{{ interface_name }}>() {%- endif %} override fun lower(value: {{ type_name }}): Pointer { - {%- match obj.imp() %} - {%- when ObjectImpl::Struct %} - return value.uniffiClonePointer() - {%- when ObjectImpl::Trait %} + {%- if obj.has_callback_interface() %} return Pointer(handleMap.insert(value)) - {%- endmatch %} + {%- else %} + return value.uniffiClonePointer() + {%- endif %} } override fun lift(value: Pointer): {{ type_name }} { diff --git a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs index ae229bc68c..55d851aed9 100644 --- a/uniffi_bindgen/src/bindings/python/gen_python/mod.rs +++ b/uniffi_bindgen/src/bindings/python/gen_python/mod.rs @@ -365,16 +365,19 @@ impl PythonCodeOracle { /// Get the name of the protocol and class name for an object. /// - /// For struct impls, the class name is the object name and the protocol name is derived from that. - /// For trait impls, the protocol name is the object name, and the class name is derived from that. + /// If we support callback interfaces, the protocol name is the object name, and the class name is derived from that. + /// Otherwise, the class name is the object name and the protocol name is derived from that. + /// + /// This split determines what types `FfiConverter.lower()` inputs. If we support callback + /// interfaces, `lower` must lower anything that implements the protocol. If not, then lower + /// only lowers the concrete class. fn object_names(&self, obj: &Object) -> (String, String) { let class_name = self.class_name(obj.name()); - match obj.imp() { - ObjectImpl::Struct => (format!("{class_name}Protocol"), class_name), - ObjectImpl::Trait => { - let protocol_name = format!("{class_name}Impl"); - (class_name, protocol_name) - } + if obj.has_callback_interface() { + let impl_name = format!("{class_name}Impl"); + (class_name, impl_name) + } else { + (format!("{class_name}Protocol"), class_name) } } } diff --git a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py index c68808d01f..c041e1a1b1 100644 --- a/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py +++ b/uniffi_bindgen/src/bindings/python/templates/ObjectTemplate.py @@ -1,3 +1,4 @@ +# IMP: {{ "{:?}"|format(obj.imp()) }} {%- let obj = ci|get_object_definition(name) %} {%- let (protocol_name, impl_name) = obj|object_names %} {%- let methods = obj.methods() %} @@ -75,7 +76,7 @@ def __ne__(self, other: object) -> {{ ne.return_type().unwrap()|type_name }}: {% endmatch %} {% endfor %} -{%- if obj.is_trait_interface() %} +{%- if obj.has_callback_interface() %} {%- let callback_handler_class = format!("UniffiCallbackInterface{}", name) %} {%- let callback_handler_obj = format!("uniffiCallbackInterface{}", name) %} {%- let ffi_init_callback = obj.ffi_init_callback() %} @@ -83,7 +84,7 @@ def __ne__(self, other: object) -> {{ ne.return_type().unwrap()|type_name }}: {%- endif %} class {{ ffi_converter_name }}: - {%- if obj.is_trait_interface() %} + {%- if obj.has_callback_interface() %} _handle_map = ConcurrentHandleMap() {%- endif %} @@ -93,7 +94,7 @@ def lift(value: int): @staticmethod def check_lower(value: {{ type_name }}): - {%- if obj.is_trait_interface() %} + {%- if obj.has_callback_interface() %} pass {%- else %} if not isinstance(value, {{ impl_name }}): @@ -102,14 +103,13 @@ def check_lower(value: {{ type_name }}): @staticmethod def lower(value: {{ protocol_name }}): - {%- match obj.imp() %} - {%- when ObjectImpl::Struct %} + {%- if obj.has_callback_interface() %} + return {{ ffi_converter_name }}._handle_map.insert(value) + {%- else %} if not isinstance(value, {{ impl_name }}): raise TypeError("Expected {{ impl_name }} instance, {} found".format(type(value).__name__)) return value._uniffi_clone_pointer() - {%- when ObjectImpl::Trait %} - return {{ ffi_converter_name }}._handle_map.insert(value) - {%- endmatch %} + {%- endif %} @classmethod def read(cls, buf: _UniffiRustBuffer): diff --git a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs index d963b599a9..85bac015d1 100644 --- a/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs +++ b/uniffi_bindgen/src/bindings/swift/gen_swift/mod.rs @@ -548,20 +548,19 @@ impl SwiftCodeOracle { /// Get the name of the protocol and class name for an object. /// - /// For struct impls, the class name is the object name and the protocol name is derived from that. - /// For trait impls, the protocol name is the object name, and the class name is derived from that. + /// If we support callback interfaces, the protocol name is the object name, and the class name is derived from that. + /// Otherwise, the class name is the object name and the protocol name is derived from that. /// - /// This split is needed because of the `FfiConverter` protocol. For struct impls, `lower` - /// can only lower the concrete class. For trait impls, `lower` can lower anything that - /// implement the protocol. + /// This split determines what types `FfiConverter.lower()` inputs. If we support callback + /// interfaces, `lower` must lower anything that implements the protocol. If not, then lower + /// only lowers the concrete class. fn object_names(&self, obj: &Object) -> (String, String) { let class_name = self.class_name(obj.name()); - match obj.imp() { - ObjectImpl::Struct => (format!("{class_name}Protocol"), class_name), - ObjectImpl::Trait => { - let protocol_name = format!("{class_name}Impl"); - (class_name, protocol_name) - } + if obj.has_callback_interface() { + let impl_name = format!("{class_name}Impl"); + (class_name, impl_name) + } else { + (format!("{class_name}Protocol"), class_name) } } } diff --git a/uniffi_bindgen/src/bindings/swift/gen_swift/object.rs b/uniffi_bindgen/src/bindings/swift/gen_swift/object.rs index ea231dd740..d4497a7b19 100644 --- a/uniffi_bindgen/src/bindings/swift/gen_swift/object.rs +++ b/uniffi_bindgen/src/bindings/swift/gen_swift/object.rs @@ -27,9 +27,8 @@ impl CodeType for ObjectCodeType { } fn initialization_fn(&self) -> Option { - match &self.imp { - ObjectImpl::Struct => None, - ObjectImpl::Trait => Some(format!("uniffiCallbackInit{}", self.name)), - } + self.imp + .has_callback_interface() + .then(|| format!("uniffiCallbackInit{}", self.name)) } } diff --git a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift index 173a98b8de..518beb5aa0 100644 --- a/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift +++ b/uniffi_bindgen/src/bindings/swift/templates/ObjectTemplate.swift @@ -143,7 +143,7 @@ public class {{ impl_class_name }}: } -{%- if obj.is_trait_interface() %} +{%- if obj.has_callback_interface() %} {%- let callback_handler = format!("uniffiCallbackInterface{}", name) %} {%- let callback_init = format!("uniffiCallbackInit{}", name) %} {%- let ffi_init_callback = obj.ffi_init_callback() %} @@ -151,7 +151,7 @@ public class {{ impl_class_name }}: {%- endif %} public struct {{ ffi_converter_name }}: FfiConverter { - {%- if obj.is_trait_interface() %} + {%- if obj.has_callback_interface() %} fileprivate static var handleMap = UniFFICallbackHandleMap<{{ type_name }}>() {%- endif %} @@ -163,15 +163,14 @@ public struct {{ ffi_converter_name }}: FfiConverter { } public static func lower(_ value: {{ type_name }}) -> UnsafeMutableRawPointer { - {%- match obj.imp() %} - {%- when ObjectImpl::Struct %} - return value.uniffiClonePointer() - {%- when ObjectImpl::Trait %} + {%- if obj.has_callback_interface() %} guard let ptr = UnsafeMutableRawPointer(bitPattern: UInt(truncatingIfNeeded: handleMap.insert(obj: value))) else { fatalError("Cast to UnsafeMutableRawPointer failed") } return ptr - {%- endmatch %} + {%- else %} + return value.uniffiClonePointer() + {%- endif %} } public static func read(from buf: inout (data: Data, offset: Data.Index)) throws -> {{ type_name }} { diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index d76a1499c8..2d3db42e60 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -257,7 +257,7 @@ impl ComponentInterface { .chain( self.object_definitions() .iter() - .filter(|o| o.is_trait_interface()) + .filter(|o| o.has_callback_interface()) .flat_map(|o| o.methods()), ) .any(|m| m.throws_type() == Some(&e.as_type())); diff --git a/uniffi_bindgen/src/interface/object.rs b/uniffi_bindgen/src/interface/object.rs index 06e32b3a37..84778dd53b 100644 --- a/uniffi_bindgen/src/interface/object.rs +++ b/uniffi_bindgen/src/interface/object.rs @@ -127,7 +127,11 @@ impl Object { } pub fn is_trait_interface(&self) -> bool { - matches!(self.imp, ObjectImpl::Trait) + self.imp.is_trait_interface() + } + + pub fn has_callback_interface(&self) -> bool { + self.imp.has_callback_interface() } pub fn constructors(&self) -> Vec<&Constructor> { @@ -214,7 +218,7 @@ impl Object { }]; self.ffi_func_free.return_type = None; self.ffi_func_free.is_object_free_function = true; - if self.is_trait_interface() { + if self.has_callback_interface() { self.ffi_init_callback = Some(FfiFunction::callback_init(&self.module_path, &self.name)); } diff --git a/uniffi_bindgen/src/scaffolding/templates/ObjectTemplate.rs b/uniffi_bindgen/src/scaffolding/templates/ObjectTemplate.rs index bea5d4be9f..e58e17cc39 100644 --- a/uniffi_bindgen/src/scaffolding/templates/ObjectTemplate.rs +++ b/uniffi_bindgen/src/scaffolding/templates/ObjectTemplate.rs @@ -2,9 +2,8 @@ // Forward work to `uniffi_macros` This keeps macro-based and UDL-based generated code consistent. #} -{%- match obj.imp() -%} -{%- when ObjectImpl::Trait %} -#[::uniffi::export_for_udl] +{%- if obj.is_trait_interface() %} +#[::uniffi::export_for_udl{% if obj.has_callback_interface() %}(with_foreign){% endif %}] pub trait r#{{ obj.name() }} { {%- for meth in obj.methods() %} fn {% if meth.is_async() %}async {% endif %}r#{{ meth.name() }}( @@ -21,7 +20,7 @@ pub trait r#{{ obj.name() }} { {%- endmatch %} {% endfor %} } -{% when ObjectImpl::Struct %} +{%- else %} {%- for tm in obj.uniffi_traits() %} {% match tm %} {% when UniffiTrait::Debug { fmt }%} @@ -78,4 +77,4 @@ impl {{ obj.rust_name() }} { } {%- endfor %} -{% endmatch %} +{% endif %} diff --git a/uniffi_core/src/metadata.rs b/uniffi_core/src/metadata.rs index f6a42e9876..1d3ce30e0e 100644 --- a/uniffi_core/src/metadata.rs +++ b/uniffi_core/src/metadata.rs @@ -39,6 +39,8 @@ pub mod codes { pub const CALLBACK_INTERFACE: u8 = 9; pub const TRAIT_METHOD: u8 = 10; pub const UNIFFI_TRAIT: u8 = 11; + pub const TRAIT_INTERFACE: u8 = 12; + pub const CALLBACK_TRAIT_INTERFACE: u8 = 13; pub const UNKNOWN: u8 = 255; // Type codes @@ -66,7 +68,8 @@ pub mod codes { pub const TYPE_CALLBACK_INTERFACE: u8 = 21; pub const TYPE_CUSTOM: u8 = 22; pub const TYPE_RESULT: u8 = 23; - pub const TYPE_FUTURE: u8 = 24; + pub const TYPE_TRAIT_INTERFACE: u8 = 24; + pub const TYPE_CALLBACK_TRAIT_INTERFACE: u8 = 25; pub const TYPE_UNIT: u8 = 255; // Literal codes for LiteralMetadata - note that we don't support diff --git a/uniffi_macros/src/export.rs b/uniffi_macros/src/export.rs index c98d6e059d..50ff16740e 100644 --- a/uniffi_macros/src/export.rs +++ b/uniffi_macros/src/export.rs @@ -67,16 +67,24 @@ pub(crate) fn expand_export( ExportItem::Trait { items, self_ident, - callback_interface: false, + with_foreign, + callback_interface_only: false, docstring, } => trait_interface::gen_trait_scaffolding( - &mod_path, args, self_ident, items, udl_mode, docstring, + &mod_path, + args, + self_ident, + items, + udl_mode, + with_foreign, + docstring, ), ExportItem::Trait { items, self_ident, - callback_interface: true, + callback_interface_only: true, docstring, + .. } => { let trait_name = ident_to_string(&self_ident); let trait_impl_ident = callback_interface::trait_impl_ident(&trait_name); diff --git a/uniffi_macros/src/export/attributes.rs b/uniffi_macros/src/export/attributes.rs index c3edcd5920..b7c5e65a73 100644 --- a/uniffi_macros/src/export/attributes.rs +++ b/uniffi_macros/src/export/attributes.rs @@ -11,6 +11,7 @@ use syn::{ pub struct ExportAttributeArguments { pub(crate) async_runtime: Option, pub(crate) callback_interface: Option, + pub(crate) with_foreign: Option, pub(crate) constructor: Option, // tried to make this a vec but that got messy quickly... pub(crate) trait_debug: Option, @@ -40,6 +41,11 @@ impl UniffiAttributeArgs for ExportAttributeArguments { callback_interface: input.parse()?, ..Self::default() }) + } else if lookahead.peek(kw::with_foreign) { + Ok(Self { + with_foreign: input.parse()?, + ..Self::default() + }) } else if lookahead.peek(kw::constructor) { Ok(Self { constructor: input.parse()?, @@ -71,18 +77,26 @@ impl UniffiAttributeArgs for ExportAttributeArguments { } fn merge(self, other: Self) -> syn::Result { - Ok(Self { + let merged = Self { async_runtime: either_attribute_arg(self.async_runtime, other.async_runtime)?, callback_interface: either_attribute_arg( self.callback_interface, other.callback_interface, )?, + with_foreign: either_attribute_arg(self.with_foreign, other.with_foreign)?, constructor: either_attribute_arg(self.constructor, other.constructor)?, trait_debug: either_attribute_arg(self.trait_debug, other.trait_debug)?, trait_display: either_attribute_arg(self.trait_display, other.trait_display)?, trait_hash: either_attribute_arg(self.trait_hash, other.trait_hash)?, trait_eq: either_attribute_arg(self.trait_eq, other.trait_eq)?, - }) + }; + if merged.callback_interface.is_some() && merged.with_foreign.is_some() { + return Err(syn::Error::new( + merged.callback_interface.unwrap().span, + "`callback_interface` and `with_foreign` are mutually exclusive", + )); + } + Ok(merged) } } diff --git a/uniffi_macros/src/export/item.rs b/uniffi_macros/src/export/item.rs index 4388acbeac..1e2be656b9 100644 --- a/uniffi_macros/src/export/item.rs +++ b/uniffi_macros/src/export/item.rs @@ -21,7 +21,8 @@ pub(super) enum ExportItem { Trait { self_ident: Ident, items: Vec, - callback_interface: bool, + with_foreign: bool, + callback_interface_only: bool, docstring: String, }, Struct { @@ -39,7 +40,11 @@ impl ExportItem { Ok(Self::Function { sig }) } syn::Item::Impl(item) => Self::from_impl(item, args.constructor.is_some()), - syn::Item::Trait(item) => Self::from_trait(item, args.callback_interface.is_some()), + syn::Item::Trait(item) => Self::from_trait( + item, + args.callback_interface.is_some() || args.with_foreign.is_some(), + args.callback_interface.is_some(), + ), syn::Item::Struct(item) => Self::from_struct(item, args), // FIXME: Support const / static? _ => Err(syn::Error::new( @@ -117,7 +122,11 @@ impl ExportItem { }) } - fn from_trait(item: syn::ItemTrait, callback_interface: bool) -> syn::Result { + fn from_trait( + item: syn::ItemTrait, + with_foreign: bool, + callback_interface_only: bool, + ) -> syn::Result { if !item.generics.params.is_empty() || item.generics.where_clause.is_some() { return Err(syn::Error::new_spanned( &item.generics, @@ -165,7 +174,8 @@ impl ExportItem { Ok(Self::Trait { items, self_ident, - callback_interface, + with_foreign, + callback_interface_only, docstring, }) } diff --git a/uniffi_macros/src/export/trait_interface.rs b/uniffi_macros/src/export/trait_interface.rs index 3b7f3cfa08..a27694214b 100644 --- a/uniffi_macros/src/export/trait_interface.rs +++ b/uniffi_macros/src/export/trait_interface.rs @@ -5,6 +5,8 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::{quote, quote_spanned}; +use uniffi_meta::ObjectImpl; + use crate::{ export::{ attributes::ExportAttributeArguments, callback_interface, gen_method_scaffolding, @@ -20,14 +22,17 @@ pub(super) fn gen_trait_scaffolding( self_ident: Ident, items: Vec, udl_mode: bool, + with_foreign: bool, docstring: String, ) -> syn::Result { if let Some(rt) = args.async_runtime { return Err(syn::Error::new_spanned(rt, "not supported for traits")); } let trait_name = ident_to_string(&self_ident); - let trait_impl = callback_interface::trait_impl(mod_path, &self_ident, &items) - .unwrap_or_else(|e| e.into_compile_error()); + let trait_impl = with_foreign.then(|| { + callback_interface::trait_impl(mod_path, &self_ident, &items) + .unwrap_or_else(|e| e.into_compile_error()) + }); let clone_fn_ident = Ident::new( &uniffi_meta::clone_fn_symbol_name(mod_path, &trait_name), @@ -94,10 +99,15 @@ pub(super) fn gen_trait_scaffolding( .collect::>()?; let meta_static_var = (!udl_mode).then(|| { - interface_meta_static_var(&self_ident, true, mod_path, docstring) + let imp = if with_foreign { + ObjectImpl::CallbackTrait + } else { + ObjectImpl::Trait + }; + interface_meta_static_var(&self_ident, imp, mod_path, docstring) .unwrap_or_else(syn::Error::into_compile_error) }); - let ffi_converter_tokens = ffi_converter(mod_path, &self_ident, udl_mode); + let ffi_converter_tokens = ffi_converter(mod_path, &self_ident, udl_mode, with_foreign); Ok(quote_spanned! { self_ident.span() => #meta_static_var @@ -108,11 +118,36 @@ pub(super) fn gen_trait_scaffolding( }) } -pub(crate) fn ffi_converter(mod_path: &str, trait_ident: &Ident, udl_mode: bool) -> TokenStream { +pub(crate) fn ffi_converter( + mod_path: &str, + trait_ident: &Ident, + udl_mode: bool, + with_foreign: bool, +) -> TokenStream { let impl_spec = tagged_impl_header("FfiConverterArc", "e! { dyn #trait_ident }, udl_mode); let lift_ref_impl_spec = tagged_impl_header("LiftRef", "e! { dyn #trait_ident }, udl_mode); let trait_name = ident_to_string(trait_ident); - let trait_impl_ident = callback_interface::trait_impl_ident(&trait_name); + let try_lift = if with_foreign { + let trait_impl_ident = callback_interface::trait_impl_ident(&trait_name); + quote! { + fn try_lift(v: Self::FfiType) -> ::uniffi::deps::anyhow::Result<::std::sync::Arc> { + Ok(::std::sync::Arc::new(<#trait_impl_ident>::new(v as u64))) + } + } + } else { + quote! { + fn try_lift(v: Self::FfiType) -> ::uniffi::deps::anyhow::Result<::std::sync::Arc> { + unsafe { + Ok(*::std::boxed::Box::from_raw(v as *mut ::std::sync::Arc)) + } + } + } + }; + let metadata_code = if with_foreign { + quote! { ::uniffi::metadata::codes::TYPE_CALLBACK_TRAIT_INTERFACE } + } else { + quote! { ::uniffi::metadata::codes::TYPE_TRAIT_INTERFACE } + }; quote! { // All traits must be `Sync + Send`. The generated scaffolding will fail to compile @@ -128,9 +163,7 @@ pub(crate) fn ffi_converter(mod_path: &str, trait_ident: &Ident, udl_mode: bool) ::std::boxed::Box::into_raw(::std::boxed::Box::new(obj)) as *const ::std::os::raw::c_void } - fn try_lift(v: Self::FfiType) -> ::uniffi::deps::anyhow::Result<::std::sync::Arc> { - Ok(::std::sync::Arc::new(<#trait_impl_ident>::new(v as u64))) - } + #try_lift fn write(obj: ::std::sync::Arc, buf: &mut Vec) { ::uniffi::deps::static_assertions::const_assert!(::std::mem::size_of::<*const ::std::ffi::c_void>() <= 8); @@ -147,10 +180,9 @@ pub(crate) fn ffi_converter(mod_path: &str, trait_ident: &Ident, udl_mode: bool) ::uniffi::deps::bytes::Buf::get_u64(buf) as Self::FfiType) } - const TYPE_ID_META: ::uniffi::MetadataBuffer = ::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::TYPE_INTERFACE) + const TYPE_ID_META: ::uniffi::MetadataBuffer = ::uniffi::MetadataBuffer::from_code(#metadata_code) .concat_str(#mod_path) - .concat_str(#trait_name) - .concat_bool(true); + .concat_str(#trait_name); } unsafe #lift_ref_impl_spec { diff --git a/uniffi_macros/src/object.rs b/uniffi_macros/src/object.rs index b63fb0ab7e..6bcc07a14e 100644 --- a/uniffi_macros/src/object.rs +++ b/uniffi_macros/src/object.rs @@ -5,6 +5,7 @@ use syn::DeriveInput; use crate::util::{ create_metadata_items, extract_docstring, ident_to_string, mod_path, tagged_impl_header, }; +use uniffi_meta::ObjectImpl; pub fn expand_object(input: DeriveInput, udl_mode: bool) -> syn::Result { let module_path = mod_path()?; @@ -20,7 +21,7 @@ pub fn expand_object(input: DeriveInput, udl_mode: bool) -> syn::Result TokenStream { const TYPE_ID_META: ::uniffi::MetadataBuffer = ::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::TYPE_INTERFACE) .concat_str(#mod_path) - .concat_str(#name) - .concat_bool(false); + .concat_str(#name); } unsafe #lower_return_impl_spec { @@ -154,20 +154,24 @@ pub(crate) fn interface_impl(ident: &Ident, udl_mode: bool) -> TokenStream { pub(crate) fn interface_meta_static_var( ident: &Ident, - is_trait: bool, + imp: ObjectImpl, module_path: &str, docstring: String, ) -> syn::Result { let name = ident_to_string(ident); + let code = match imp { + ObjectImpl::Struct => quote! { ::uniffi::metadata::codes::INTERFACE }, + ObjectImpl::Trait => quote! { ::uniffi::metadata::codes::TRAIT_INTERFACE }, + ObjectImpl::CallbackTrait => quote! { ::uniffi::metadata::codes::CALLBACK_TRAIT_INTERFACE }, + }; Ok(create_metadata_items( "interface", &name, quote! { - ::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::INTERFACE) + ::uniffi::MetadataBuffer::from_code(#code) .concat_str(#module_path) .concat_str(#name) - .concat_bool(#is_trait) .concat_long_str(#docstring) }, None, diff --git a/uniffi_macros/src/util.rs b/uniffi_macros/src/util.rs index d419d80b70..4268fffdea 100644 --- a/uniffi_macros/src/util.rs +++ b/uniffi_macros/src/util.rs @@ -247,6 +247,7 @@ pub(crate) fn derive_ffi_traits(ty: &Ident, udl_mode: bool, trait_names: &[&str] pub mod kw { syn::custom_keyword!(async_runtime); syn::custom_keyword!(callback_interface); + syn::custom_keyword!(with_foreign); syn::custom_keyword!(constructor); syn::custom_keyword!(default); syn::custom_keyword!(flat_error); diff --git a/uniffi_meta/src/metadata.rs b/uniffi_meta/src/metadata.rs index 7506b9d7ab..23cee48257 100644 --- a/uniffi_meta/src/metadata.rs +++ b/uniffi_meta/src/metadata.rs @@ -7,7 +7,7 @@ // `uniffi_core`. // This is the easy way out of that issue and is a temporary hacky solution. -/// Metadata constants, make sure to keep this in sync with copy in `uniffi_meta::reader` +/// Metadata constants, make sure to keep this in sync with copy in `uniffi_core::metadata` pub mod codes { // Top-level metadata item codes pub const FUNC: u8 = 0; @@ -22,6 +22,8 @@ pub mod codes { pub const CALLBACK_INTERFACE: u8 = 9; pub const TRAIT_METHOD: u8 = 10; pub const UNIFFI_TRAIT: u8 = 11; + pub const TRAIT_INTERFACE: u8 = 12; + pub const CALLBACK_TRAIT_INTERFACE: u8 = 13; //pub const UNKNOWN: u8 = 255; // Type codes @@ -49,7 +51,8 @@ pub mod codes { pub const TYPE_CALLBACK_INTERFACE: u8 = 21; pub const TYPE_CUSTOM: u8 = 22; pub const TYPE_RESULT: u8 = 23; - //pub const TYPE_FUTURE: u8 = 24; + pub const TYPE_TRAIT_INTERFACE: u8 = 24; + pub const TYPE_CALLBACK_TRAIT_INTERFACE: u8 = 25; pub const TYPE_UNIT: u8 = 255; // Literal codes diff --git a/uniffi_meta/src/reader.rs b/uniffi_meta/src/reader.rs index fa7f4447e9..5d0e78eb4f 100644 --- a/uniffi_meta/src/reader.rs +++ b/uniffi_meta/src/reader.rs @@ -54,7 +54,9 @@ impl<'a> MetadataReader<'a> { codes::RECORD => self.read_record()?.into(), codes::ENUM => self.read_enum(false)?.into(), codes::ERROR => self.read_error()?.into(), - codes::INTERFACE => self.read_object()?.into(), + codes::INTERFACE => self.read_object(ObjectImpl::Struct)?.into(), + codes::TRAIT_INTERFACE => self.read_object(ObjectImpl::Trait)?.into(), + codes::CALLBACK_TRAIT_INTERFACE => self.read_object(ObjectImpl::CallbackTrait)?.into(), codes::CALLBACK_INTERFACE => self.read_callback_interface()?.into(), codes::TRAIT_METHOD => self.read_trait_method()?.into(), codes::UNIFFI_TRAIT => self.read_uniffi_trait()?.into(), @@ -155,7 +157,17 @@ impl<'a> MetadataReader<'a> { codes::TYPE_INTERFACE => Type::Object { module_path: self.read_string()?, name: self.read_string()?, - imp: ObjectImpl::from_is_trait(self.read_bool()?), + imp: ObjectImpl::Struct, + }, + codes::TYPE_TRAIT_INTERFACE => Type::Object { + module_path: self.read_string()?, + name: self.read_string()?, + imp: ObjectImpl::Trait, + }, + codes::TYPE_CALLBACK_TRAIT_INTERFACE => Type::Object { + module_path: self.read_string()?, + name: self.read_string()?, + imp: ObjectImpl::CallbackTrait, }, codes::TYPE_CALLBACK_INTERFACE => Type::CallbackInterface { module_path: self.read_string()?, @@ -314,11 +326,11 @@ impl<'a> MetadataReader<'a> { Ok(ErrorMetadata::Enum { enum_, is_flat }) } - fn read_object(&mut self) -> Result { + fn read_object(&mut self, imp: ObjectImpl) -> Result { Ok(ObjectMetadata { module_path: self.read_string()?, name: self.read_string()?, - imp: ObjectImpl::from_is_trait(self.read_bool()?), + imp, docstring: self.read_optional_long_string()?, }) } diff --git a/uniffi_meta/src/types.rs b/uniffi_meta/src/types.rs index 647f4e9929..51bf156b50 100644 --- a/uniffi_meta/src/types.rs +++ b/uniffi_meta/src/types.rs @@ -21,8 +21,12 @@ use crate::Checksum; #[derive(Debug, Copy, Clone, Eq, PartialEq, Checksum, Ord, PartialOrd)] pub enum ObjectImpl { + // A single Rust type Struct, + // A trait that's can be implemented by Rust types Trait, + // A trait + a callback interface -- can be implemented by both Rust and foreign types. + CallbackTrait, } impl ObjectImpl { @@ -31,21 +35,19 @@ impl ObjectImpl { /// Includes `r#`, traits get a leading `dyn`. If we ever supported associated types, then /// this would also include them. pub fn rust_name_for(&self, name: &str) -> String { - if self == &ObjectImpl::Trait { + if self.is_trait_interface() { format!("dyn r#{name}") } else { format!("r#{name}") } } - // uniffi_meta and procmacro support tend to carry around `is_trait` bools. This makes that - // mildly less painful - pub fn from_is_trait(is_trait: bool) -> Self { - if is_trait { - ObjectImpl::Trait - } else { - ObjectImpl::Struct - } + pub fn is_trait_interface(&self) -> bool { + matches!(self, Self::Trait | Self::CallbackTrait) + } + + pub fn has_callback_interface(&self) -> bool { + matches!(self, Self::CallbackTrait) } } diff --git a/uniffi_udl/src/attributes.rs b/uniffi_udl/src/attributes.rs index 1df337c9e6..9c063e6336 100644 --- a/uniffi_udl/src/attributes.rs +++ b/uniffi_udl/src/attributes.rs @@ -44,6 +44,8 @@ pub(super) enum Attribute { Custom, // The interface described is implemented as a trait. Trait, + // Modifies `Trait` to enable foreign implementations (callback interfaces) + WithForeign, Async, } @@ -52,6 +54,7 @@ pub(super) enum Attribute { #[derive(Debug, Copy, Clone, Checksum)] pub(super) enum RustKind { Object, + CallbackTrait, Trait, Record, Enum, @@ -82,6 +85,7 @@ impl TryFrom<&weedle::attribute::ExtendedAttribute<'_>> for Attribute { "Error" => Ok(Attribute::Error), "Custom" => Ok(Attribute::Custom), "Trait" => Ok(Attribute::Trait), + "WithForeign" => Ok(Attribute::WithForeign), "Async" => Ok(Attribute::Async), _ => anyhow::bail!("ExtendedAttributeNoArgs not supported: {:?}", (attr.0).0), }, @@ -170,6 +174,7 @@ fn rust_kind_from_id_or_string(nm: &weedle::attribute::IdentifierOrString<'_>) - "enum" => RustKind::Enum, "trait" => RustKind::Trait, "callback" => RustKind::CallbackInterface, + "trait_with_foreign" => RustKind::CallbackTrait, _ => anyhow::bail!("Unknown `[Rust=]` kind {:?}", str_lit.0), }, weedle::attribute::IdentifierOrString::Identifier(_) => { @@ -347,12 +352,21 @@ impl InterfaceAttributes { self.0.iter().any(|attr| attr.is_error()) } - pub fn object_impl(&self) -> ObjectImpl { - if self.0.iter().any(|attr| matches!(attr, Attribute::Trait)) { - ObjectImpl::Trait - } else { - ObjectImpl::Struct - } + pub fn contains_trait(&self) -> bool { + self.0.iter().any(|attr| matches!(attr, Attribute::Trait)) + } + + pub fn contains_with_foreign(&self) -> bool { + self.0.iter().any(|attr| matches!(attr, Attribute::WithForeign)) + } + + pub fn object_impl(&self) -> Result { + Ok(match (self.contains_trait(), self.contains_with_foreign()) { + (true, true) => ObjectImpl::CallbackTrait, + (true, false) => ObjectImpl::Trait, + (false, false) => ObjectImpl::Struct, + (false, true) => bail!("WithForeign can't be specified without Trait") + }) } pub fn get_traits(&self) -> Vec { self.0 @@ -374,6 +388,7 @@ impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for InterfaceAttribu Attribute::Enum => Ok(()), Attribute::Error => Ok(()), Attribute::Trait => Ok(()), + Attribute::WithForeign => Ok(()), Attribute::Traits(_) => Ok(()), _ => bail!(format!("{attr:?} not supported for interface definition")), })?; @@ -838,11 +853,19 @@ mod test { fn test_trait_attribute() { let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[Trait]").unwrap(); let attrs = InterfaceAttributes::try_from(&node).unwrap(); - assert_eq!(attrs.object_impl(), ObjectImpl::Trait); + assert_eq!(attrs.object_impl().unwrap(), ObjectImpl::Trait); + + let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[Trait, WithForeign]").unwrap(); + let attrs = InterfaceAttributes::try_from(&node).unwrap(); + assert_eq!(attrs.object_impl().unwrap(), ObjectImpl::CallbackTrait); let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[]").unwrap(); let attrs = InterfaceAttributes::try_from(&node).unwrap(); - assert_eq!(attrs.object_impl(), ObjectImpl::Struct); + assert_eq!(attrs.object_impl().unwrap(), ObjectImpl::Struct); + + let (_, node) = weedle::attribute::ExtendedAttributeList::parse("[WithForeign]").unwrap(); + let attrs = InterfaceAttributes::try_from(&node).unwrap(); + assert!(attrs.object_impl().is_err()) } #[test] diff --git a/uniffi_udl/src/converters/interface.rs b/uniffi_udl/src/converters/interface.rs index d0d7493428..ef9bdd9540 100644 --- a/uniffi_udl/src/converters/interface.rs +++ b/uniffi_udl/src/converters/interface.rs @@ -23,7 +23,7 @@ impl APIConverter for weedle::InterfaceDefinition<'_> { }; let object_name = self.identifier.0; - let object_impl = attributes.object_impl(); + let object_impl = attributes.object_impl()?; // Convert each member into a constructor or method, guarding against duplicate names. // They get added to the ci and aren't carried in ObjectMetadata. let mut member_names = HashSet::new(); diff --git a/uniffi_udl/src/finder.rs b/uniffi_udl/src/finder.rs index fd369f7ca6..259557ad07 100644 --- a/uniffi_udl/src/finder.rs +++ b/uniffi_udl/src/finder.rs @@ -75,7 +75,7 @@ impl TypeFinder for weedle::InterfaceDefinition<'_> { Type::Object { name, module_path: types.module_path(), - imp: attrs.object_impl(), + imp: attrs.object_impl()?, }, ) } @@ -142,6 +142,11 @@ impl TypeFinder for weedle::TypedefDefinition<'_> { name, imp: ObjectImpl::Trait, }, + Some(RustKind::CallbackTrait) => Type::Object { + module_path, + name, + imp: ObjectImpl::CallbackTrait, + }, Some(RustKind::Record) => Type::Record { module_path, name }, Some(RustKind::Enum) => Type::Enum { module_path, name }, Some(RustKind::CallbackInterface) => Type::CallbackInterface { module_path, name },