From 3d58ae3a6dcb470479a097f5b04443a7d1e536f7 Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Wed, 11 Oct 2023 11:36:58 -0400 Subject: [PATCH] Make From optional This is in preparation of #1578 -- allowing foreign languages to implement traits. One issue there is that if we want to allow foreign languages to implement traits, then any Result return types need to implement `LiftReturn`, and that impl currently has a bound on `E: From` impl, but use a panicking version if not. The trait bound is no longer required to implement `LiftReturn`. --- CHANGELOG.md | 6 ++ docs/manual/src/udl/callback_interfaces.md | 17 ++--- uniffi_core/src/ffi/callbackinterface.rs | 74 ++++++++++++++++++++++ uniffi_core/src/ffi_converter_impls.rs | 8 +-- uniffi_core/src/ffi_converter_traits.rs | 15 +++++ uniffi_core/src/lib.rs | 2 +- uniffi_macros/src/custom.rs | 4 +- uniffi_macros/src/enum_.rs | 4 +- uniffi_macros/src/error.rs | 6 +- uniffi_macros/src/record.rs | 6 +- uniffi_macros/src/util.rs | 21 +++++- 11 files changed, 140 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 244953b29c..a56d07276f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,12 @@ - Updated the async functionality to correctly handle cancellation (#1669) - Kotlin: Fixed low-level issue with exported async APIs +### What's changed? + +- Implementing `From`. However, this is means that any exception from the foreign bindings will lead to a panic. -### Extra requirements for errors used in callback interfaces +### Errors used in callback interfaces In order to support errors in callback interfaces, UniFFI must be able to properly [lift the error](../internals/lifting_and_lowering.md). This means that the if the error is described by an `enum` rather than an `interface` in the UDL (see [Errors](./errors.md)) then all variants of the Rust enum must be unit variants. -In addition to expected errors, a callback interface call can result in all kinds of -unexpected errors. Some examples are the foreign code throws an exception that's not part -of the exception type or there was a problem marshalling the data for the call. UniFFI -uses `uniffi::UnexpectedUniFFICallbackError` for these cases. Your code must include a -`From` impl for your error type to handle those or -the UniFFI scaffolding code will fail to compile. See `example/callbacks` for an -example of how to do this. +### Unexpected errors + +When Rust code invokes a callback interface method, that call may result in all kinds of unexpected errors. +Some examples are the foreign code throws an exception that's not part of the exception type or there was a problem marshalling the data for the call. +UniFFI creates an `uniffi::UnexpectedUniFFICallbackError` for these cases. +If your code defines a `From` impl for your error type, then those errors will be converted into your error type which will be returned to the Rust caller. +If not, then any unexpected errors will result in a panic. +See `example/callbacks` for an example of this. ## 3. Define a callback interface in the UDL diff --git a/uniffi_core/src/ffi/callbackinterface.rs b/uniffi_core/src/ffi/callbackinterface.rs index d4c8b42e3e..7be66880bb 100644 --- a/uniffi_core/src/ffi/callbackinterface.rs +++ b/uniffi_core/src/ffi/callbackinterface.rs @@ -232,3 +232,77 @@ impl fmt::Display for UnexpectedUniFFICallbackError { } impl std::error::Error for UnexpectedUniFFICallbackError {} + +// Autoref-based specialization for converting UnexpectedUniFFICallbackError into error types. +// +// For more details, see: +// https://github.com/dtolnay/case-studies/blob/master/autoref-specialization/README.md + +// Define two ZST types: +// - One implements `try_convert_unexpected_callback_error` by always returning an error value. +// - The specialized version implements it using `From` + +#[doc(hidden)] +#[derive(Debug)] +pub struct UnexpectedUniFFICallbackErrorConverterGeneric; + +impl UnexpectedUniFFICallbackErrorConverterGeneric { + pub fn try_convert_unexpected_callback_error( + &self, + e: UnexpectedUniFFICallbackError, + ) -> anyhow::Result { + Err(e.into()) + } +} + +#[doc(hidden)] +#[derive(Debug)] +pub struct UnexpectedUniFFICallbackErrorConverterSpecialized; + +impl UnexpectedUniFFICallbackErrorConverterSpecialized { + pub fn try_convert_unexpected_callback_error( + &self, + e: UnexpectedUniFFICallbackError, + ) -> anyhow::Result + where + E: From, + { + Ok(E::from(e)) + } +} + +// Macro to convert an UnexpectedUniFFICallbackError value for a particular type. This is used in +// the `ConvertError` implementation. +#[doc(hidden)] +#[macro_export] +macro_rules! convert_unexpected_error { + ($error:ident, $ty:ty) => {{ + // Trait for generic conversion, implemented for all &T. + pub trait GetConverterGeneric { + fn get_converter(&self) -> $crate::UnexpectedUniFFICallbackErrorConverterGeneric; + } + + impl GetConverterGeneric for &T { + fn get_converter(&self) -> $crate::UnexpectedUniFFICallbackErrorConverterGeneric { + $crate::UnexpectedUniFFICallbackErrorConverterGeneric + } + } + // Trait for specialized conversion, implemented for all T that implements + // `Into`. I.e. it's implemented for UnexpectedUniFFICallbackError when + // ErrorType implements From. + pub trait GetConverterSpecialized { + fn get_converter(&self) -> $crate::UnexpectedUniFFICallbackErrorConverterSpecialized; + } + + impl> GetConverterSpecialized for T { + fn get_converter(&self) -> $crate::UnexpectedUniFFICallbackErrorConverterSpecialized { + $crate::UnexpectedUniFFICallbackErrorConverterSpecialized + } + } + // Here's the hack. Because of the auto-ref rules, this will use `GetConverterSpecialized` + // if it's implemented and `GetConverterGeneric` if not. + (&$error) + .get_converter() + .try_convert_unexpected_callback_error($error) + }}; +} diff --git a/uniffi_core/src/ffi_converter_impls.rs b/uniffi_core/src/ffi_converter_impls.rs index fc7824fafe..af18f3873b 100644 --- a/uniffi_core/src/ffi_converter_impls.rs +++ b/uniffi_core/src/ffi_converter_impls.rs @@ -23,8 +23,8 @@ /// "UT" means an abitrary `UniFfiTag` type. use crate::{ check_remaining, derive_ffi_traits, ffi_converter_rust_buffer_lift_and_lower, metadata, - FfiConverter, ForeignExecutor, Lift, LiftReturn, Lower, LowerReturn, MetadataBuffer, Result, - RustBuffer, UnexpectedUniFFICallbackError, + ConvertError, FfiConverter, ForeignExecutor, Lift, LiftReturn, Lower, LowerReturn, + MetadataBuffer, Result, RustBuffer, UnexpectedUniFFICallbackError, }; use anyhow::bail; use bytes::buf::{Buf, BufMut}; @@ -535,7 +535,7 @@ where unsafe impl LiftReturn for Result where R: LiftReturn, - E: Lift + From, + E: Lift + ConvertError, { fn lift_callback_return(buf: RustBuffer) -> Self { Ok(R::lift_callback_return(buf)) @@ -553,7 +553,7 @@ where } fn handle_callback_unexpected_error(e: UnexpectedUniFFICallbackError) -> Self { - Err(E::from(e)) + Err(E::try_convert_unexpected_callback_error(e).unwrap_or_else(|e| panic!("{e}"))) } const TYPE_ID_META: MetadataBuffer = MetadataBuffer::from_code(metadata::codes::TYPE_RESULT) diff --git a/uniffi_core/src/ffi_converter_traits.rs b/uniffi_core/src/ffi_converter_traits.rs index 7846374270..3b5914e32f 100644 --- a/uniffi_core/src/ffi_converter_traits.rs +++ b/uniffi_core/src/ffi_converter_traits.rs @@ -347,6 +347,10 @@ pub unsafe trait LiftRef { type LiftType: Lift + Borrow; } +pub trait ConvertError: Sized { + fn try_convert_unexpected_callback_error(e: UnexpectedUniFFICallbackError) -> Result; +} + /// Derive FFI traits /// /// This can be used to derive: @@ -373,6 +377,7 @@ macro_rules! derive_ffi_traits { $crate::derive_ffi_traits!(impl LowerReturn for $ty); $crate::derive_ffi_traits!(impl LiftReturn for $ty); $crate::derive_ffi_traits!(impl LiftRef for $ty); + $crate::derive_ffi_traits!(impl ConvertError for $ty); }; (local $ty:ty) => { @@ -381,6 +386,7 @@ macro_rules! derive_ffi_traits { $crate::derive_ffi_traits!(impl LowerReturn for $ty); $crate::derive_ffi_traits!(impl LiftReturn for $ty); $crate::derive_ffi_traits!(impl LiftRef for $ty); + $crate::derive_ffi_traits!(impl ConvertError for $ty); }; (impl $(<$($generic:ident),*>)? $(::uniffi::)? Lower<$ut:path> for $ty:ty $(where $($where:tt)*)?) => { @@ -448,4 +454,13 @@ macro_rules! derive_ffi_traits { type LiftType = Self; } }; + + (impl $(<$($generic:ident),*>)? $(::uniffi::)? ConvertError<$ut:path> for $ty:ty $(where $($where:tt)*)?) => { + impl $(<$($generic),*>)* $crate::ConvertError<$ut> for $ty $(where $($where)*)* + { + fn try_convert_unexpected_callback_error(e: $crate::UnexpectedUniFFICallbackError) -> $crate::deps::anyhow::Result { + $crate::convert_unexpected_error!(e, $ty) + } + } + }; } diff --git a/uniffi_core/src/lib.rs b/uniffi_core/src/lib.rs index 4bc383d1f4..9003b08f9b 100644 --- a/uniffi_core/src/lib.rs +++ b/uniffi_core/src/lib.rs @@ -45,7 +45,7 @@ pub mod metadata; pub use ffi::*; pub use ffi_converter_traits::{ - FfiConverter, FfiConverterArc, Lift, LiftRef, LiftReturn, Lower, LowerReturn, + ConvertError, FfiConverter, FfiConverterArc, Lift, LiftRef, LiftReturn, Lower, LowerReturn, }; pub use metadata::*; diff --git a/uniffi_macros/src/custom.rs b/uniffi_macros/src/custom.rs index 5fb590f3a1..9d8e5acde6 100644 --- a/uniffi_macros/src/custom.rs +++ b/uniffi_macros/src/custom.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::util::{derive_ffi_traits, ident_to_string, mod_path, tagged_impl_header}; +use crate::util::{derive_all_ffi_traits, ident_to_string, mod_path, tagged_impl_header}; use proc_macro2::{Ident, TokenStream}; use quote::quote; use syn::Path; @@ -15,7 +15,7 @@ pub(crate) fn expand_ffi_converter_custom_type( udl_mode: bool, ) -> syn::Result { let impl_spec = tagged_impl_header("FfiConverter", ident, udl_mode); - let derive_ffi_traits = derive_ffi_traits(ident, udl_mode); + let derive_ffi_traits = derive_all_ffi_traits(ident, udl_mode); let name = ident_to_string(ident); let mod_path = mod_path()?; diff --git a/uniffi_macros/src/enum_.rs b/uniffi_macros/src/enum_.rs index 3ddccdd7c1..32abfa08cc 100644 --- a/uniffi_macros/src/enum_.rs +++ b/uniffi_macros/src/enum_.rs @@ -3,7 +3,7 @@ use quote::quote; use syn::{Data, DataEnum, DeriveInput, Field, Index}; use crate::util::{ - create_metadata_items, derive_ffi_traits, ident_to_string, mod_path, tagged_impl_header, + create_metadata_items, derive_all_ffi_traits, ident_to_string, mod_path, tagged_impl_header, try_metadata_value_from_usize, try_read_field, }; @@ -64,7 +64,7 @@ fn enum_or_error_ffi_converter_impl( ) -> TokenStream { let name = ident_to_string(ident); let impl_spec = tagged_impl_header("FfiConverter", ident, udl_mode); - let derive_ffi_traits = derive_ffi_traits(ident, udl_mode); + let derive_ffi_traits = derive_all_ffi_traits(ident, udl_mode); let mod_path = match mod_path() { Ok(p) => p, Err(e) => return e.into_compile_error(), diff --git a/uniffi_macros/src/error.rs b/uniffi_macros/src/error.rs index 5ab6f4f424..cb25df5d68 100644 --- a/uniffi_macros/src/error.rs +++ b/uniffi_macros/src/error.rs @@ -8,8 +8,8 @@ use syn::{ use crate::{ enum_::{rich_error_ffi_converter_impl, variant_metadata}, util::{ - chain, create_metadata_items, either_attribute_arg, ident_to_string, kw, mod_path, - parse_comma_separated, tagged_impl_header, try_metadata_value_from_usize, + chain, create_metadata_items, derive_ffi_traits, either_attribute_arg, ident_to_string, kw, + mod_path, parse_comma_separated, tagged_impl_header, try_metadata_value_from_usize, AttributeSliceExt, UniffiAttributeArgs, }, }; @@ -87,6 +87,7 @@ fn flat_error_ffi_converter_impl( ) -> TokenStream { let name = ident_to_string(ident); let lower_impl_spec = tagged_impl_header("Lower", ident, udl_mode); + let derive_ffi_traits = derive_ffi_traits(ident, udl_mode, &["ConvertError"]); let mod_path = match mod_path() { Ok(p) => p, Err(e) => return e.into_compile_error(), @@ -161,6 +162,7 @@ fn flat_error_ffi_converter_impl( quote! { #lower_impl #lift_impl + #derive_ffi_traits } } diff --git a/uniffi_macros/src/record.rs b/uniffi_macros/src/record.rs index 453f67ce86..abf2743ec6 100644 --- a/uniffi_macros/src/record.rs +++ b/uniffi_macros/src/record.rs @@ -6,8 +6,8 @@ use syn::{ }; use crate::util::{ - create_metadata_items, derive_ffi_traits, either_attribute_arg, ident_to_string, kw, mod_path, - tagged_impl_header, try_metadata_value_from_usize, try_read_field, AttributeSliceExt, + create_metadata_items, derive_all_ffi_traits, either_attribute_arg, ident_to_string, kw, + mod_path, tagged_impl_header, try_metadata_value_from_usize, try_read_field, AttributeSliceExt, UniffiAttributeArgs, }; @@ -41,7 +41,7 @@ pub(crate) fn record_ffi_converter_impl( udl_mode: bool, ) -> syn::Result { let impl_spec = tagged_impl_header("FfiConverter", ident, udl_mode); - let derive_ffi_traits = derive_ffi_traits(ident, udl_mode); + let derive_ffi_traits = derive_all_ffi_traits(ident, udl_mode); let name = ident_to_string(ident); let mod_path = mod_path()?; let write_impl: TokenStream = record.fields.iter().map(write_field).collect(); diff --git a/uniffi_macros/src/util.rs b/uniffi_macros/src/util.rs index e33c4bc505..9f213ea1d7 100644 --- a/uniffi_macros/src/util.rs +++ b/uniffi_macros/src/util.rs @@ -216,7 +216,7 @@ pub(crate) fn tagged_impl_header( } } -pub(crate) fn derive_ffi_traits(ty: &Ident, udl_mode: bool) -> TokenStream { +pub(crate) fn derive_all_ffi_traits(ty: &Ident, udl_mode: bool) -> TokenStream { if udl_mode { quote! { ::uniffi::derive_ffi_traits!(local #ty); } } else { @@ -224,6 +224,25 @@ pub(crate) fn derive_ffi_traits(ty: &Ident, udl_mode: bool) -> TokenStream { } } +pub(crate) fn derive_ffi_traits(ty: &Ident, udl_mode: bool, trait_names: &[&str]) -> TokenStream { + let trait_idents = trait_names + .iter() + .map(|name| Ident::new(name, Span::call_site())); + if udl_mode { + quote! { + #( + ::uniffi::derive_ffi_traits!(impl #trait_idents for #ty); + )* + } + } else { + quote! { + #( + ::uniffi::derive_ffi_traits!(impl #trait_idents for #ty); + )* + } + } +} + /// Custom keywords pub mod kw { syn::custom_keyword!(async_runtime);