From 268453c910b235c5149a5615283de34e00bec53c Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Fri, 3 May 2024 17:17:07 -0400 Subject: [PATCH] Make UDL generation implement FFI traits for all tags (#1865) This means it works exactly like the proc-macro code which simplifies things significantly. Also, this fixes the ext-types fixture to work with types that are custom, remote, and external. Removed the `use_udl_*` macros, they're not needed anymore. Added the `remote_type!` macro, it's now needed for using remote types who's FFI trait impls are defined in another crate (which is now possible from using UDL and proc-macros) One feature that we've lost in defining callback/trait interfaces using traits defined in a remote crate. I doubt anyone is using that feature, but I think the plan should be to re-implement it before the next release. As part of the same work, I think we can also support methods on remote interfaces. --- fixtures/ext-types/lib/src/ext-types-lib.udl | 7 +- fixtures/ext-types/lib/src/lib.rs | 14 +--- fixtures/ext-types/proc-macro-lib/src/lib.rs | 7 +- fixtures/ext-types/sub-lib/src/lib.rs | 3 - uniffi_bindgen/src/scaffolding/mod.rs | 7 +- .../templates/ExternalTypesTemplate.rs | 17 ---- .../templates/scaffolding_template.rs | 3 - uniffi_core/src/lib.rs | 67 --------------- uniffi_macros/src/derive.rs | 3 +- uniffi_macros/src/export.rs | 2 +- .../src/export/callback_interface.rs | 9 +- uniffi_macros/src/export/trait_interface.rs | 9 +- uniffi_macros/src/lib.rs | 82 ++++--------------- uniffi_macros/src/remote.rs | 62 ++++++++++++++ uniffi_macros/src/util.rs | 8 +- 15 files changed, 103 insertions(+), 197 deletions(-) delete mode 100644 uniffi_bindgen/src/scaffolding/templates/ExternalTypesTemplate.rs create mode 100644 uniffi_macros/src/remote.rs diff --git a/fixtures/ext-types/lib/src/ext-types-lib.udl b/fixtures/ext-types/lib/src/ext-types-lib.udl index 828a499874..94ffffcdba 100644 --- a/fixtures/ext-types/lib/src/ext-types-lib.udl +++ b/fixtures/ext-types/lib/src/ext-types-lib.udl @@ -55,17 +55,20 @@ typedef extern Url; [External="custom_types"] typedef extern Handle; -// Here are some different kinds of "external" types - the types are described +// Here are some different kinds of remote types - the types are described // in this UDL, but the types themselves are defined in a different crate. + +[Remote] dictionary ExternalCrateDictionary { string sval; }; +[Remote] interface ExternalCrateInterface { string value(); }; -[NonExhaustive] +[Remote, NonExhaustive] enum ExternalCrateNonExhaustiveEnum { "One", "Two", diff --git a/fixtures/ext-types/lib/src/lib.rs b/fixtures/ext-types/lib/src/lib.rs index 63fdac77ae..9b39c0231a 100644 --- a/fixtures/ext-types/lib/src/lib.rs +++ b/fixtures/ext-types/lib/src/lib.rs @@ -11,17 +11,9 @@ use uniffi_one::{ use uniffi_sublib::SubLibType; use url::Url; -// #1988 -uniffi::ffi_converter_forward!( - ext_types_custom::Ouid, - ext_types_custom::UniFfiTag, - crate::UniFfiTag -); -uniffi::ffi_converter_forward!( - ext_types_custom::ANestedGuid, - ext_types_custom::UniFfiTag, - crate::UniFfiTag -); +// Remote types require a macro call in the Rust source + +uniffi::remote_type!(Url, custom_types); pub struct CombinedType { pub uoe: UniffiOneEnum, diff --git a/fixtures/ext-types/proc-macro-lib/src/lib.rs b/fixtures/ext-types/proc-macro-lib/src/lib.rs index 26bdf31510..d501599e6c 100644 --- a/fixtures/ext-types/proc-macro-lib/src/lib.rs +++ b/fixtures/ext-types/proc-macro-lib/src/lib.rs @@ -6,12 +6,7 @@ use uniffi_one::{ }; use url::Url; -uniffi::use_udl_record!(uniffi_one, UniffiOneType); -uniffi::use_udl_enum!(uniffi_one, UniffiOneEnum); -uniffi::use_udl_object!(uniffi_one, UniffiOneInterface); -uniffi::use_udl_record!(ext_types_custom, Guid); -uniffi::use_udl_record!(custom_types, Url); -uniffi::use_udl_record!(custom_types, Handle); +uniffi::remote_type!(Url, custom_types); #[derive(uniffi::Record)] pub struct CombinedType { diff --git a/fixtures/ext-types/sub-lib/src/lib.rs b/fixtures/ext-types/sub-lib/src/lib.rs index 565e7f4616..49f97fc004 100644 --- a/fixtures/ext-types/sub-lib/src/lib.rs +++ b/fixtures/ext-types/sub-lib/src/lib.rs @@ -1,9 +1,6 @@ use std::sync::Arc; use uniffi_one::{UniffiOneEnum, UniffiOneInterface, UniffiOneTrait}; -uniffi::use_udl_object!(uniffi_one, UniffiOneInterface); -uniffi::use_udl_enum!(uniffi_one, UniffiOneEnum); - #[derive(Default, uniffi::Record)] pub struct SubLibType { pub maybe_enum: Option, diff --git a/uniffi_bindgen/src/scaffolding/mod.rs b/uniffi_bindgen/src/scaffolding/mod.rs index 7fd81831aa..ba7335fe80 100644 --- a/uniffi_bindgen/src/scaffolding/mod.rs +++ b/uniffi_bindgen/src/scaffolding/mod.rs @@ -7,7 +7,7 @@ use askama::Template; use std::borrow::Borrow; use super::interface::*; -use heck::{ToShoutySnakeCase, ToSnakeCase}; +use heck::ToShoutySnakeCase; #[derive(Template)] #[template(syntax = "rs", escape = "none", path = "scaffolding_template.rs")] @@ -71,9 +71,4 @@ mod filters { Type::External { name, .. } => format!("r#{name}"), }) } - - // 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/ExternalTypesTemplate.rs b/uniffi_bindgen/src/scaffolding/templates/ExternalTypesTemplate.rs deleted file mode 100644 index db0517f32d..0000000000 --- a/uniffi_bindgen/src/scaffolding/templates/ExternalTypesTemplate.rs +++ /dev/null @@ -1,17 +0,0 @@ -// Support for external types. - -// Types with an external `FfiConverter`... -{% for (name, crate_name, kind, tagged) in ci.iter_external_types() %} -// The FfiConverter for `{{ name }}` is defined in `{{ crate_name }}` -// If it has its existing FfiConverter defined with a UniFFITag, it needs forwarding. -{% if tagged %} -{%- match kind %} -{%- when ExternalKind::DataClass %} -::uniffi::ffi_converter_forward!(r#{{ name }}, ::{{ crate_name|crate_name_rs }}::UniFfiTag, crate::UniFfiTag); -{%- when ExternalKind::Interface %} -::uniffi::ffi_converter_arc_forward!(r#{{ name }}, ::{{ crate_name|crate_name_rs }}::UniFfiTag, crate::UniFfiTag); -{%- when ExternalKind::Trait %} -::uniffi::ffi_converter_arc_forward!(dyn r#{{ name }}, ::{{ crate_name|crate_name_rs }}::UniFfiTag, crate::UniFfiTag); -{%- endmatch %} -{% endif %} -{%- endfor %} diff --git a/uniffi_bindgen/src/scaffolding/templates/scaffolding_template.rs b/uniffi_bindgen/src/scaffolding/templates/scaffolding_template.rs index c88e204e97..e5554b1fa9 100644 --- a/uniffi_bindgen/src/scaffolding/templates/scaffolding_template.rs +++ b/uniffi_bindgen/src/scaffolding/templates/scaffolding_template.rs @@ -45,8 +45,5 @@ uniffi::deps::static_assertions::assert_impl_all!({{ k|type_rs }}: ::std::cmp::E {% include "CallbackInterfaceTemplate.rs" %} {% endfor %} -// External and Wrapped types -{% include "ExternalTypesTemplate.rs" %} - // Export scaffolding checksums for UDL items {% include "Checksums.rs" %} diff --git a/uniffi_core/src/lib.rs b/uniffi_core/src/lib.rs index 53fa6641c3..0987284a64 100644 --- a/uniffi_core/src/lib.rs +++ b/uniffi_core/src/lib.rs @@ -183,73 +183,6 @@ macro_rules! ffi_converter_rust_buffer_lift_and_lower { }; } -/// Macro to implement `FfiConverter` for a UniFfiTag using a different UniFfiTag -/// -/// This is used for external types -#[macro_export] -macro_rules! ffi_converter_forward { - // Forward a `FfiConverter` implementation - ($T:ty, $existing_impl_tag:ty, $new_impl_tag:ty) => { - ::uniffi::do_ffi_converter_forward!( - FfiConverter, - $T, - $T, - $existing_impl_tag, - $new_impl_tag - ); - - $crate::derive_ffi_traits!(local $T); - }; -} - -/// Macro to implement `FfiConverterArc` for a UniFfiTag using a different UniFfiTag -/// -/// This is used for external types -#[macro_export] -macro_rules! ffi_converter_arc_forward { - ($T:ty, $existing_impl_tag:ty, $new_impl_tag:ty) => { - ::uniffi::do_ffi_converter_forward!( - FfiConverterArc, - ::std::sync::Arc<$T>, - $T, - $existing_impl_tag, - $new_impl_tag - ); - - // Note: no need to call derive_ffi_traits! because there is a blanket impl for all Arc - }; -} - -// Generic code between the two macros above -#[doc(hidden)] -#[macro_export] -macro_rules! do_ffi_converter_forward { - ($trait:ident, $rust_type:ty, $T:ty, $existing_impl_tag:ty, $new_impl_tag:ty) => { - unsafe impl $crate::$trait<$new_impl_tag> for $T { - type FfiType = <$T as $crate::$trait<$existing_impl_tag>>::FfiType; - - fn lower(obj: $rust_type) -> Self::FfiType { - <$T as $crate::$trait<$existing_impl_tag>>::lower(obj) - } - - fn try_lift(v: Self::FfiType) -> $crate::Result<$rust_type> { - <$T as $crate::$trait<$existing_impl_tag>>::try_lift(v) - } - - fn write(obj: $rust_type, buf: &mut Vec) { - <$T as $crate::$trait<$existing_impl_tag>>::write(obj, buf) - } - - fn try_read(buf: &mut &[u8]) -> $crate::Result<$rust_type> { - <$T as $crate::$trait<$existing_impl_tag>>::try_read(buf) - } - - const TYPE_ID_META: ::uniffi::MetadataBuffer = - <$T as $crate::$trait<$existing_impl_tag>>::TYPE_ID_META; - } - }; -} - #[cfg(test)] mod test { use super::{FfiConverter, UniFfiTag}; diff --git a/uniffi_macros/src/derive.rs b/uniffi_macros/src/derive.rs index 509409b96c..ca92cd4776 100644 --- a/uniffi_macros/src/derive.rs +++ b/uniffi_macros/src/derive.rs @@ -70,8 +70,7 @@ impl DeriveOptions { /// Construct DeriveOptions for `udl_derive` pub fn udl_derive() -> Self { Self { - // TODO: change this to false - local_tag: true, + local_tag: false, generate_metadata: false, } } diff --git a/uniffi_macros/src/export.rs b/uniffi_macros/src/export.rs index 3843544b21..025385808e 100644 --- a/uniffi_macros/src/export.rs +++ b/uniffi_macros/src/export.rs @@ -112,7 +112,7 @@ pub(crate) fn expand_export( quote! { #(#items)* } }); let ffi_converter_tokens = - ffi_converter_callback_interface_impl(&self_ident, &trait_impl_ident, udl_mode); + ffi_converter_callback_interface_impl(&self_ident, &trait_impl_ident); Ok(quote! { #trait_impl diff --git a/uniffi_macros/src/export/callback_interface.rs b/uniffi_macros/src/export/callback_interface.rs index e9621913b7..2244b266a9 100644 --- a/uniffi_macros/src/export/callback_interface.rs +++ b/uniffi_macros/src/export/callback_interface.rs @@ -129,14 +129,15 @@ pub fn trait_impl_ident(trait_name: &str) -> Ident { pub fn ffi_converter_callback_interface_impl( trait_ident: &Ident, trait_impl_ident: &Ident, - udl_mode: bool, ) -> TokenStream { + // TODO: support remote callback interfaces + let remote = false; let trait_name = ident_to_string(trait_ident); 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 type_id_impl_spec = tagged_impl_header("TypeId", &box_dyn_trait, udl_mode); - let derive_ffi_traits = derive_ffi_traits(&box_dyn_trait, udl_mode, &["LiftRef", "LiftReturn"]); + let lift_impl_spec = tagged_impl_header("Lift", &box_dyn_trait, remote); + let type_id_impl_spec = tagged_impl_header("TypeId", &box_dyn_trait, remote); + let derive_ffi_traits = derive_ffi_traits(&box_dyn_trait, remote, &["LiftRef", "LiftReturn"]); let mod_path = match mod_path() { Ok(p) => p, Err(e) => return e.into_compile_error(), diff --git a/uniffi_macros/src/export/trait_interface.rs b/uniffi_macros/src/export/trait_interface.rs index 51eb94d031..447e52377f 100644 --- a/uniffi_macros/src/export/trait_interface.rs +++ b/uniffi_macros/src/export/trait_interface.rs @@ -99,7 +99,7 @@ pub(super) fn gen_trait_scaffolding( interface_meta_static_var(&self_ident, imp, mod_path, docstring.as_str()) .unwrap_or_else(syn::Error::into_compile_error) }); - let ffi_converter_tokens = ffi_converter(mod_path, &self_ident, udl_mode, with_foreign); + let ffi_converter_tokens = ffi_converter(mod_path, &self_ident, with_foreign); Ok(quote_spanned! { self_ident.span() => #meta_static_var @@ -113,11 +113,12 @@ pub(super) fn gen_trait_scaffolding( pub(crate) fn ffi_converter( mod_path: &str, trait_ident: &Ident, - udl_mode: bool, with_foreign: bool, ) -> TokenStream { - let impl_spec = tagged_impl_header("FfiConverterArc", "e! { dyn #trait_ident }, udl_mode); - let lift_ref_impl_spec = tagged_impl_header("LiftRef", "e! { dyn #trait_ident }, udl_mode); + // TODO: support defining remote trait interfaces + let remote = false; + let impl_spec = tagged_impl_header("FfiConverterArc", "e! { dyn #trait_ident }, remote); + let lift_ref_impl_spec = tagged_impl_header("LiftRef", "e! { dyn #trait_ident }, remote); let trait_name = ident_to_string(trait_ident); let try_lift = if with_foreign { let trait_impl_ident = callback_interface::trait_impl_ident(&trait_name); diff --git a/uniffi_macros/src/lib.rs b/uniffi_macros/src/lib.rs index eed9f1b88a..f7d92d8454 100644 --- a/uniffi_macros/src/lib.rs +++ b/uniffi_macros/src/lib.rs @@ -22,6 +22,7 @@ mod ffiops; mod fnsig; mod object; mod record; +mod remote; mod setup_scaffolding; mod test; mod util; @@ -139,7 +140,7 @@ pub fn custom_newtype(tokens: TokenStream) -> TokenStream { /// implements the FFI traits for the local UniFfiTag. /// /// Use this to wrap the definition of an item defined in a remote crate. -/// See https://mozilla.github.io/uniffi-rs/udl/remote_ext_types.html for details. +/// See `` for details. #[doc(hidden)] #[proc_macro_attribute] pub fn remote(attrs: TokenStream, input: TokenStream) -> TokenStream { @@ -170,34 +171,14 @@ pub fn udl_remote(attrs: TokenStream, input: TokenStream) -> TokenStream { .into() } -// Derive items for UDL mode -// -// The Askama templates generate placeholder items wrapped with the `#[udl_derive()]` -// attribute. The macro code then generates derived items based on the input. This system ensures -// that the same code path is used for UDL-based code and proc-macros. -// -// # Differences between UDL-mode and normal mode -// -// ## Metadata symbols / checksum functions -// -// In UDL mode, we don't export the static metadata symbols or generate the checksum -// functions. This could be changed, but there doesn't seem to be much benefit at this point. -// -// ## The FfiConverter parameter -// -// In UDL-mode, we only implement `FfiConverter` for the local tag (`FfiConverter`) -// -// The reason for this split is remote types, i.e. types defined in remote crates that we -// don't control and therefore can't define a blanket impl on because of the orphan rules. -// -// With UDL, we handle this by only implementing `FfiConverter` for the -// type. This gets around the orphan rules since a local type is in the trait, but requires -// a `uniffi::ffi_converter_forward!` call if the type is used in a second local crate (an -// External typedef). This is natural for UDL-based generation, since you always need to -// define the external type in the UDL file. -// -// With proc-macros this system isn't so natural. Instead, we create a blanket implementation -// for all UT and support for remote types is still TODO. +/// Derive items for UDL mode +/// +/// The Askama templates generate placeholder items wrapped with the `#[udl_derive()]` +/// attribute. The macro code then generates derived items based on the input. This system ensures +/// that the same code path is used for UDL-based code and proc-macros. +/// +/// `udl_derive` works almost exactly like the `derive_*` macros, except it doesn't generate +/// metadata items, since we get those from parsing the UDL. #[doc(hidden)] #[proc_macro_attribute] pub fn udl_derive(attrs: TokenStream, input: TokenStream) -> TokenStream { @@ -267,45 +248,12 @@ pub fn include_scaffolding(udl_stem: TokenStream) -> TokenStream { }.into() } -// Use a UniFFI types from dependent crates that uses UDL files -// See the derive_for_udl and export_for_udl section for a discussion of why this is needed. -#[proc_macro] -pub fn use_udl_record(tokens: TokenStream) -> TokenStream { - use_udl_simple_type(tokens) -} - -#[proc_macro] -pub fn use_udl_enum(tokens: TokenStream) -> TokenStream { - use_udl_simple_type(tokens) -} - -#[proc_macro] -pub fn use_udl_error(tokens: TokenStream) -> TokenStream { - use_udl_simple_type(tokens) -} - -fn use_udl_simple_type(tokens: TokenStream) -> TokenStream { - let util::ExternalTypeItem { - crate_ident, - type_ident, - .. - } = parse_macro_input!(tokens); - quote! { - ::uniffi::ffi_converter_forward!(#type_ident, #crate_ident::UniFfiTag, crate::UniFfiTag); - } - .into() -} - +/// Use the FFI trait implementations defined in another crate for a remote type +/// +/// See `` for details. #[proc_macro] -pub fn use_udl_object(tokens: TokenStream) -> TokenStream { - let util::ExternalTypeItem { - crate_ident, - type_ident, - .. - } = parse_macro_input!(tokens); - quote! { - ::uniffi::ffi_converter_arc_forward!(#type_ident, #crate_ident::UniFfiTag, crate::UniFfiTag); - }.into() +pub fn remote_type(tokens: TokenStream) -> TokenStream { + remote::expand_remote_type(parse_macro_input!(tokens)).into() } /// A helper macro to generate and include component scaffolding. diff --git a/uniffi_macros/src/remote.rs b/uniffi_macros/src/remote.rs new file mode 100644 index 0000000000..8ad88cd04d --- /dev/null +++ b/uniffi_macros/src/remote.rs @@ -0,0 +1,62 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * 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::TokenStream; +use quote::quote; +use syn::{ + parse::{Parse, ParseStream}, + Ident, Token, Type, +}; + +pub struct RemoteTypeArgs { + pub ty: Type, + pub sep: Token![,], + pub implementing_crate: Ident, +} + +impl Parse for RemoteTypeArgs { + fn parse(input: ParseStream<'_>) -> syn::Result { + Ok(Self { + ty: input.parse()?, + sep: input.parse()?, + implementing_crate: input.parse()?, + }) + } +} + +pub fn expand_remote_type(args: RemoteTypeArgs) -> TokenStream { + let RemoteTypeArgs { + ty, + implementing_crate, + .. + } = args; + let existing_tag = quote! { #implementing_crate::UniFfiTag }; + + quote! { + unsafe impl ::uniffi::FfiConverter for #ty { + type FfiType = <#ty as ::uniffi::FfiConverter<#existing_tag>>::FfiType; + + fn lower(obj: #ty) -> Self::FfiType { + <#ty as ::uniffi::FfiConverter<#existing_tag>>::lower(obj) + } + + fn try_lift(v: Self::FfiType) -> ::uniffi::Result<#ty> { + <#ty as ::uniffi::FfiConverter<#existing_tag>>::try_lift(v) + } + + fn write(obj: #ty, buf: &mut Vec) { + <#ty as ::uniffi::FfiConverter<#existing_tag>>::write(obj, buf) + } + + fn try_read(buf: &mut &[u8]) -> ::uniffi::Result<#ty> { + <#ty as ::uniffi::FfiConverter<#existing_tag>>::try_read(buf) + } + + const TYPE_ID_META: ::uniffi::MetadataBuffer = + <#ty as ::uniffi::FfiConverter<#existing_tag>>::TYPE_ID_META; + } + + ::uniffi::derive_ffi_traits!(local #ty); + } +} diff --git a/uniffi_macros/src/util.rs b/uniffi_macros/src/util.rs index 56368b15b8..37d709fb18 100644 --- a/uniffi_macros/src/util.rs +++ b/uniffi_macros/src/util.rs @@ -206,10 +206,10 @@ pub fn either_attribute_arg(a: Option, b: Option) -> syn::Res pub(crate) fn tagged_impl_header( trait_name: &str, ident: &impl ToTokens, - udl_mode: bool, + remote: bool, ) -> TokenStream { let trait_name = Ident::new(trait_name, Span::call_site()); - if udl_mode { + if remote { quote! { impl ::uniffi::#trait_name for #ident } } else { quote! { impl ::uniffi::#trait_name for #ident } @@ -218,13 +218,13 @@ pub(crate) fn tagged_impl_header( pub(crate) fn derive_ffi_traits( ty: impl ToTokens, - udl_mode: bool, + remote: bool, trait_names: &[&str], ) -> TokenStream { let trait_idents = trait_names .iter() .map(|name| Ident::new(name, Span::call_site())); - if udl_mode { + if remote { quote! { #( ::uniffi::derive_ffi_traits!(impl #trait_idents for #ty);