Make UDL generation implement FFI traits for all tags (#1865)

bendk · bendk · commit 5681d5553133 · 2024-05-04T10:11:27.000-04:00
This means it works exactly like the proc-macro code which simplifies
things significantly.

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)
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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
@@ -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
@@ -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<UniffiOneEnum>,
diff --git 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<String, askama::Error> {
-        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
diff --git 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
@@ -183,73 +183,6 @@ macro_rules! ffi_converter_rust_buffer_lift_and_lower {
     };
 }
 
-/// Macro to implement `FfiConverter<T>` 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<T>` 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<T>
-    };
-}
-
-// 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<u8>) {
-                <$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
@@ -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
@@ -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
@@ -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
@@ -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", &quote! { dyn #trait_ident }, udl_mode);
-    let lift_ref_impl_spec = tagged_impl_header("LiftRef", &quote! { dyn #trait_ident }, udl_mode);
+    // TODO: support defining remote trait interfaces
+    let remote = false;
+    let impl_spec = tagged_impl_header("FfiConverterArc", &quote! { dyn #trait_ident }, remote);
+    let lift_ref_impl_spec = tagged_impl_header("LiftRef", &quote! { 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
@@ -22,6 +22,7 @@ mod ffiops;
 mod fnsig;
 mod object;
 mod record;
+mod remote;
 mod setup_scaffolding;
 mod test;
 mod util;
@@ -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(<kind>)]`
-// 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<UT> parameter
-//
-// In UDL-mode, we only implement `FfiConverter` for the local tag (`FfiConverter<crate::UniFfiTag>`)
-//
-// 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<crate::UniFfiTag>` 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(<kind>)]`
+/// 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 https://mozilla.github.io/uniffi-rs/udl/remote_ext_types.html 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
diff --git a/uniffi_macros/src/util.rs b/uniffi_macros/src/util.rs

Original file line number	Diff line number	Diff line change
`@@ -70,8 +70,7 @@ impl DeriveOptions {`
`70`	`70`	/// Construct DeriveOptions for `udl_derive`
`71`	`71`	`pub fn udl_derive() -> Self {`
`72`	`72`	`Self {`
`73`		`- // TODO: change this to false`
`74`		`- local_tag: true,`
	`73`	`+ local_tag: false,`
`75`	`74`	`generate_metadata: false,`
`76`	`75`	`}`
`77`	`76`	`}`