From b48013bfce63fdec313d168238178fcaa4c2fd2c Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Fri, 3 Feb 2023 12:37:44 -0500 Subject: [PATCH] Added return handling to `FfiConverter` Added the `FfiConverter::lower_return` method. This is like `lower()` but specialized for scaffolding returns. This allows us to always use a single function to handle scaffolding calls, rather than `call_with_output` or `call_with_result` depending on if the return type is a `Result<>` or not. Having a single code-path for return handling simplifies the code generation, especially the macros. We no longer need to try to parse `Result<>` type names. This is especially useful for crates that type-alias their `Result<>` types. Updated the async code to work with the new system. Now `RustFuture` only needs 1 generic argument. Updated `object_references.md` and removed example code that's no longer valid. Replaced it with higher-level descriptions of what's going on, hopefully this will stay not get out of date as quickly. --- .../manual/src/internals/object_references.md | 102 ++------- fixtures/keywords/rust/src/lib.rs | 2 +- fixtures/metadata/src/tests.rs | 3 +- .../tests/ui/non_hashable_record_key.stderr | 78 ++----- uniffi_bindgen/src/interface/error.rs | 5 +- uniffi_bindgen/src/interface/function.rs | 36 ++++ uniffi_bindgen/src/interface/mod.rs | 2 +- uniffi_bindgen/src/interface/object.rs | 38 +++- uniffi_bindgen/src/macro_metadata/ci.rs | 2 +- uniffi_bindgen/src/scaffolding/mod.rs | 28 ++- .../templates/CallbackInterfaceTemplate.rs | 2 + .../templates/ExternalTypesTemplate.rs | 2 + .../scaffolding/templates/ObjectTemplate.rs | 26 ++- .../templates/TopLevelFunctionTemplate.rs | 6 +- .../src/scaffolding/templates/macros.rs | 73 +------ uniffi_core/src/ffi/mod.rs | 2 +- uniffi_core/src/ffi/rustbuffer.rs | 19 +- uniffi_core/src/ffi/rustcalls.rs | 119 ++++------- uniffi_core/src/ffi/rustfuture.rs | 82 +++----- uniffi_core/src/ffi_converter_impls.rs | 61 +++++- uniffi_core/src/lib.rs | 58 +++++- uniffi_core/src/metadata.rs | 2 + uniffi_macros/src/enum_.rs | 1 + uniffi_macros/src/error.rs | 15 +- uniffi_macros/src/export.rs | 30 +-- uniffi_macros/src/export/metadata/convert.rs | 61 ------ uniffi_macros/src/export/scaffolding.rs | 196 ++++-------------- uniffi_macros/src/object.rs | 3 +- uniffi_macros/src/record.rs | 1 + uniffi_meta/src/reader.rs | 168 +++++++++------ 30 files changed, 540 insertions(+), 683 deletions(-) diff --git a/docs/manual/src/internals/object_references.md b/docs/manual/src/internals/object_references.md index c4e479649e..902eb7940b 100644 --- a/docs/manual/src/internals/object_references.md +++ b/docs/manual/src/internals/object_references.md @@ -56,91 +56,31 @@ interface TodoList { }; ``` -On the Rust side of the generated bindings, the instance constructor will create an instance of the -corresponding `TodoList` Rust struct, wrap it in an `Arc<>` and return the Arc's raw pointer to the -foreign language code: - -```rust -pub extern "C" fn todolist_12ba_TodoList_new( - err: &mut uniffi::deps::ffi_support::ExternError, -) -> *const std::os::raw::c_void /* *const TodoList */ { - uniffi::deps::ffi_support::call_with_output(err, || { - let _new = TodoList::new(); - let _arc = std::sync::Arc::new(_new); - as uniffi::FfiConverter>::lower(_arc) - }) -} -``` - -The UniFFI runtime implements lowering for object instances using `Arc::into_raw`: - -```rust -unsafe impl FfiConverter for std::sync::Arc { - type FfiType = *const std::os::raw::c_void; - fn lower(self) -> Self::FfiType { - std::sync::Arc::into_raw(self) as Self::FfiType - } -} -``` - -which does the "arc to pointer" dance for us. Note that this has "leaked" the -`Arc<>` reference out of Rusts ownership system and given it to the foreign-language code. -The foreign-language code must pass that pointer back into Rust in order to free it, -or our instance will leak. - -When invoking a method on the instance, the foreign-language code passes the -raw pointer back to the Rust code, conceptually passing a "borrow" of the `Arc<>` to -the Rust scaffolding. The Rust side turns it back into a cloned `Arc<>` which -lives for the duration of the method call: - -```rust -pub extern "C" fn todolist_12ba_TodoList_add_item( - ptr: *const std::os::raw::c_void, - todo: uniffi::RustBuffer, - err: &mut uniffi::deps::ffi_support::ExternError, -) -> () { - uniffi::deps::ffi_support::call_with_result(err, || -> Result<_, TodoError> { - let _retval = TodoList::add_item( - & as uniffi::FfiConverter>::try_lift(ptr).unwrap(), - ::try_lift(todo).unwrap())?, - ) - Ok(_retval) - }) -} -``` - -The UniFFI runtime implements lifting for object instances using `Arc::from_raw`: - -```rust -unsafe impl FfiConverter for std::sync::Arc { - type FfiType = *const std::os::raw::c_void; - fn try_lift(v: Self::FfiType) -> Result { - let v = v as *const T; - // We musn't drop the `Arc` that is owned by the foreign-language code. - let foreign_arc = std::mem::ManuallyDrop::new(unsafe { Self::from_raw(v) }); - // Take a clone for our own use. - Ok(std::sync::Arc::clone(&*foreign_arc)) - } -``` - -Notice that we take care to ensure the reference that is owned by the foreign-language -code remains alive. +On the Rust side of the generated bindings: + - The instance constructor will create an instance of the corresponding `TodoList` Rust struct + - The owned value is wrapped it in an `Arc<>` + - The `Arc<>` is lowered into the foreign code using `Arc::into_raw` and returned as an object pointer. + +This is the "arc to pointer" dance. Note that this has "leaked" the `Arc<>` +reference out of Rusts ownership system and given it to the foreign-language +code. The foreign-language code must pass that pointer back into Rust in order +to free it, or our instance will leak. + +When invoking a method on the instance: + - The foreign-language code passes the raw pointer back to the Rust code, conceptually passing a "borrow" of the `Arc<>` to the Rust scaffolding. + - The Rust side calls `Arc::from_raw` to convert the pointer into an an `Arc<>` + - It wraps the `Arc` in `std::mem::ManuallyDrop<>`, which we never actually + drop. This is because the Rust side is are borrowing the Arc and shouldn't + run the destructor and decrement the reference count. + - The `Arc<>` is cloned and returned to the Rust code Finally, when the foreign-language code frees the instance, it passes the raw pointer a special destructor function so that the Rust code can drop that initial reference (and if that happens to be the final reference, -the Rust object will be dropped.) - -```rust -pub extern "C" fn ffi_todolist_12ba_TodoList_object_free(ptr: *const std::os::raw::c_void) { - if let Err(e) = std::panic::catch_unwind(|| { - assert!(!ptr.is_null()); - unsafe { std::sync::Arc::from_raw(ptr as *const TodoList) }; - }) { - uniffi::deps::log::error!("ffi_todolist_12ba_TodoList_object_free panicked: {:?}", e); - } -} -``` +the Rust object will be dropped.). This simply calls `Arc::from_raw`, then +lets the value drop. Passing instances as arguments and returning them as values works similarly, except that UniFFI does not automatically wrap/unwrap the containing `Arc`. + +To see this in action, use `cargo expand` to see the exact generated code. diff --git a/fixtures/keywords/rust/src/lib.rs b/fixtures/keywords/rust/src/lib.rs index 0f58779bd1..5b241bb492 100644 --- a/fixtures/keywords/rust/src/lib.rs +++ b/fixtures/keywords/rust/src/lib.rs @@ -24,7 +24,7 @@ impl r#break { } #[allow(non_camel_case_types)] -trait r#continue { +pub trait r#continue { fn r#return(&self, v: r#return) -> r#return; fn r#continue(&self, v: Vec>) -> Option>; fn r#break(&self, _v: Option>) -> HashMap>; diff --git a/fixtures/metadata/src/tests.rs b/fixtures/metadata/src/tests.rs index a3620fb30b..989609f230 100644 --- a/fixtures/metadata/src/tests.rs +++ b/fixtures/metadata/src/tests.rs @@ -4,7 +4,7 @@ /// This entire crate is just a set of tests for metadata handling. We use a separate crate /// for testing because the metadata handling is split up between several crates, and no crate -/// on all the functionality. +/// owns all the functionality. use crate::UniFfiTag; use uniffi_meta::*; @@ -342,7 +342,6 @@ mod test_function_metadata { unimplemented!() } - #[uniffi::export] impl Calculator { #[allow(unused)] diff --git a/fixtures/uitests/tests/ui/non_hashable_record_key.stderr b/fixtures/uitests/tests/ui/non_hashable_record_key.stderr index d978710953..5aff491060 100644 --- a/fixtures/uitests/tests/ui/non_hashable_record_key.stderr +++ b/fixtures/uitests/tests/ui/non_hashable_record_key.stderr @@ -1,60 +1,14 @@ -error[E0277]: the trait bound `f32: std::cmp::Eq` is not satisfied - --> $OUT_DIR[uniffi_uitests]/records.uniffi.rs - | - | uniffi::deps::static_assertions::assert_impl_all!(f32: ::std::cmp::Eq, ::std::hash::Hash); // record - | ^^^ the trait `std::cmp::Eq` is not implemented for `f32` - | - = help: the following other types implement trait `std::cmp::Eq`: - i128 - i16 - i32 - i64 - i8 - isize - u128 - u16 - and $N others -note: required by a bound in `assert_impl_all` - --> $OUT_DIR[uniffi_uitests]/records.uniffi.rs - | - | uniffi::deps::static_assertions::assert_impl_all!(f32: ::std::cmp::Eq, ::std::hash::Hash); // record - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all` - = note: this error originates in the macro `uniffi::deps::static_assertions::assert_impl_all` (in Nightly builds, run with -Z macro-backtrace for more info) - -error[E0277]: the trait bound `f32: Hash` is not satisfied - --> $OUT_DIR[uniffi_uitests]/records.uniffi.rs - | - | uniffi::deps::static_assertions::assert_impl_all!(f32: ::std::cmp::Eq, ::std::hash::Hash); // record - | ^^^ the trait `Hash` is not implemented for `f32` - | - = help: the following other types implement trait `Hash`: - i128 - i16 - i32 - i64 - i8 - isize - u128 - u16 - and $N others -note: required by a bound in `assert_impl_all` - --> $OUT_DIR[uniffi_uitests]/records.uniffi.rs - | - | uniffi::deps::static_assertions::assert_impl_all!(f32: ::std::cmp::Eq, ::std::hash::Hash); // record - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `assert_impl_all` - = note: this error originates in the macro `uniffi::deps::static_assertions::assert_impl_all` (in Nightly builds, run with -Z macro-backtrace for more info) - -error[E0425]: cannot find function `get_dict` in this scope - --> $OUT_DIR[uniffi_uitests]/records.uniffi.rs - | - | as uniffi::FfiConverter>::lower(r#get_dict()) - | ^^^^^^^^^^ not found in this scope - error[E0277]: the trait bound `f32: Hash` is not satisfied --> $OUT_DIR[uniffi_uitests]/records.uniffi.rs | - | as uniffi::FfiConverter>::lower(r#get_dict()) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `Hash` is not implemented for `f32` + | / pub extern "C" fn r#uitests_a12c_get_dict( + | | + | | call_status: &mut uniffi::RustCallStatus + | | ) -> as ::uniffi::FfiConverter>::ReturnType { +... | + | | ) + | | } + | |_^ the trait `Hash` is not implemented for `f32` | = help: the following other types implement trait `Hash`: i128 @@ -71,8 +25,14 @@ error[E0277]: the trait bound `f32: Hash` is not satisfied error[E0277]: the trait bound `f32: std::cmp::Eq` is not satisfied --> $OUT_DIR[uniffi_uitests]/records.uniffi.rs | - | as uniffi::FfiConverter>::lower(r#get_dict()) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::cmp::Eq` is not implemented for `f32` + | / pub extern "C" fn r#uitests_a12c_get_dict( + | | + | | call_status: &mut uniffi::RustCallStatus + | | ) -> as ::uniffi::FfiConverter>::ReturnType { +... | + | | ) + | | } + | |_^ the trait `std::cmp::Eq` is not implemented for `f32` | = help: the following other types implement trait `std::cmp::Eq`: i128 @@ -85,3 +45,9 @@ error[E0277]: the trait bound `f32: std::cmp::Eq` is not satisfied u16 and $N others = note: required for `HashMap` to implement `FfiConverter` + +error[E0425]: cannot find function `get_dict` in this scope + --> $OUT_DIR[uniffi_uitests]/records.uniffi.rs + | + | r#get_dict()) + | ^^^^^^^^^^ not found in this scope diff --git a/uniffi_bindgen/src/interface/error.rs b/uniffi_bindgen/src/interface/error.rs index 9aa57255e8..dd3a3357ec 100644 --- a/uniffi_bindgen/src/interface/error.rs +++ b/uniffi_bindgen/src/interface/error.rs @@ -92,9 +92,8 @@ use super::{APIConverter, ComponentInterface}; /// Represents an Error that might be thrown by functions/methods in the component interface. /// /// Errors are represented in the UDL as enums with the special `[Error]` attribute, but -/// they're handled in the FFI very differently. We create them in `uniffi::call_with_result()` if -/// the wrapped function returns an `Err` value -/// struct and assign an integer error code to each variant. +/// they're handled in the FFI very differently. Defining an error means that exported functions +/// cat return a `Result` where `R` is a UniFFI type and `E` is the error type. #[derive(Debug, Clone, PartialEq, Eq, Checksum)] pub struct Error { pub name: String, diff --git a/uniffi_bindgen/src/interface/function.rs b/uniffi_bindgen/src/interface/function.rs index 2fc6e3807f..d283451f62 100644 --- a/uniffi_bindgen/src/interface/function.rs +++ b/uniffi_bindgen/src/interface/function.rs @@ -255,6 +255,42 @@ impl APIConverter for weedle::argument::SingleArgument<'_> { } } +/// Implemented by function-like types (Function, Method, Constructor) +pub trait Callable { + fn arguments(&self) -> Vec<&Argument>; + fn return_type(&self) -> Option; + fn throws_type(&self) -> Option; +} + +impl Callable for Function { + fn arguments(&self) -> Vec<&Argument> { + self.arguments() + } + + fn return_type(&self) -> Option { + self.return_type().cloned() + } + + fn throws_type(&self) -> Option { + self.throws_type() + } +} + +// Needed because Askama likes to add extra refs to variables +impl Callable for &T { + fn arguments(&self) -> Vec<&Argument> { + (*self).arguments() + } + + fn return_type(&self) -> Option { + (*self).return_type() + } + + fn throws_type(&self) -> Option { + (*self).throws_type() + } +} + #[cfg(test)] mod test { use super::*; diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index 6fe31c8411..584f8a467b 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -64,7 +64,7 @@ pub use enum_::Enum; mod error; pub use error::Error; mod function; -pub use function::{Argument, Function}; +pub use function::{Argument, Callable, Function}; mod literal; pub use literal::{Literal, Radix}; mod namespace; diff --git a/uniffi_bindgen/src/interface/object.rs b/uniffi_bindgen/src/interface/object.rs index 48c14fd725..e0f6874731 100644 --- a/uniffi_bindgen/src/interface/object.rs +++ b/uniffi_bindgen/src/interface/object.rs @@ -65,7 +65,7 @@ use uniffi_meta::Checksum; use super::attributes::{Attribute, ConstructorAttributes, InterfaceAttributes, MethodAttributes}; use super::ffi::{FfiArgument, FfiFunction, FfiType}; -use super::function::Argument; +use super::function::{Argument, Callable}; use super::types::{Type, TypeIterator}; use super::{convert_type, APIConverter, ComponentInterface}; @@ -211,10 +211,11 @@ impl APIConverter for weedle::InterfaceDefinition<'_> { for member in &self.members.body { match member { weedle::interface::InterfaceMember::Constructor(t) => { - let cons: Constructor = t.convert(ci)?; + let mut cons: Constructor = t.convert(ci)?; if !member_names.insert(cons.name.clone()) { bail!("Duplicate interface member name: \"{}\"", cons.name()) } + cons.object_name = object.name.clone(); object.constructors.push(cons); } weedle::interface::InterfaceMember::Operation(t) => { @@ -239,6 +240,7 @@ impl APIConverter for weedle::InterfaceDefinition<'_> { #[derive(Debug, Clone, Checksum)] pub struct Constructor { pub(super) name: String, + pub(super) object_name: String, pub(super) arguments: Vec, // We don't include the FFIFunc in the hash calculation, because: // - it is entirely determined by the other fields, @@ -301,6 +303,8 @@ impl Default for Constructor { fn default() -> Self { Constructor { name: String::from("new"), + // We don't know the name of the containing `Object` at this point, fill it in later. + object_name: Default::default(), arguments: Vec::new(), ffi_func: Default::default(), attributes: Default::default(), @@ -316,6 +320,8 @@ impl APIConverter for weedle::interface::ConstructorInterfaceMember }; Ok(Constructor { name: String::from(attributes.get_name().unwrap_or("new")), + // We don't know the name of the containing `Object` at this point, fill it in later. + object_name: Default::default(), arguments: self.args.body.list.convert(ci)?, ffi_func: Default::default(), attributes, @@ -485,6 +491,34 @@ impl APIConverter for weedle::interface::OperationInterfaceMember<'_> { } } +impl Callable for Constructor { + fn arguments(&self) -> Vec<&Argument> { + self.arguments() + } + + fn return_type(&self) -> Option { + Some(Type::Object(self.object_name.clone())) + } + + fn throws_type(&self) -> Option { + self.throws_type() + } +} + +impl Callable for Method { + fn arguments(&self) -> Vec<&Argument> { + self.arguments() + } + + fn return_type(&self) -> Option { + self.return_type().cloned() + } + + fn throws_type(&self) -> Option { + self.throws_type() + } +} + #[cfg(test)] mod test { use super::*; diff --git a/uniffi_bindgen/src/macro_metadata/ci.rs b/uniffi_bindgen/src/macro_metadata/ci.rs index e72f08716d..811d3ecd55 100644 --- a/uniffi_bindgen/src/macro_metadata/ci.rs +++ b/uniffi_bindgen/src/macro_metadata/ci.rs @@ -48,7 +48,7 @@ pub fn add_to_ci( Some(ns) => ns, None => bail!("Unknown namespace for {item_desc} ({crate_name})"), }; - if item_ns != &iface_ns { + if item_ns != iface_ns { return Err(anyhow!("Found {item_desc} from crate `{crate_name}`.") .context(format!( "Main crate is expected to be named `{iface_ns}` based on the UDL namespace." diff --git a/uniffi_bindgen/src/scaffolding/mod.rs b/uniffi_bindgen/src/scaffolding/mod.rs index d56c2ad4f1..2a2c983586 100644 --- a/uniffi_bindgen/src/scaffolding/mod.rs +++ b/uniffi_bindgen/src/scaffolding/mod.rs @@ -53,6 +53,11 @@ mod filters { type_rs(v)? ), Type::Custom { name, .. } => format!("r#{name}"), + Type::External { + name, + kind: ExternalKind::Interface, + .. + } => format!("::std::sync::Arc"), Type::External { name, .. } => format!("r#{name}"), }) } @@ -86,15 +91,34 @@ mod filters { kind: ExternalKind::Interface, .. } => { - format!("<::std::sync::Arc as uniffi::FfiConverter>") + format!("<::std::sync::Arc as ::uniffi::FfiConverter>") } _ => format!( - "<{} as uniffi::FfiConverter>", + "<{} as ::uniffi::FfiConverter>", type_rs(type_)? ), }) } + // Map return types to their fully-qualified `FfiConverter` impl. + pub fn return_ffi_converter(callable: &T) -> Result { + let result_type = match callable.return_type() { + Some(t) => type_rs(&t)?, + None => "()".to_string(), + }; + Ok(match callable.throws_type() { + Some(e) => format!( + " as ::uniffi::FfiConverter>", + result_type, + type_rs(&e)? + ), + None => format!( + "<{} as ::uniffi::FfiConverter>", + result_type + ), + }) + } + // Turns a `crate-name` into the `crate_name` the .rs code needs to specify. pub fn crate_name_rs(nm: &str) -> Result { Ok(format!("r#{}", nm.to_string().to_snake_case())) diff --git a/uniffi_bindgen/src/scaffolding/templates/CallbackInterfaceTemplate.rs b/uniffi_bindgen/src/scaffolding/templates/CallbackInterfaceTemplate.rs index 11bc8cfdaf..3057f09b81 100644 --- a/uniffi_bindgen/src/scaffolding/templates/CallbackInterfaceTemplate.rs +++ b/uniffi_bindgen/src/scaffolding/templates/CallbackInterfaceTemplate.rs @@ -193,6 +193,8 @@ unsafe impl ::uniffi::FfiConverter for Box for r#{{ name }} { <{{ name }} as UniffiCustomTypeConverter>::into_custom({{ builtin|ffi_converter }}::try_read(buf)?) } + ::uniffi::ffi_converter_default_return!(crate::UniFfiTag); + const TYPE_ID_META: ::uniffi::MetadataBuffer = ::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::TYPE_CUSTOM) .concat_str("{{ name }}") .concat({{ builtin|ffi_converter }}::TYPE_ID_META); diff --git a/uniffi_bindgen/src/scaffolding/templates/ObjectTemplate.rs b/uniffi_bindgen/src/scaffolding/templates/ObjectTemplate.rs index eeace1d2f7..814be65177 100644 --- a/uniffi_bindgen/src/scaffolding/templates/ObjectTemplate.rs +++ b/uniffi_bindgen/src/scaffolding/templates/ObjectTemplate.rs @@ -37,10 +37,11 @@ uniffi::deps::static_assertions::assert_impl_all!(r#{{ obj.name() }}: Sync, Send #[doc(hidden)] #[no_mangle] pub extern "C" fn {{ ffi_free.name() }}(ptr: *const std::os::raw::c_void, call_status: &mut uniffi::RustCallStatus) { - uniffi::call_with_output(call_status, || { + uniffi::rust_call(call_status, || { assert!(!ptr.is_null()); {#- turn it into an Arc and explicitly drop it. #} - drop(unsafe { std::sync::Arc::from_raw(ptr as *const r#{{ obj.name() }}) }) + drop(unsafe { ::std::sync::Arc::from_raw(ptr as *const r#{{ obj.name() }}) }); + Ok(()) }) } @@ -48,7 +49,8 @@ pub extern "C" fn {{ ffi_free.name() }}(ptr: *const std::os::raw::c_void, call_s #[doc(hidden)] #[no_mangle] pub extern "C" fn r#{{ cons.ffi_func().name() }}( - {%- call rs::arg_list_ffi_decl(cons.ffi_func()) %}) -> *const std::os::raw::c_void /* *const {{ obj.name() }} */ { + {%- call rs::arg_list_ffi_decl(cons.ffi_func()) %} + ) -> *const std::os::raw::c_void /* *const {{ obj.name() }} */ { uniffi::deps::log::debug!("{{ cons.ffi_func().name() }}"); {% if obj.uses_deprecated_threadsafe_attribute() %} uniffi_note_threadsafe_deprecation_{{ obj.name() }}(); @@ -56,20 +58,32 @@ pub extern "C" fn {{ ffi_free.name() }}(ptr: *const std::os::raw::c_void, call_s // If the constructor does not have the same signature as declared in the UDL, then // this attempt to call it will fail with a (somewhat) helpful compiler error. - {% call rs::to_rs_constructor_call(obj, cons) %} + uniffi::rust_call(call_status, || { + {{ cons|return_ffi_converter }}::lower_return( + {%- if cons.throws() %} + r#{{ obj.name() }}::{% call rs::to_rs_call(cons) %}.map(::std::sync::Arc::new).map_err(Into::into) + {%- else %} + ::std::sync::Arc::new(r#{{ obj.name() }}::{% call rs::to_rs_call(cons) %}) + {%- endif %} + ) + }) } {%- endfor %} {%- for meth in obj.methods() %} #[doc(hidden)] #[no_mangle] - #[allow(clippy::let_unit_value)] // Sometimes we generate code that binds `_retval` to `()`. + #[allow(clippy::let_unit_value,clippy::unit_arg)] // The generated code uses the unit type like other types to keep things uniform pub extern "C" fn r#{{ meth.ffi_func().name() }}( {%- call rs::arg_list_ffi_decl(meth.ffi_func()) %} ) {% call rs::return_signature(meth) %} { uniffi::deps::log::debug!("{{ meth.ffi_func().name() }}"); // If the method does not have the same signature as declared in the UDL, then // this attempt to call it will fail with a (somewhat) helpful compiler error. - {% call rs::to_rs_method_call(obj, meth) %} + uniffi::rust_call(call_status, || { + {{ meth|return_ffi_converter }}::lower_return( + r#{{ obj.name() }}::{% call rs::to_rs_call(meth) %}{% if meth.throws() %}.map_err(Into::into){% endif %} + ) + }) } {% endfor %} diff --git a/uniffi_bindgen/src/scaffolding/templates/TopLevelFunctionTemplate.rs b/uniffi_bindgen/src/scaffolding/templates/TopLevelFunctionTemplate.rs index 0c6d5a47de..5a0cdcefcd 100644 --- a/uniffi_bindgen/src/scaffolding/templates/TopLevelFunctionTemplate.rs +++ b/uniffi_bindgen/src/scaffolding/templates/TopLevelFunctionTemplate.rs @@ -6,12 +6,14 @@ #} #[doc(hidden)] #[no_mangle] -#[allow(clippy::let_unit_value)] // Sometimes we generate code that binds `_retval` to `()`. +#[allow(clippy::let_unit_value,clippy::unit_arg)] // The generated code uses the unit type like other types to keep things uniform pub extern "C" fn r#{{ func.ffi_func().name() }}( {% call rs::arg_list_ffi_decl(func.ffi_func()) %} ) {% call rs::return_signature(func) %} { // If the provided function does not match the signature specified in the UDL // then this attempt to call it will not compile, and will give guidance as to why. uniffi::deps::log::debug!("{{ func.ffi_func().name() }}"); - {% call rs::to_rs_function_call(func) %} + uniffi::rust_call(call_status, || {{ func|return_ffi_converter }}::lower_return( + {% call rs::to_rs_call(func) %}){% if func.throws() %}.map_err(Into::into){% endif %} + ) } diff --git a/uniffi_bindgen/src/scaffolding/templates/macros.rs b/uniffi_bindgen/src/scaffolding/templates/macros.rs index 05f1730859..e472e4b111 100644 --- a/uniffi_bindgen/src/scaffolding/templates/macros.rs +++ b/uniffi_bindgen/src/scaffolding/templates/macros.rs @@ -50,70 +50,9 @@ r#{{ func.name() }}({% call _arg_list_rs_call(func) -%}) {%- endif %} {%- endmacro -%} -{% macro return_signature(func) %}{% match func.ffi_func().return_type() %}{% when Some with (return_type) %} -> {% call return_type_func(func) %}{%- else -%}{%- endmatch -%}{%- endmacro -%} - -{% macro return_type_func(func) %}{% match func.ffi_func().return_type() %}{% when Some with (return_type) %}{{ return_type|type_ffi }}{%- else -%}(){%- endmatch -%}{%- endmacro -%} - -{% macro ret(func) %}{% match func.return_type() %}{% when Some with (return_type) %}{{ return_type|ffi_converter }}::lower(_retval){% else %}_retval{% endmatch %}{% endmacro %} - -{% macro construct(obj, cons) %} - r#{{- obj.name() }}::{% call to_rs_call(cons) -%} -{% endmacro %} - -{% macro to_rs_constructor_call(obj, cons) %} -{% match cons.throws_type() %} -{% when Some with (e) %} - uniffi::call_with_result(call_status, || { - let _new = {% call construct(obj, cons) %}.map_err(Into::into).map_err({{ e|ffi_converter }}::lower)?; - let _arc = std::sync::Arc::new(_new); - Ok({{ obj.type_().borrow()|ffi_converter }}::lower(_arc)) - }) -{% else %} - uniffi::call_with_output(call_status, || { - let _new = {% call construct(obj, cons) %}; - let _arc = std::sync::Arc::new(_new); - {{ obj.type_().borrow()|ffi_converter }}::lower(_arc) - }) -{% endmatch %} -{% endmacro %} - -{% macro to_rs_method_call(obj, meth) -%} -{% match meth.throws_type() -%} -{% when Some with (e) -%} -uniffi::call_with_result(call_status, || { - let _retval = r#{{ obj.name() }}::{% call to_rs_call(meth) %}.map_err(Into::into).map_err({{ e|ffi_converter }}::lower)?; - Ok({% call ret(meth) %}) -}) -{% else %} -uniffi::call_with_output(call_status, || { - {% match meth.return_type() -%} - {% when Some with (return_type) -%} - let retval = r#{{ obj.name() }}::{% call to_rs_call(meth) %}; - {{ return_type|ffi_converter }}::lower(retval) - {% else -%} - r#{{ obj.name() }}::{% call to_rs_call(meth) %} - {% endmatch -%} -}) -{% endmatch -%} -{% endmacro -%} - -{% macro to_rs_function_call(func) %} -{% match func.throws_type() %} -{% when Some with (e) %} -uniffi::call_with_result(call_status, || { - let _retval = {% call to_rs_call(func) %}.map_err(Into::into).map_err({{ e|ffi_converter }}::lower)?; - Ok({% call ret(func) %}) -}) -{% else %} -uniffi::call_with_output(call_status, || { - {% match func.return_type() -%} - {% when Some with (return_type) -%} - {{ return_type|ffi_converter }}::lower({% call to_rs_call(func) %}) - {% else -%} - {% if func.full_arguments().is_empty() %}#[allow(clippy::redundant_closure)]{% endif %} - {% call to_rs_call(func) %} - {%- if func.return_type().is_none() %};{% endif %} - {% endmatch -%} -}){%- if func.return_type().is_none() -%};{% endif %} -{% endmatch %} -{% endmacro %} +{% macro return_signature(func) %} +{%- match func.return_type() %} +{%- when Some with (return_type) %} -> {{ return_type|ffi_converter }}::ReturnType +{%- else -%} +{%- endmatch -%} +{%- endmacro -%} diff --git a/uniffi_core/src/ffi/mod.rs b/uniffi_core/src/ffi/mod.rs index c41daacd03..72bd7b1822 100644 --- a/uniffi_core/src/ffi/mod.rs +++ b/uniffi_core/src/ffi/mod.rs @@ -11,7 +11,7 @@ pub mod rustbuffer; pub mod rustcalls; pub mod rustfuture; -use ffidefault::FfiDefault; +pub use ffidefault::FfiDefault; pub use foreignbytes::*; pub use foreigncallbacks::*; pub use rustbuffer::*; diff --git a/uniffi_core/src/ffi/rustbuffer.rs b/uniffi_core/src/ffi/rustbuffer.rs index 63af586fb6..ac09bfd988 100644 --- a/uniffi_core/src/ffi/rustbuffer.rs +++ b/uniffi_core/src/ffi/rustbuffer.rs @@ -2,7 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use crate::ffi::{call_with_output, ForeignBytes, RustCallStatus}; +use crate::ffi::{rust_call, ForeignBytes, RustCallStatus}; /// Support for passing an allocated-by-Rust buffer of bytes over the FFI. /// @@ -207,8 +207,8 @@ pub extern "C" fn uniffi_rustbuffer_alloc( size: i32, call_status: &mut RustCallStatus, ) -> RustBuffer { - call_with_output(call_status, || { - RustBuffer::new_with_size(size.max(0) as usize) + rust_call(call_status, || { + Ok(RustBuffer::new_with_size(size.max(0) as usize)) }) } @@ -225,9 +225,9 @@ pub unsafe extern "C" fn uniffi_rustbuffer_from_bytes( bytes: ForeignBytes, call_status: &mut RustCallStatus, ) -> RustBuffer { - call_with_output(call_status, || { + rust_call(call_status, || { let bytes = bytes.as_slice(); - RustBuffer::from_vec(bytes.to_vec()) + Ok(RustBuffer::from_vec(bytes.to_vec())) }) } @@ -239,7 +239,10 @@ pub unsafe extern "C" fn uniffi_rustbuffer_from_bytes( /// corrupting the allocator state. #[no_mangle] pub unsafe extern "C" fn uniffi_rustbuffer_free(buf: RustBuffer, call_status: &mut RustCallStatus) { - call_with_output(call_status, || RustBuffer::destroy(buf)) + rust_call(call_status, || { + RustBuffer::destroy(buf); + Ok(()) + }) } /// Reserve additional capacity in a byte buffer that had previously been passed to the @@ -263,13 +266,13 @@ pub unsafe extern "C" fn uniffi_rustbuffer_reserve( additional: i32, call_status: &mut RustCallStatus, ) -> RustBuffer { - call_with_output(call_status, || { + rust_call(call_status, || { let additional: usize = additional .try_into() .expect("additional buffer length negative or overflowed"); let mut v = buf.destroy_into_vec(); v.reserve(additional); - RustBuffer::from_vec(v) + Ok(RustBuffer::from_vec(v)) }) } diff --git a/uniffi_core/src/ffi/rustcalls.rs b/uniffi_core/src/ffi/rustcalls.rs index 908caa22b9..9be3dea51b 100644 --- a/uniffi_core/src/ffi/rustcalls.rs +++ b/uniffi_core/src/ffi/rustcalls.rs @@ -8,11 +8,10 @@ //! //! It handles: //! - Catching panics -//! - Adapting `Result<>` types into either a return value or an error +//! - Adapting the result of `FfiConverter::lower_return()` into either a return value or an +//! exception -use super::FfiDefault; -use crate::{FfiConverter, RustBuffer, UniFfiTag}; -use anyhow::Result; +use crate::{FfiConverter, FfiDefault, RustBuffer, UniFfiTag}; use std::mem::MaybeUninit; use std::panic; @@ -80,8 +79,27 @@ const CALL_SUCCESS: i8 = 0; // CALL_SUCCESS is set by the calling code const CALL_ERROR: i8 = 1; const CALL_PANIC: i8 = 2; -// Generalized rust call handling function -fn make_call(out_status: &mut RustCallStatus, callback: F) -> R +/// Handle a scaffolding calls +/// +/// `callback` is responsible for making the actual Rust call and returning a special result type: +/// - For successfull calls, return `Ok(value)` +/// - For errors that should be translated into thrown exceptions in the foreign code, serialize +/// the error into a `RustBuffer`, then return `Ok(buf)` +/// - The success type, must implement `FfiDefault`. +/// - `FfiConverter::lower_return` returns `Result<>` types that meet the above criteria> +/// - If the function returns a `Ok` value it will be unwrapped and returned +/// - If the function returns a `Err` value: +/// - `out_status.code` will be set to `CALL_ERROR` +/// - `out_status.error_buf` will be set to a newly allocated `RustBuffer` containing the error. The calling +/// code is responsible for freeing the `RustBuffer` +/// - `FfiDefault::ffi_default()` is returned, although foreign code should ignore this value +/// - If the function panics: +/// - `out_status.code` will be set to `CALL_PANIC` +/// - `out_status.error_buf` will be set to a newly allocated `RustBuffer` containing a +/// serialized error message. The calling code is responsible for freeing the `RustBuffer` +/// - `FfiDefault::ffi_default()` is returned, although foreign code should ignore this value + +pub fn rust_call(out_status: &mut RustCallStatus, callback: F) -> R where F: panic::UnwindSafe + FnOnce() -> Result, R: FfiDefault, @@ -136,61 +154,14 @@ where } } -/// Wrap a rust function call and return the result directly -/// -/// `callback` is responsible for making the call to the Rust function. It must convert any return -/// value into a type that implements `IntoFfi` (typically handled with `FfiConverter::lower()`). -/// -/// - If the function succeeds then the function's return value will be returned to the outer code -/// - If the function panics: -/// - `out_status.code` will be set to `CALL_PANIC` -/// - the return value is undefined -pub fn call_with_output(out_status: &mut RustCallStatus, callback: F) -> R -where - F: panic::UnwindSafe + FnOnce() -> R, - R: FfiDefault, -{ - make_call(out_status, || Ok(callback())) -} - -/// Wrap a rust function call that returns a `Result<_, RustBuffer>` -/// -/// `callback` is responsible for making the call to the Rust function. -/// - `callback` must convert any return value into a type that implements `IntoFfi` -/// - `callback` must convert any `Error` the into a `RustBuffer` to be returned over the FFI -/// - (Both of these are typically handled with `FfiConverter::lower()`) -/// -/// - If the function returns an `Ok` value it will be unwrapped and returned -/// - If the function returns an `Err`: -/// - `out_status.code` will be set to `CALL_ERROR` -/// - `out_status.error_buf` will be set to a newly allocated `RustBuffer` containing the error. The calling -/// code is responsible for freeing the `RustBuffer` -/// - the return value is undefined -/// - If the function panics: -/// - `out_status.code` will be set to `CALL_PANIC` -/// - the return value is undefined -pub fn call_with_result(out_status: &mut RustCallStatus, callback: F) -> R -where - F: panic::UnwindSafe + FnOnce() -> Result, - R: FfiDefault, -{ - make_call(out_status, callback) -} - #[cfg(test)] mod test { use super::*; use crate::{ - ffi_converter_rust_buffer_lift_and_lower, FfiConverter, MetadataBuffer, UniFfiTag, + ffi_converter_default_return, ffi_converter_rust_buffer_lift_and_lower, MetadataBuffer, + UniFfiTag, }; - fn function(a: u8) -> i8 { - match a { - 0 => 100, - x => panic!("Unexpected value: {x}"), - } - } - fn create_call_status() -> RustCallStatus { RustCallStatus { code: 0, @@ -198,36 +169,19 @@ mod test { } } - #[test] - fn test_call_with_output() { - let mut status = create_call_status(); - let return_value = call_with_output(&mut status, || function(0)); - assert_eq!(status.code, CALL_SUCCESS); - assert_eq!(return_value, 100); - - call_with_output(&mut status, || function(1)); - assert_eq!(status.code, CALL_PANIC); - unsafe { - assert_eq!( - >::try_lift(status.error_buf.assume_init()) - .unwrap(), - "Unexpected value: 1" - ); - } - } - #[derive(Debug, PartialEq)] struct TestError(String); // Use FfiConverter to simplify lifting TestError out of RustBuffer to check it unsafe impl FfiConverter for TestError { ffi_converter_rust_buffer_lift_and_lower!(UniFfiTag); + ffi_converter_default_return!(UniFfiTag); fn write(obj: TestError, buf: &mut Vec) { >::write(obj.0, buf); } - fn try_read(buf: &mut &[u8]) -> Result { + fn try_read(buf: &mut &[u8]) -> anyhow::Result { >::try_read(buf).map(TestError) } @@ -235,7 +189,7 @@ mod test { const TYPE_ID_META: MetadataBuffer = MetadataBuffer::new(); } - fn function_with_result(a: u8) -> Result { + fn test_callback(a: u8) -> Result { match a { 0 => Ok(100), 1 => Err(TestError("Error".to_owned())), @@ -244,16 +198,17 @@ mod test { } #[test] - fn test_call_with_result() { + fn test_rust_call() { let mut status = create_call_status(); - let return_value = call_with_result(&mut status, || { - function_with_result(0).map_err(TestError::lower) + let return_value = rust_call(&mut status, || { + as FfiConverter>::lower_return(test_callback(0)) }); + assert_eq!(status.code, CALL_SUCCESS); assert_eq!(return_value, 100); - call_with_result(&mut status, || { - function_with_result(1).map_err(TestError::lower) + rust_call(&mut status, || { + as FfiConverter>::lower_return(test_callback(1)) }); assert_eq!(status.code, CALL_ERROR); unsafe { @@ -265,8 +220,8 @@ mod test { } let mut status = create_call_status(); - call_with_result(&mut status, || { - function_with_result(2).map_err(TestError::lower) + rust_call(&mut status, || { + as FfiConverter>::lower_return(test_callback(2)) }); assert_eq!(status.code, CALL_PANIC); unsafe { diff --git a/uniffi_core/src/ffi/rustfuture.rs b/uniffi_core/src/ffi/rustfuture.rs index ab28c087c8..7e6730da60 100644 --- a/uniffi_core/src/ffi/rustfuture.rs +++ b/uniffi_core/src/ffi/rustfuture.rs @@ -290,11 +290,11 @@ //! [`RawWaker`]: https://doc.rust-lang.org/std/task/struct.RawWaker.html use super::FfiDefault; -use crate::{call_with_result, FfiConverter, RustBuffer, RustCallStatus}; +use crate::{rust_call, FfiConverter, RustCallStatus}; use std::{ ffi::c_void, future::Future, - mem::ManuallyDrop, + mem::{ManuallyDrop, MaybeUninit}, panic, pin::Pin, sync::Arc, @@ -305,13 +305,13 @@ use std::{ /// /// See the module documentation to learn more. #[repr(transparent)] -pub struct RustFuture(Pin> + 'static>>); +pub struct RustFuture(Pin + 'static>>); -impl RustFuture { +impl RustFuture { /// Create a new `RustFuture` from a regular `Future`. pub fn new(future: F) -> Self where - F: Future> + 'static, + F: Future + 'static, { Self(Box::pin(future)) } @@ -322,7 +322,7 @@ impl RustFuture { #[cfg(feature = "tokio")] pub fn new_tokio(future: F) -> Self where - F: Future> + 'static, + F: Future + 'static, { Self(Box::pin(async_compat::Compat::new(future))) } @@ -343,7 +343,7 @@ impl RustFuture { &mut self, foreign_waker: RustFutureForeignWakerFunction, foreign_waker_environment: *const c_void, - ) -> RustFuturePoll> { + ) -> Poll { let waker = unsafe { Waker::from_raw(RawWaker::new( RustFutureForeignWaker::new(foreign_waker, foreign_waker_environment) @@ -353,7 +353,7 @@ impl RustFuture { }; let mut context = Context::from_waker(&waker); - self.0.as_mut().poll(&mut context).into() + self.0.as_mut().poll(&mut context) } } @@ -378,32 +378,6 @@ pub enum RustFuturePoll { Throwing, } -impl From> for RustFuturePoll { - fn from(value: Poll) -> Self { - match value { - Poll::Ready(ready) => Self::Ready(ready), - Poll::Pending => Self::Pending, - } - } -} - -impl RustFuturePoll> { - /// Transpose a `RustFuturePoll>` to a - /// `Result, E>`. - fn transpose(self) -> Result, E> { - match self { - Self::Ready(ready) => match ready { - Ok(ok) => Ok(RustFuturePoll::Ready(ok)), - Err(error) => Err(error), - }, - - Self::Pending => Ok(RustFuturePoll::Pending), - - Self::Throwing => Ok(RustFuturePoll::Throwing), - } - } -} - impl FfiDefault for RustFuturePoll { fn ffi_default() -> Self { Self::Throwing @@ -420,8 +394,8 @@ mod tests_rust_future { let pointer_size = mem::size_of::<*const u8>(); let rust_future_size = pointer_size * 2; - assert_eq!(mem::size_of::>(), rust_future_size); - assert_eq!(mem::size_of::>(), rust_future_size); + assert_eq!(mem::size_of::>(), rust_future_size); + assert_eq!(mem::size_of::>(), rust_future_size); } } @@ -612,36 +586,38 @@ const PENDING: bool = false; /// # Panics /// /// The function panics if `future` or `waker` is a NULL pointer. -pub fn uniffi_rustfuture_poll( - future: Option<&mut RustFuture>, +pub fn uniffi_rustfuture_poll( + future: Option<&mut RustFuture>, waker: Option, waker_environment: *const c_void, - polled_result: &mut T::FfiType, + polled_result: &mut MaybeUninit, call_status: &mut RustCallStatus, ) -> bool where T: FfiConverter, - E: FfiConverter, { // If polling the future panics, an error will be recorded in call_status and the future will // be dropped, so there is no potential for observing any inconsistent state in it. let mut future = panic::AssertUnwindSafe(future.expect("`future` is a null pointer")); let waker = waker.expect("`waker` is a null pointer"); - match call_with_result(call_status, move || { - future - .poll(waker, waker_environment) - .transpose() - .map_err(E::lower) - }) { - RustFuturePoll::Ready(ready) => { - *polled_result = T::lower(ready); - + let rust_call_result = rust_call(call_status, move || { + // Poll the future and convert the value into a Result + match future.poll(waker, waker_environment) { + Poll::Ready(v) => T::lower_return(v).map(RustFuturePoll::Ready), + Poll::Pending => Ok(RustFuturePoll::Pending), + } + }); + match rust_call_result { + // Sucessfull return + RustFuturePoll::Ready(v) => { + polled_result.write(v); READY } - + // Future still pending RustFuturePoll::Pending => PENDING, - + // Error return or panic. `rust_call()` has update `call_status` with the error status and + // set `error_buf`. RustFuturePoll::Throwing => READY, } } @@ -653,8 +629,8 @@ where /// scope. /// /// Please see the module documentation to learn more. -pub fn uniffi_rustfuture_drop( - _future: Option>>, +pub fn uniffi_rustfuture_drop( + _future: Option>>, _call_status: &mut RustCallStatus, ) { } diff --git a/uniffi_core/src/ffi_converter_impls.rs b/uniffi_core/src/ffi_converter_impls.rs index 748ce0d1e9..45af247d26 100644 --- a/uniffi_core/src/ffi_converter_impls.rs +++ b/uniffi_core/src/ffi_converter_impls.rs @@ -3,8 +3,8 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use crate::{ - check_remaining, ffi_converter_rust_buffer_lift_and_lower, metadata, FfiConverter, Interface, - MetadataBuffer, Result, RustBuffer, + check_remaining, ffi_converter_default_return, ffi_converter_rust_buffer_lift_and_lower, + metadata, FfiConverter, Interface, MetadataBuffer, Result, RustBuffer, }; /// This module contains builtin `FFIConverter` implementations. These cover: /// - Simple privitive types: u8, i32, String, Arc, etc @@ -42,6 +42,8 @@ macro_rules! impl_ffi_converter_for_num_primitive { ($T:ty, $type_code:expr) => { paste! { unsafe impl FfiConverter for $T { + ffi_converter_default_return!(UT); + type FfiType = $T; fn lower(obj: $T) -> Self::FfiType { @@ -83,6 +85,8 @@ impl_ffi_converter_for_num_primitive!(f64, metadata::codes::TYPE_F64); /// Booleans are passed as an `i8` in order to avoid problems with handling /// C-compatible boolean values on JVM-based languages. unsafe impl FfiConverter for bool { + ffi_converter_default_return!(UT); + type FfiType = i8; fn lower(obj: bool) -> Self::FfiType { @@ -111,6 +115,8 @@ unsafe impl FfiConverter for bool { /// Support for passing the unit type via the FFI. This is currently only used for void returns unsafe impl FfiConverter for () { + ffi_converter_default_return!(UT); + type FfiType = (); fn try_lift(_: Self::FfiType) -> Result<()> { @@ -129,6 +135,8 @@ unsafe impl FfiConverter for () { } unsafe impl FfiConverter for Infallible { + ffi_converter_default_return!(UT); + type FfiType = RustBuffer; fn try_lift(_: Self::FfiType) -> Result { @@ -162,6 +170,8 @@ unsafe impl FfiConverter for Infallible { /// followed by utf8-encoded bytes. (It's a signed integer because unsigned types are /// currently experimental in Kotlin). unsafe impl FfiConverter for String { + ffi_converter_default_return!(UT); + type FfiType = RustBuffer; // This returns a struct with a raw pointer to the underlying bytes, so it's very @@ -223,6 +233,7 @@ unsafe impl FfiConverter for String { /// if the total offset should be added to or subtracted from the unix epoch. unsafe impl FfiConverter for SystemTime { ffi_converter_rust_buffer_lift_and_lower!(UT); + ffi_converter_default_return!(UT); fn write(obj: SystemTime, buf: &mut Vec) { let mut sign = 1; @@ -268,6 +279,7 @@ unsafe impl FfiConverter for SystemTime { /// and 999,999,999. unsafe impl FfiConverter for Duration { ffi_converter_rust_buffer_lift_and_lower!(UT); + ffi_converter_default_return!(UT); fn write(obj: Duration, buf: &mut Vec) { buf.put_u64(obj.as_secs()); @@ -293,6 +305,7 @@ unsafe impl FfiConverter for Duration { /// but that seems more fiddly and less safe in the short term, so it can wait. unsafe impl> FfiConverter for Option { ffi_converter_rust_buffer_lift_and_lower!(UT); + ffi_converter_default_return!(UT); fn write(obj: Option, buf: &mut Vec) { match obj { @@ -328,6 +341,7 @@ unsafe impl> FfiConverter for Option { /// similar struct. But that's for future work. unsafe impl> FfiConverter for Vec { ffi_converter_rust_buffer_lift_and_lower!(UT); + ffi_converter_default_return!(UT); fn write(obj: Vec, buf: &mut Vec) { // TODO: would be nice not to panic here :-/ @@ -366,6 +380,7 @@ where V: FfiConverter, { ffi_converter_rust_buffer_lift_and_lower!(UT); + ffi_converter_default_return!(UT); fn write(obj: HashMap, buf: &mut Vec) { // TODO: would be nice not to panic here :-/ @@ -453,6 +468,48 @@ unsafe impl> FfiConverter for std::sync::Arc { >::try_lift(buf.get_u64() as Self::FfiType) } + ffi_converter_default_return!(UT); + const TYPE_ID_META: MetadataBuffer = MetadataBuffer::from_code(metadata::codes::TYPE_INTERFACE).concat_str(T::NAME); } + +/// Support `Result<>` via the FFI. +/// +/// This is currently supported for function returns. Lifting/lowering Result<> arguments is not +/// implemented. +unsafe impl FfiConverter for Result +where + R: FfiConverter, + E: FfiConverter, +{ + type FfiType = (); // Placeholder while lower/lift/serializing is unimplemented + type ReturnType = R::ReturnType; + + fn try_lift(_: Self::FfiType) -> Result { + unimplemented!(); + } + + fn lower(_: Self) -> Self::FfiType { + unimplemented!(); + } + + fn write(_: Self, _: &mut Vec) { + unimplemented!(); + } + + fn try_read(_: &mut &[u8]) -> Result { + unimplemented!(); + } + + fn lower_return(v: Self) -> Result { + match v { + Ok(r) => R::lower_return(r), + Err(e) => Err(E::lower(e)), + } + } + + const TYPE_ID_META: MetadataBuffer = MetadataBuffer::from_code(metadata::codes::TYPE_RESULT) + .concat(R::TYPE_ID_META) + .concat(E::TYPE_ID_META); +} diff --git a/uniffi_core/src/lib.rs b/uniffi_core/src/lib.rs index 0bac06b368..7c69efe11c 100644 --- a/uniffi_core/src/lib.rs +++ b/uniffi_core/src/lib.rs @@ -21,6 +21,8 @@ //! where they are part of a compound data structure that is serialized for transfer. //! * How to [read](FfiConverter::try_read) values of the Rust type from buffer, for cases //! where they are received as part of a compound data structure that was serialized for transfer. +//! * How to [return](FfiConverter::lower_return) values of the Rust type from scaffolding +//! functions. //! //! This logic encapsulates the Rust-side handling of data transfer. Each foreign-language binding //! must also implement a matching set of data-handling rules for each data type. @@ -127,13 +129,34 @@ macro_rules! assert_compatible_version { /// /// The `FfiConverter` trait defines how to pass values of a particular type back-and-forth over /// the uniffi generated FFI layer, both as standalone argument or return values, and as -/// part of serialized compound data structures. +/// part of serialized compound data structures. `FfiConverter` is mainly used in generated code. +/// The goal is to make it easy for the code generator to name the correct FFI-related function for +/// a given type. /// /// FfiConverter has a generic parameter, that's filled in with a type local to the UniFFI consumer crate. /// This allows us to work around the Rust orphan rules for remote types. See /// `https://mozilla.github.io/uniffi-rs/internals/lifting_and_lowering.html#code-generation-and-the-fficonverter-trait` /// for details. /// +/// ## Scope +/// +/// It could be argued that FfiConverter handles too many concerns and should be split into +/// separate traits (like `FfiLiftAndLower`, `FfiSerialize`, `FfiReturn`). However, there are good +/// reasons to keep all the functionality in one trait. +/// +/// The main reason is that splitting the traits requires fairly complex blanket implementations, +/// for example `impl FfiReturn for T: FfiLiftAndLower`. Writing these impls often +/// means fighting with `rustc` over what counts as a conflicting implementation. In fact, as of +/// Rust 1.66, that previous example conflicts with `impl FfiReturn for ()`, since other +/// crates can implement `impl FfiReturn for ()`. In general, this path gets +/// complicated very quickly and that distracts from the logic that we want to define, which is +/// fairly simple. +/// +/// The main downside of having a single `FfiConverter` trait is that we need to implement it for +/// types that only support some of the functionality. For example, `Result<>` supports returning +/// values, but not lifting/lowering/serializing them. This is a bit weird, but works okay -- +/// especially since `FfiConverter` is rarely used outside of generated code. +/// /// ## Safety /// /// This is an unsafe trait (implementing it requires `unsafe impl`) because we can't guarantee @@ -155,6 +178,11 @@ pub unsafe trait FfiConverter: Sized { /// serialization is simpler and safer as a starting point. type FfiType; + /// The type that the should be returned by scaffolding functions for this type. + /// + /// This is usually the same as `FfiType`, but `Result<>` has specialized handling. + type ReturnType: FfiDefault; + /// Lower a rust value of the target type, into an FFI value of type Self::FfiType. /// /// This trait method is used for sending data from rust to the foreign language code, @@ -165,6 +193,14 @@ pub unsafe trait FfiConverter: Sized { /// the foreign language code, e.g. by boxing the value and passing a pointer. fn lower(obj: Self) -> Self::FfiType; + /// Lower this value for scaffolding function return + /// + /// This method converts values into the `Result<>` type that [rust_call] expects. For + /// successful calls, return `Ok(lower_return)`. For errors that should be translated into + /// thrown exceptions on the foreign code, serialize the error into a RustBuffer and return + /// `Err(buf)` + fn lower_return(obj: Self) -> Result; + /// Lift a rust value of the target type, from an FFI value of type Self::FfiType. /// /// This trait method is used for receiving data from the foreign language code in rust, @@ -243,6 +279,21 @@ where } } +/// Macro to implement returning values by simply lowering them and returning them +/// +/// This is what we use for all FfiConverters except for `Result`. This would be nicer as a +/// trait default, but Rust doesn't support defaults on associated types. +#[macro_export] +macro_rules! ffi_converter_default_return { + ($uniffi_tag:ty) => { + type ReturnType = >::FfiType; + + fn lower_return(v: Self) -> Result { + Ok(>::lower(v)) + } + }; +} + /// Macro to implement lowering/lifting using a `RustBuffer` /// /// For complex types where it's too fiddly or too unsafe to convert them into a special-purpose @@ -281,11 +332,16 @@ macro_rules! ffi_converter_forward { ($T:ty, $existing_impl_tag:ty, $new_impl_tag:ty) => { unsafe impl $crate::FfiConverter<$new_impl_tag> for $T { type FfiType = <$T as $crate::FfiConverter<$existing_impl_tag>>::FfiType; + type ReturnType = <$T as $crate::FfiConverter<$existing_impl_tag>>::FfiType; fn lower(obj: $T) -> Self::FfiType { <$T as $crate::FfiConverter<$existing_impl_tag>>::lower(obj) } + fn lower_return(v: Self) -> Result { + <$T as $crate::FfiConverter<$existing_impl_tag>>::lower_return(v) + } + fn try_lift(v: Self::FfiType) -> $crate::Result<$T> { <$T as $crate::FfiConverter<$existing_impl_tag>>::try_lift(v) } diff --git a/uniffi_core/src/metadata.rs b/uniffi_core/src/metadata.rs index 1942368338..1121c1deb0 100644 --- a/uniffi_core/src/metadata.rs +++ b/uniffi_core/src/metadata.rs @@ -60,6 +60,8 @@ pub mod codes { pub const TYPE_DURATION: u8 = 20; 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_UNIT: u8 = 255; } diff --git a/uniffi_macros/src/enum_.rs b/uniffi_macros/src/enum_.rs index 3df669c449..0617317c0d 100644 --- a/uniffi_macros/src/enum_.rs +++ b/uniffi_macros/src/enum_.rs @@ -116,6 +116,7 @@ fn enum_or_error_ffi_converter_impl( #[automatically_derived] unsafe #impl_spec { ::uniffi::ffi_converter_rust_buffer_lift_and_lower!(crate::UniFfiTag); + ::uniffi::ffi_converter_default_return!(crate::UniFfiTag); fn write(obj: Self, buf: &mut ::std::vec::Vec) { #write_impl diff --git a/uniffi_macros/src/error.rs b/uniffi_macros/src/error.rs index d14f4bf740..293a8aecc6 100644 --- a/uniffi_macros/src/error.rs +++ b/uniffi_macros/src/error.rs @@ -55,13 +55,11 @@ pub fn expand_error(input: DeriveInput) -> syn::Result { pub(crate) fn expand_ffi_converter_error(attr: ErrorAttr, input: DeriveInput) -> TokenStream { match input.data { Data::Enum(e) => error_ffi_converter_impl(&input.ident, &e, &attr), - _ => { - syn::Error::new( - proc_macro2::Span::call_site(), - "This attribute must only be used on enums", - ) - .into_compile_error() - } + _ => syn::Error::new( + proc_macro2::Span::call_site(), + "This attribute must only be used on enums", + ) + .into_compile_error(), } } @@ -132,6 +130,7 @@ fn flat_error_ffi_converter_impl( #[automatically_derived] unsafe #impl_spec { ::uniffi::ffi_converter_rust_buffer_lift_and_lower!(crate::UniFfiTag); + ::uniffi::ffi_converter_default_return!(crate::UniFfiTag); fn write(obj: Self, buf: &mut ::std::vec::Vec) { #write_impl @@ -153,7 +152,7 @@ pub(crate) fn error_meta_static_var( flat: bool, ) -> syn::Result { let name = ident.to_string(); - let flat_code = if flat { 1u8 } else { 0u8 }; + let flat_code = u8::from(flat); let mut metadata_expr = quote! { ::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::ERROR) .concat_str(module_path!()) diff --git a/uniffi_macros/src/export.rs b/uniffi_macros/src/export.rs index 3a3844ca17..744dd353af 100644 --- a/uniffi_macros/src/export.rs +++ b/uniffi_macros/src/export.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use proc_macro2::{Ident, Span, TokenStream}; -use quote::quote_spanned; +use quote::{quote, quote_spanned}; use syn::{ parse::{Parse, ParseStream}, spanned::Spanned, @@ -14,10 +14,7 @@ pub(crate) mod metadata; mod scaffolding; pub use self::metadata::gen_metadata; -use self::{ - metadata::convert::try_split_result, - scaffolding::{gen_fn_scaffolding, gen_method_scaffolding}, -}; +use self::scaffolding::{gen_fn_scaffolding, gen_method_scaffolding}; use crate::util::{either_attribute_arg, parse_comma_separated, UniffiAttribute}; // TODO(jplatte): Ensure no generics, … @@ -41,14 +38,14 @@ pub struct Signature { ident: Ident, is_async: bool, inputs: Vec, - output: Option, + output: TokenStream, } impl Signature { fn new(item: syn::Signature) -> syn::Result { let output = match item.output { - syn::ReturnType::Default => None, - syn::ReturnType::Type(_, ty) => Some(FunctionReturn::new(ty)?), + syn::ReturnType::Default => quote! { () }, + syn::ReturnType::Type(_, ty) => quote! { #ty }, }; Ok(Self { @@ -60,23 +57,6 @@ impl Signature { } } -pub struct FunctionReturn { - ty: Box, - throws: Option, -} - -impl FunctionReturn { - fn new(ty: Box) -> syn::Result { - Ok(match try_split_result(&ty)? { - Some((ok_type, throws)) => FunctionReturn { - ty: Box::new(ok_type.to_owned()), - throws: Some(throws), - }, - None => FunctionReturn { ty, throws: None }, - }) - } -} - pub fn expand_export( metadata: ExportItem, arguments: ExportAttributeArguments, diff --git a/uniffi_macros/src/export/metadata/convert.rs b/uniffi_macros/src/export/metadata/convert.rs index d53c7404ef..c393d0cf85 100644 --- a/uniffi_macros/src/export/metadata/convert.rs +++ b/uniffi_macros/src/export/metadata/convert.rs @@ -2,16 +2,8 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use proc_macro2::Ident; use quote::ToTokens; -fn type_as_type_name(arg: &syn::Type) -> syn::Result<&Ident> { - type_as_type_path(arg)? - .path - .get_ident() - .ok_or_else(|| type_not_supported(arg)) -} - pub(super) fn type_as_type_path(ty: &syn::Type) -> syn::Result<&syn::TypePath> { match ty { syn::Type::Group(g) => type_as_type_path(&g.elem), @@ -21,62 +13,9 @@ pub(super) fn type_as_type_path(ty: &syn::Type) -> syn::Result<&syn::TypePath> { } } -fn arg_as_type(arg: &syn::GenericArgument) -> syn::Result<&syn::Type> { - match arg { - syn::GenericArgument::Type(t) => Ok(t), - _ => Err(syn::Error::new_spanned( - arg, - "non-type generic parameters are not currently supported by uniffi::export", - )), - } -} - fn type_not_supported(ty: &impl ToTokens) -> syn::Error { syn::Error::new_spanned( ty, "this type is not currently supported by uniffi::export in this position", ) } - -pub(crate) fn try_split_result(ty: &syn::Type) -> syn::Result> { - let type_path = type_as_type_path(ty)?; - - if type_path.qself.is_some() { - return Err(syn::Error::new_spanned( - type_path, - "qualified self types are not currently supported by uniffi::export", - )); - } - - if type_path.path.segments.len() > 1 { - return Err(syn::Error::new_spanned( - type_path, - "qualified paths in types are not currently supported by uniffi::export", - )); - } - - let (ident, a) = match &type_path.path.segments.first() { - Some(seg) => match &seg.arguments { - syn::PathArguments::AngleBracketed(a) => (&seg.ident, a), - syn::PathArguments::None | syn::PathArguments::Parenthesized(_) => return Ok(None), - }, - None => return Ok(None), - }; - - let mut it = a.args.iter(); - if let Some(arg1) = it.next() { - if let Some(arg2) = it.next() { - if it.next().is_none() { - let arg1 = arg_as_type(arg1)?; - let arg2 = arg_as_type(arg2)?; - - if let "Result" = ident.to_string().as_str() { - let throws = type_as_type_name(arg2)?.to_owned(); - return Ok(Some((arg1, throws))); - } - } - } - } - - Ok(None) -} diff --git a/uniffi_macros/src/export/scaffolding.rs b/uniffi_macros/src/export/scaffolding.rs index f95c577bb5..d88e2f3490 100644 --- a/uniffi_macros/src/export/scaffolding.rs +++ b/uniffi_macros/src/export/scaffolding.rs @@ -6,7 +6,7 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::{format_ident, quote, ToTokens}; use syn::{ext::IdentExt, spanned::Spanned, FnArg, Pat}; -use super::{AsyncRuntime, ExportAttributeArguments, FunctionReturn, Signature}; +use super::{AsyncRuntime, ExportAttributeArguments, Signature}; use crate::util::{create_metadata_static_var, try_metadata_value_from_usize, type_name}; pub(super) fn gen_fn_scaffolding( @@ -213,6 +213,7 @@ impl ScaffoldingBits { fn gen_function_meta_static_var(&self, sig: &Signature) -> syn::Result { let name = type_name(&sig.ident); + let return_ty = &sig.output; let is_async = sig.is_async; let args_len = try_metadata_value_from_usize( // Use param_lifts to calculate this instead of sig.inputs to avoid counting any self @@ -221,7 +222,6 @@ impl ScaffoldingBits { "UniFFI limits functions to 256 arguments", )?; let arg_metadata_calls = &self.arg_metadata_calls; - let result_metadata_calls = self.result_metadata_calls(sig); Ok(create_metadata_static_var( "FUNC", &name, @@ -232,7 +232,7 @@ impl ScaffoldingBits { .concat_bool(#is_async) .concat_value(#args_len) #(#arg_metadata_calls)* - #result_metadata_calls + .concat(<#return_ty as ::uniffi::FfiConverter>::TYPE_ID_META) }, )) } @@ -244,6 +244,7 @@ impl ScaffoldingBits { ) -> syn::Result { let object_name = type_name(self_ident); let name = type_name(&sig.ident); + let return_ty = &sig.output; let is_async = sig.is_async; let args_len = try_metadata_value_from_usize( // Use param_lifts to calculate this instead of sig.inputs to avoid counting any self @@ -252,7 +253,6 @@ impl ScaffoldingBits { "UniFFI limits functions to 256 arguments", )?; let arg_metadata_calls = &self.arg_metadata_calls; - let result_metadata_calls = self.result_metadata_calls(sig); Ok(create_metadata_static_var( "METHOD", &format!("{}_{}", object_name, name), @@ -264,30 +264,10 @@ impl ScaffoldingBits { .concat_bool(#is_async) .concat_value(#args_len) #(#arg_metadata_calls)* - #result_metadata_calls + .concat(<#return_ty as ::uniffi::FfiConverter>::TYPE_ID_META) }, )) } - - fn result_metadata_calls(&self, sig: &Signature) -> TokenStream { - match &sig.output { - Some(FunctionReturn { - ty, - throws: Some(throws), - }) => quote! { - .concat(<#ty as ::uniffi::FfiConverter>::TYPE_ID_META) - .concat(<#throws as ::uniffi::FfiConverter>::TYPE_ID_META) - }, - Some(FunctionReturn { ty, throws: None }) => quote! { - .concat(<#ty as ::uniffi::FfiConverter>::TYPE_ID_META) - .concat_value(::uniffi::metadata::codes::TYPE_UNIT) - }, - None => quote! { - .concat_value(::uniffi::metadata::codes::TYPE_UNIT) - .concat_value(::uniffi::metadata::codes::TYPE_UNIT) - }, - } - } } fn gen_ffi_function( @@ -297,165 +277,75 @@ fn gen_ffi_function( arguments: &ExportAttributeArguments, ) -> TokenStream { let name = sig.ident.to_string(); - let mut extra_functions = Vec::new(); - let is_async = sig.is_async; let rust_fn_call = bits.rust_fn_call(); let fn_params = &bits.params; + let return_ty = &sig.output; - let (return_ty, throw_ty, return_expr, throws) = match &sig.output { - Some(FunctionReturn { ty, throws: None }) if is_async => { - let return_ty = quote! { #ty }; - let throw_ty = Some(quote! { ::std::convert::Infallible }); - - ( - return_ty.clone(), - throw_ty.clone(), - quote! { Option>> }, - &None, - ) - } - - Some(FunctionReturn { ty, throws }) if is_async => { - let return_ty = quote! { #ty }; - let throw_ty = Some(quote! { #throws }); - - ( - return_ty.clone(), - throw_ty.clone(), - quote! { Option>> }, - throws, + if !sig.is_async { + if let Some(async_runtime) = &arguments.async_runtime { + return syn::Error::new( + async_runtime.span(), + "this attribute is only allowed on async functions", ) + .into_compile_error(); } - None if is_async => { - let return_ty = quote! { () }; - let throw_ty = Some(quote! { ::std::convert::Infallible }); - - ( - return_ty.clone(), - throw_ty.clone(), - quote! { Option>> }, - &None, - ) + quote! { + #[doc(hidden)] + #[no_mangle] + pub extern "C" fn #ffi_ident( + #(#fn_params,)* + call_status: &mut ::uniffi::RustCallStatus, + ) -> <#return_ty as ::uniffi::FfiConverter>::ReturnType { + ::uniffi::deps::log::debug!(#name); + ::uniffi::rust_call(call_status, || { + <#return_ty as ::uniffi::FfiConverter>::lower_return(#rust_fn_call) + }) + } } - - Some(FunctionReturn { ty, throws }) => ( - quote! { #ty }, - None, - quote! { <#ty as ::uniffi::FfiConverter>::FfiType }, - throws, - ), - - None => ( - quote! { () }, - None, - quote! { <() as ::uniffi::FfiConverter>::FfiType }, - &None, - ), - }; - - let body_expr = if is_async { + } else { let rust_future_ctor = match &arguments.async_runtime { Some(AsyncRuntime::Tokio(_)) => quote! { new_tokio }, None => quote! { new }, }; - - let body = match throws { - Some(_) => quote! { #rust_fn_call.await }, - None => quote! { Ok(#rust_fn_call.await) }, - }; + let ffi_poll_ident = format_ident!("{}_poll", ffi_ident); + let ffi_drop_ident = format_ident!("{}_drop", ffi_ident); quote! { - ::uniffi::call_with_output(call_status, || { - Some(Box::new(::uniffi::RustFuture::#rust_future_ctor( - async move { - #body - } - ))) - }) - } - } else { - match throws { - Some(error_ident) => { - quote! { - ::uniffi::call_with_result(call_status, || { - let val = #rust_fn_call.map_err(|e| { - <#error_ident as ::uniffi::FfiConverter>::lower( - ::std::convert::Into::into(e), - ) - })?; - - Ok(<#return_ty as ::uniffi::FfiConverter>::lower(val)) - }) - } - } - - None => { - quote! { - ::uniffi::call_with_output(call_status, || { - <#return_ty as ::uniffi::FfiConverter>::lower(#rust_fn_call) - }) - } + #[doc(hidden)] + #[no_mangle] + pub extern "C" fn #ffi_ident( + #(#fn_params,)* + call_status: &mut ::uniffi::RustCallStatus, + ) -> Box<::uniffi::RustFuture<#return_ty>> { + ::uniffi::deps::log::debug!(#name); + Box::new(::uniffi::RustFuture::#rust_future_ctor( + async move { #rust_fn_call.await } + )) } - } - }; - if is_async { - let ffi_poll_ident = format_ident!("{}_poll", ffi_ident); - let ffi_drop_ident = format_ident!("{}_drop", ffi_ident); - - // Monomorphised poll function. - extra_functions.push(quote! { + // Monomorphised poll function. #[doc(hidden)] #[no_mangle] pub extern "C" fn #ffi_poll_ident( - future: ::std::option::Option<&mut ::uniffi::RustFuture<#return_ty, #throw_ty>>, + future: ::std::option::Option<&mut ::uniffi::RustFuture<#return_ty>>, waker: ::std::option::Option<::uniffi::RustFutureForeignWakerFunction>, waker_environment: *const ::std::ffi::c_void, - polled_result: &mut <#return_ty as ::uniffi::FfiConverter>::FfiType, + polled_result: &mut ::std::mem::MaybeUninit<<#return_ty as ::uniffi::FfiConverter>::ReturnType>, call_status: &mut ::uniffi::RustCallStatus, ) -> bool { - ::uniffi::ffi::uniffi_rustfuture_poll::<_, _, crate::UniFfiTag>(future, waker, waker_environment, polled_result, call_status) + ::uniffi::ffi::uniffi_rustfuture_poll::<_, crate::UniFfiTag>(future, waker, waker_environment, polled_result, call_status) } - }); - // Monomorphised drop function. - extra_functions.push(quote! { + // Monomorphised drop function. #[doc(hidden)] #[no_mangle] pub extern "C" fn #ffi_drop_ident( - future: ::std::option::Option<::std::boxed::Box<::uniffi::RustFuture<#return_ty, #throw_ty>>>, + future: ::std::option::Option<::std::boxed::Box<::uniffi::RustFuture<#return_ty>>>, call_status: &mut ::uniffi::RustCallStatus, ) { ::uniffi::ffi::uniffi_rustfuture_drop(future, call_status) } - }); - } - - let argument_error = match &arguments.async_runtime { - Some(async_runtime) if !is_async => Some( - syn::Error::new( - async_runtime.span(), - "this attribute is only allowed on async functions", - ) - .into_compile_error(), - ), - _ => None, - }; - - quote! { - #[doc(hidden)] - #[no_mangle] - pub extern "C" fn #ffi_ident( - #(#fn_params,)* - call_status: &mut ::uniffi::RustCallStatus, - ) -> #return_expr { - ::uniffi::deps::log::debug!(#name); - #body_expr } - - #( #extra_functions )* - - #argument_error } } diff --git a/uniffi_macros/src/object.rs b/uniffi_macros/src/object.rs index 72bf86fdab..ae1580de41 100644 --- a/uniffi_macros/src/object.rs +++ b/uniffi_macros/src/object.rs @@ -23,12 +23,13 @@ pub fn expand_object(input: DeriveInput, module_path: String) -> syn::Result(); unsafe { ::std::sync::Arc::decrement_strong_count(ptr); } + Ok(()) }); } diff --git a/uniffi_macros/src/record.rs b/uniffi_macros/src/record.rs index 5b30bdc742..432a7d645e 100644 --- a/uniffi_macros/src/record.rs +++ b/uniffi_macros/src/record.rs @@ -54,6 +54,7 @@ pub(crate) fn record_ffi_converter_impl( #[automatically_derived] unsafe #impl_spec { ::uniffi::ffi_converter_rust_buffer_lift_and_lower!(crate::UniFfiTag); + ::uniffi::ffi_converter_default_return!(crate::UniFfiTag); fn write(obj: Self, buf: &mut ::std::vec::Vec) { #write_impl diff --git a/uniffi_meta/src/reader.rs b/uniffi_meta/src/reader.rs index ccf3eac611..056e87ee96 100644 --- a/uniffi_meta/src/reader.rs +++ b/uniffi_meta/src/reader.rs @@ -42,6 +42,7 @@ pub mod codes { pub const TYPE_DURATION: u8 = 20; pub const TYPE_CALLBACK_INTERFACE: u8 = 21; pub const TYPE_CUSTOM: u8 = 22; + pub const TYPE_RESULT: u8 = 23; pub const TYPE_UNIT: u8 = 255; } @@ -50,10 +51,12 @@ pub mod codes { /// We implement this on &[u8] byte buffers pub trait MetadataReader { fn read_u8(&mut self) -> Result; + fn peak_u8(&mut self) -> Result; fn read_bool(&mut self) -> Result; fn read_string(&mut self) -> Result; fn read_type(&mut self) -> Result; fn read_optional_type(&mut self) -> Result>; + fn read_return_type(&mut self) -> Result<(Option, Option)>; fn read_metadata(&mut self) -> Result; fn read_func(&mut self) -> Result; fn read_method(&mut self) -> Result; @@ -78,6 +81,14 @@ impl MetadataReader for &[u8] { } } + fn peak_u8(&mut self) -> Result { + if !self.is_empty() { + Ok(self[0]) + } else { + bail!("Buffer is empty") + } + } + fn read_bool(&mut self) -> Result { Ok(self.read_u8()? == 1) } @@ -90,60 +101,78 @@ impl MetadataReader for &[u8] { } fn read_type(&mut self) -> Result { - self.read_optional_type()? - .ok_or_else(|| anyhow::format_err!("saw TYPE_UNIT when a type was required")) + let value = self.read_u8()?; + Ok(match value { + codes::TYPE_U8 => Type::U8, + codes::TYPE_I8 => Type::I8, + codes::TYPE_U16 => Type::U16, + codes::TYPE_I16 => Type::I16, + codes::TYPE_U32 => Type::U32, + codes::TYPE_I32 => Type::I32, + codes::TYPE_U64 => Type::U64, + codes::TYPE_I64 => Type::I64, + codes::TYPE_F32 => Type::F32, + codes::TYPE_F64 => Type::F64, + codes::TYPE_BOOL => Type::Bool, + codes::TYPE_STRING => Type::String, + codes::TYPE_DURATION => Type::Duration, + codes::TYPE_SYSTEM_TIME => Type::SystemTime, + codes::TYPE_RECORD => Type::Record { + name: self.read_string()?, + }, + codes::TYPE_ENUM => Type::Enum { + name: self.read_string()?, + }, + codes::TYPE_ERROR => Type::Error { + name: self.read_string()?, + }, + codes::TYPE_INTERFACE => Type::ArcObject { + object_name: self.read_string()?, + }, + codes::TYPE_CALLBACK_INTERFACE => Type::CallbackInterface { + name: self.read_string()?, + }, + codes::TYPE_CUSTOM => Type::Custom { + name: self.read_string()?, + builtin: Box::new(self.read_type()?), + }, + codes::TYPE_OPTION => Type::Option { + inner_type: Box::new(self.read_type()?), + }, + codes::TYPE_VEC => Type::Vec { + inner_type: Box::new(self.read_type()?), + }, + codes::TYPE_HASH_MAP => Type::HashMap { + key_type: Box::new(self.read_type()?), + value_type: Box::new(self.read_type()?), + }, + codes::TYPE_UNIT => bail!("Unexpected TYPE_UNIT"), + codes::TYPE_RESULT => bail!("Unexpected TYPE_RESULT"), + _ => bail!("Unexpected metadata type code: {value:?}"), + }) } fn read_optional_type(&mut self) -> Result> { - let value = self.read_u8()?; - Ok(match value { - codes::TYPE_UNIT => None, - v => Some(match v { - codes::TYPE_U8 => Type::U8, - codes::TYPE_I8 => Type::I8, - codes::TYPE_U16 => Type::U16, - codes::TYPE_I16 => Type::I16, - codes::TYPE_U32 => Type::U32, - codes::TYPE_I32 => Type::I32, - codes::TYPE_U64 => Type::U64, - codes::TYPE_I64 => Type::I64, - codes::TYPE_F32 => Type::F32, - codes::TYPE_F64 => Type::F64, - codes::TYPE_BOOL => Type::Bool, - codes::TYPE_STRING => Type::String, - codes::TYPE_DURATION => Type::Duration, - codes::TYPE_SYSTEM_TIME => Type::SystemTime, - codes::TYPE_RECORD => Type::Record { - name: self.read_string()?, - }, - codes::TYPE_ENUM => Type::Enum { - name: self.read_string()?, - }, - codes::TYPE_ERROR => Type::Error { - name: self.read_string()?, - }, - codes::TYPE_INTERFACE => Type::ArcObject { - object_name: self.read_string()?, - }, - codes::TYPE_CALLBACK_INTERFACE => Type::CallbackInterface { - name: self.read_string()?, - }, - codes::TYPE_CUSTOM => Type::Custom { - name: self.read_string()?, - builtin: Box::new(self.read_type()?), - }, - codes::TYPE_OPTION => Type::Option { - inner_type: Box::new(self.read_type()?), - }, - codes::TYPE_VEC => Type::Vec { - inner_type: Box::new(self.read_type()?), - }, - codes::TYPE_HASH_MAP => Type::HashMap { - key_type: Box::new(self.read_type()?), - value_type: Box::new(self.read_type()?), - }, - _ => bail!("Unexpected metadata type code: {value:?}"), - }), + Ok(match self.peak_u8()? { + codes::TYPE_UNIT => { + _ = self.read_u8(); + None + } + _ => Some(self.read_type()?), + }) + } + + fn read_return_type(&mut self) -> Result<(Option, Option)> { + Ok(match self.peak_u8()? { + codes::TYPE_UNIT => { + _ = self.read_u8(); + (None, None) + } + codes::TYPE_RESULT => { + _ = self.read_u8(); + (self.read_optional_type()?, self.read_optional_type()?) + } + _ => (Some(self.read_type()?), None), }) } @@ -166,25 +195,36 @@ impl MetadataReader for &[u8] { } fn read_func(&mut self) -> Result { + let module_path = self.read_string()?; + let name = self.read_string()?; + let is_async = self.read_bool()?; + let inputs = self.read_inputs()?; + let (return_type, throws) = self.read_return_type()?; Ok(FnMetadata { - module_path: self.read_string()?, - name: self.read_string()?, - is_async: self.read_bool()?, - inputs: self.read_inputs()?, - return_type: self.read_optional_type()?, - throws: self.read_optional_type()?, + module_path, + name, + is_async, + inputs, + return_type, + throws, }) } fn read_method(&mut self) -> Result { + let module_path = self.read_string()?; + let self_name = self.read_string()?; + let name = self.read_string()?; + let is_async = self.read_bool()?; + let inputs = self.read_inputs()?; + let (return_type, throws) = self.read_return_type()?; Ok(MethodMetadata { - module_path: self.read_string()?, - self_name: self.read_string()?, - name: self.read_string()?, - is_async: self.read_bool()?, - inputs: self.read_inputs()?, - return_type: self.read_optional_type()?, - throws: self.read_optional_type()?, + module_path, + self_name, + name, + is_async, + inputs, + return_type, + throws, }) }