Skip to content

Commit

Permalink
Allow callback interfaces to return other callback interfaces (mozill…
Browse files Browse the repository at this point in the history
…a#1947)

The easy part was implementing `LiftReturn`.  The hard part was figuring
out how to handle TYPE_ID_META in the metadata code.  In general, we
have an issue: should we get that from `Lift`/`LiftReturn` or
`Lower`/`LowerReturn`.

As mentioned in the `fnsig` code, I feel like this is a bid of a
band-aid.  However, I think a real fix would require significant changes
to the FFI trait system and I don't want to do that so close to the
v0.26 release.
  • Loading branch information
bendk committed Jan 22, 2024
1 parent 22a9192 commit cbe337c
Show file tree
Hide file tree
Showing 8 changed files with 101 additions and 7 deletions.
6 changes: 6 additions & 0 deletions fixtures/proc-macro/src/callback_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,10 @@ pub trait TestCallbackInterface {
fn with_bytes(&self, rwb: RecordWithBytes) -> Vec<u8>;
fn try_parse_int(&self, value: String) -> Result<u32, BasicError>;
fn callback_handler(&self, h: std::sync::Arc<Object>) -> u32;
fn get_other_callback_interface(&self) -> Box<dyn OtherCallbackInterface>;
}

#[uniffi::export(callback_interface)]
pub trait OtherCallbackInterface {
fn multiply(&self, a: u32, b: u32) -> u32;
}
2 changes: 2 additions & 0 deletions fixtures/proc-macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ fn call_callback_interface(cb: Box<dyn TestCallbackInterface>) {
Err(BasicError::UnexpectedError { .. }),
));
assert_eq!(42, cb.callback_handler(Object::new()));

assert_eq!(6, cb.get_other_callback_interface().multiply(2, 3));
}

// Type that's defined in the UDL and not wrapped with #[uniffi::export]
Expand Down
6 changes: 6 additions & 0 deletions fixtures/proc-macro/tests/bindings/test_proc_macro.kts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,12 @@ class KtTestCallbackInterface : TestCallbackInterface {
val v = o.takeError(BasicException.InvalidInput());
return v
}

override fun getOtherCallbackInterface() = KtTestCallbackInterface2()
}

class KtTestCallbackInterface2 : OtherCallbackInterface {
override fun multiply(a: UInt, b: UInt) = a * b
}

callCallbackInterface(KtTestCallbackInterface())
7 changes: 7 additions & 0 deletions fixtures/proc-macro/tests/bindings/test_proc_macro.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,13 @@ def callback_handler(self, h):
v = h.take_error(BasicError.InvalidInput())
return v

def get_other_callback_interface(self):
return PyTestCallbackInterface2()

class PyTestCallbackInterface2(OtherCallbackInterface):
def multiply(self, a, b):
return a * b

call_callback_interface(PyTestCallbackInterface())

# udl exposed functions with procmacro types.
Expand Down
10 changes: 10 additions & 0 deletions fixtures/proc-macro/tests/bindings/test_proc_macro.swift
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,16 @@ class SwiftTestCallbackInterface : TestCallbackInterface {
var v = h.takeError(e: BasicError.InvalidInput)
return v
}

func getOtherCallbackInterface() -> OtherCallbackInterface {
SwiftTestCallbackInterface2()
}
}

class SwiftTestCallbackInterface2 : OtherCallbackInterface {
func multiply(a: UInt32, b: UInt32) -> UInt32 {
return a * b;
}
}

callCallbackInterface(cb: SwiftTestCallbackInterface())
12 changes: 6 additions & 6 deletions uniffi_macros/src/export/callback_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
use crate::{
export::ImplItem,
fnsig::{FnKind, FnSignature, ReceiverArg},
util::{create_metadata_items, ident_to_string, mod_path, tagged_impl_header},
util::{
create_metadata_items, derive_ffi_traits, ident_to_string, mod_path, tagged_impl_header,
},
};
use proc_macro2::{Span, TokenStream};
use quote::quote;
Expand Down Expand Up @@ -96,7 +98,7 @@ pub fn ffi_converter_callback_interface_impl(
let dyn_trait = quote! { dyn #trait_ident };
let box_dyn_trait = quote! { ::std::boxed::Box<#dyn_trait> };
let lift_impl_spec = tagged_impl_header("Lift", &box_dyn_trait, udl_mode);
let lift_ref_impl_spec = tagged_impl_header("LiftRef", &dyn_trait, udl_mode);
let derive_ffi_traits = derive_ffi_traits(&box_dyn_trait, udl_mode, &["LiftRef", "LiftReturn"]);
let mod_path = match mod_path() {
Ok(p) => p,
Err(e) => return e.into_compile_error(),
Expand Down Expand Up @@ -125,9 +127,7 @@ pub fn ffi_converter_callback_interface_impl(
.concat_str(#trait_name);
}

unsafe #lift_ref_impl_spec {
type LiftType = #box_dyn_trait;
}
#derive_ffi_traits
}
}

Expand Down Expand Up @@ -199,7 +199,7 @@ pub(super) fn metadata_items(

iter::once(Ok(callback_interface_items))
.chain(items.iter().map(|item| match item {
ImplItem::Method(sig) => sig.metadata_items(),
ImplItem::Method(sig) => sig.metadata_items_for_callback_interface(),
_ => unreachable!("traits have no constructors"),
}))
.collect()
Expand Down
59 changes: 59 additions & 0 deletions uniffi_macros/src/fnsig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,65 @@ impl FnSignature {
}
}

/// Generate metadata items for callback interfaces
///
/// Unfortunately, most of this is duplicate code from [Self::metadata_items] and
/// [Self::metadata_expr]. However, one issue with that code is that it needs to assume if the
/// arguments are being lifted vs lowered in order to get TYPE_ID_META. That code uses
/// `<Type as Lift>::TYPE_ID_META` for arguments and `<Type as LowerReturn>::TYPE_ID_META` for
/// return types, which works for accidental/historical reasons.
///
/// The one exception is callback interfaces (#1947), which are handled by this method.
///
/// TODO: fix the metadata system so that this is not needed.
pub(crate) fn metadata_items_for_callback_interface(&self) -> syn::Result<TokenStream> {
let Self {
name,
return_ty,
is_async,
mod_path,
docstring,
..
} = &self;
match &self.kind {
FnKind::TraitMethod {
self_ident, index, ..
} => {
let object_name = ident_to_string(self_ident);
let args_len = try_metadata_value_from_usize(
// Use param_lifts to calculate this instead of sig.inputs to avoid counting any self
// params
self.args.len(),
"UniFFI limits functions to 256 arguments",
)?;
let arg_metadata_calls = self.args.iter().map(NamedArg::arg_metadata);
let metadata_expr = quote! {
::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::TRAIT_METHOD)
.concat_str(#mod_path)
.concat_str(#object_name)
.concat_u32(#index)
.concat_str(#name)
.concat_bool(#is_async)
.concat_value(#args_len)
#(#arg_metadata_calls)*
.concat(<#return_ty as ::uniffi::LiftReturn<crate::UniFfiTag>>::TYPE_ID_META)
.concat_long_str(#docstring)
};
Ok(create_metadata_items(
"method",
&format!("{object_name}_{name}"),
metadata_expr,
Some(self.checksum_symbol_name()),
))
}

// This should never happen and indicates an error in the internal code
_ => panic!(
"metadata_items_for_callback_interface can only be called with `TraitMethod` sigs"
),
}
}

pub(crate) fn checksum_symbol_name(&self) -> String {
let name = &self.name;
match &self.kind {
Expand Down
6 changes: 5 additions & 1 deletion uniffi_macros/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,11 @@ pub(crate) fn derive_all_ffi_traits(ty: &Ident, udl_mode: bool) -> TokenStream {
}
}

pub(crate) fn derive_ffi_traits(ty: &Ident, udl_mode: bool, trait_names: &[&str]) -> TokenStream {
pub(crate) fn derive_ffi_traits(
ty: impl ToTokens,
udl_mode: bool,
trait_names: &[&str],
) -> TokenStream {
let trait_idents = trait_names
.iter()
.map(|name| Ident::new(name, Span::call_site()));
Expand Down

0 comments on commit cbe337c

Please sign in to comment.