Adding the TypeId FFI trait

The metadata code now uses this trait to get the `TYPE_ID_METADATA` rather than Lift/Lower. This feels cleaner, since it was often not clear which trait to use before. This removes the hacky handling of type id metadata for callback interfaces that I added in #1950
mozilla · Jan 22, 2024 · 6725e5e · 6725e5e
1 parent ba95908
commit 6725e5e
Show file tree

Hide file tree

Showing 12 changed files with 98 additions and 120 deletions.
diff --git a/fixtures/metadata/src/tests.rs b/fixtures/metadata/src/tests.rs
@@ -103,9 +103,9 @@ mod test_type_ids {
     use super::*;
     use std::collections::HashMap;
     use std::sync::Arc;
-    use uniffi_core::Lower;
+    use uniffi_core::TypeId;
 
-    fn check_type_id<T: Lower<UniFfiTag>>(correct_type: Type) {
+    fn check_type_id<T: TypeId<UniFfiTag>>(correct_type: Type) {
         let buf = &mut T::TYPE_ID_META.as_ref();
         assert_eq!(
             uniffi_meta::read_metadata_type(buf).unwrap(),

diff --git a/uniffi/tests/ui/proc_macro_arc.stderr b/uniffi/tests/ui/proc_macro_arc.stderr
@@ -10,6 +10,17 @@ error[E0277]: the trait bound `Foo: FfiConverterArc<UniFfiTag>` is not satisfied
    = note: required for `Arc<Foo>` to implement `LowerReturn<UniFfiTag>`
    = note: this error originates in the attribute macro `uniffi::export` (in Nightly builds, run with -Z macro-backtrace for more info)
 
+error[E0277]: the trait bound `Foo: FfiConverterArc<UniFfiTag>` is not satisfied
+  --> tests/ui/proc_macro_arc.rs:10:1
+   |
+10 | #[uniffi::export]
+   | ^^^^^^^^^^^^^^^^^ the trait `FfiConverterArc<UniFfiTag>` is not implemented for `Foo`
+   |
+   = help: the trait `uniffi::TypeId<UT>` is implemented for `Arc<T>`
+   = note: required for `Arc<Foo>` to implement `FfiConverter<UniFfiTag>`
+   = note: required for `Arc<Foo>` to implement `uniffi::TypeId<UniFfiTag>`
+   = note: this error originates in the attribute macro `uniffi::export` (in Nightly builds, run with -Z macro-backtrace for more info)
+
 error[E0277]: the trait bound `child::Foo: FfiConverterArc<UniFfiTag>` is not satisfied
   --> tests/ui/proc_macro_arc.rs:20:5
    |
@@ -20,3 +31,14 @@ error[E0277]: the trait bound `child::Foo: FfiConverterArc<UniFfiTag>` is not sa
    = note: required for `Arc<child::Foo>` to implement `FfiConverter<UniFfiTag>`
    = note: required for `Arc<child::Foo>` to implement `Lift<UniFfiTag>`
    = note: this error originates in the attribute macro `uniffi::export` (in Nightly builds, run with -Z macro-backtrace for more info)
+
+error[E0277]: the trait bound `child::Foo: FfiConverterArc<UniFfiTag>` is not satisfied
+  --> tests/ui/proc_macro_arc.rs:20:5
+   |
+20 |     #[uniffi::export]
+   |     ^^^^^^^^^^^^^^^^^ the trait `FfiConverterArc<UniFfiTag>` is not implemented for `child::Foo`
+   |
+   = help: the trait `uniffi::TypeId<UT>` is implemented for `Arc<T>`
+   = note: required for `Arc<child::Foo>` to implement `FfiConverter<UniFfiTag>`
+   = note: required for `Arc<child::Foo>` to implement `uniffi::TypeId<UniFfiTag>`
+   = note: this error originates in the attribute macro `uniffi::export` (in Nightly builds, run with -Z macro-backtrace for more info)
diff --git a/uniffi_core/src/ffi_converter_impls.rs b/uniffi_core/src/ffi_converter_impls.rs
@@ -24,7 +24,7 @@
 use crate::{
     check_remaining, derive_ffi_traits, ffi_converter_rust_buffer_lift_and_lower, metadata,
     ConvertError, FfiConverter, Lift, LiftRef, LiftReturn, Lower, LowerReturn, MetadataBuffer,
-    Result, RustBuffer, UnexpectedUniFFICallbackError,
+    Result, RustBuffer, TypeId, UnexpectedUniFFICallbackError,
 };
 use anyhow::bail;
 use bytes::buf::{Buf, BufMut};
@@ -270,9 +270,6 @@ unsafe impl<UT, T: Lower<UT>> Lower<UT> for Option<T> {
     fn lower(obj: Option<T>) -> RustBuffer {
         Self::lower_into_rust_buffer(obj)
     }
-
-    const TYPE_ID_META: MetadataBuffer =
-        MetadataBuffer::from_code(metadata::codes::TYPE_OPTION).concat(T::TYPE_ID_META);
 }
 
 unsafe impl<UT, T: Lift<UT>> Lift<UT> for Option<T> {
@@ -290,7 +287,9 @@ unsafe impl<UT, T: Lift<UT>> Lift<UT> for Option<T> {
     fn try_lift(buf: RustBuffer) -> Result<Option<T>> {
         Self::try_lift_from_rust_buffer(buf)
     }
+}
 
+impl<UT, T: TypeId<UT>> TypeId<UT> for Option<T> {
     const TYPE_ID_META: MetadataBuffer =
         MetadataBuffer::from_code(metadata::codes::TYPE_OPTION).concat(T::TYPE_ID_META);
 }
@@ -320,9 +319,6 @@ unsafe impl<UT, T: Lower<UT>> Lower<UT> for Vec<T> {
     fn lower(obj: Vec<T>) -> RustBuffer {
         Self::lower_into_rust_buffer(obj)
     }
-
-    const TYPE_ID_META: MetadataBuffer =
-        MetadataBuffer::from_code(metadata::codes::TYPE_VEC).concat(T::TYPE_ID_META);
 }
 
 /// Support for associative arrays via the FFI - `record<u32, u64>` in UDL.
@@ -346,7 +342,9 @@ unsafe impl<UT, T: Lift<UT>> Lift<UT> for Vec<T> {
     fn try_lift(buf: RustBuffer) -> Result<Vec<T>> {
         Self::try_lift_from_rust_buffer(buf)
     }
+}
 
+impl<UT, T: TypeId<UT>> TypeId<UT> for Vec<T> {
     const TYPE_ID_META: MetadataBuffer =
         MetadataBuffer::from_code(metadata::codes::TYPE_VEC).concat(T::TYPE_ID_META);
 }
@@ -371,10 +369,6 @@ where
     fn lower(obj: HashMap<K, V>) -> RustBuffer {
         Self::lower_into_rust_buffer(obj)
     }
-
-    const TYPE_ID_META: MetadataBuffer = MetadataBuffer::from_code(metadata::codes::TYPE_HASH_MAP)
-        .concat(K::TYPE_ID_META)
-        .concat(V::TYPE_ID_META);
 }
 
 unsafe impl<K, V, UT> Lift<UT> for HashMap<K, V>
@@ -399,7 +393,13 @@ where
     fn try_lift(buf: RustBuffer) -> Result<HashMap<K, V>> {
         Self::try_lift_from_rust_buffer(buf)
     }
+}
 
+impl<K, V, UT> TypeId<UT> for HashMap<K, V>
+where
+    K: TypeId<UT> + std::hash::Hash + Eq,
+    V: TypeId<UT>,
+{
     const TYPE_ID_META: MetadataBuffer = MetadataBuffer::from_code(metadata::codes::TYPE_HASH_MAP)
         .concat(K::TYPE_ID_META)
         .concat(V::TYPE_ID_META);
@@ -442,6 +442,7 @@ derive_ffi_traits!(impl<T, UT> Lift<UT> for Arc<T> where Arc<T>: FfiConverter<UT
 derive_ffi_traits!(impl<T, UT> LowerReturn<UT> for Arc<T> where Arc<T>: Lower<UT>, T: ?Sized);
 derive_ffi_traits!(impl<T, UT> LiftReturn<UT> for Arc<T> where Arc<T>: Lift<UT>, T: ?Sized);
 derive_ffi_traits!(impl<T, UT> LiftRef<UT> for Arc<T> where Arc<T>: Lift<UT>, T: ?Sized);
+derive_ffi_traits!(impl<T, UT> TypeId<UT> for Arc<T> where Arc<T>: FfiConverter<UT>, T: ?Sized);
 
 // Implement LowerReturn/LiftReturn for the unit type (void returns)
 
@@ -451,13 +452,13 @@ unsafe impl<UT> LowerReturn<UT> for () {
     fn lower_return(_: ()) -> Result<Self::ReturnType, RustBuffer> {
         Ok(())
     }
-
-    const TYPE_ID_META: MetadataBuffer = MetadataBuffer::from_code(metadata::codes::TYPE_UNIT);
 }
 
 unsafe impl<UT> LiftReturn<UT> for () {
     fn lift_callback_return(_buf: RustBuffer) -> Self {}
+}
 
+impl<UT> TypeId<UT> for () {
     const TYPE_ID_META: MetadataBuffer = MetadataBuffer::from_code(metadata::codes::TYPE_UNIT);
 }
 
@@ -484,10 +485,6 @@ where
             Err(ohno) => panic!("Failed to convert arg '{arg_name}': {ohno}"),
         }
     }
-
-    const TYPE_ID_META: MetadataBuffer = MetadataBuffer::from_code(metadata::codes::TYPE_RESULT)
-        .concat(R::TYPE_ID_META)
-        .concat(E::TYPE_ID_META);
 }
 
 unsafe impl<UT, R, E> LiftReturn<UT> for Result<R, E>
@@ -513,7 +510,13 @@ where
     fn handle_callback_unexpected_error(e: UnexpectedUniFFICallbackError) -> Self {
         Err(E::try_convert_unexpected_callback_error(e).unwrap_or_else(|e| panic!("{e}")))
     }
+}
 
+impl<UT, R, E> TypeId<UT> for Result<R, E>
+where
+    R: TypeId<UT>,
+    E: TypeId<UT>,
+{
     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/ffi_converter_traits.rs b/uniffi_core/src/ffi_converter_traits.rs
@@ -9,7 +9,7 @@
 //! proc-macro generated code.  The goal is to allow the proc-macros to go from a type name to the
 //! correct function for a given FFI operation.
 //!
-//! The traits form a sort-of tree structure from general to specific:
+//! The main traits form a sort-of tree structure from general to specific:
 //! ```ignore
 //!
 //!                   [FfiConverter]
@@ -23,6 +23,10 @@
 //!       [LowerReturn]           [LiftRef]  [LiftReturn]
 //! ```
 //!
+//! There's also:
+//!   - [TypeId], which is implemented for all types that implement any of the above traits.
+//!   - [ConvertError], which is implement for errors that can be used in callback interfaces.
+//!
 //! The `derive_ffi_traits` macro can be used to derive the specific traits from the general ones.
 //! Here's the main ways we implement these traits:
 //!
@@ -39,7 +43,7 @@
 //!
 //! ## Safety
 //!
-//! All traits are unsafe (implementing it requires `unsafe impl`) because we can't guarantee
+//! Most traits are unsafe (implementing it requires `unsafe impl`) because we can't guarantee
 //! that it's safe to pass your type out to foreign-language code and back again. Buggy
 //! implementations of this trait might violate some assumptions made by the generated code,
 //! or might not match with the corresponding code in the generated foreign-language bindings.
@@ -125,8 +129,6 @@ pub unsafe trait FfiConverter<UT>: Sized {
     fn try_read(buf: &mut &[u8]) -> Result<Self>;
 
     /// Type ID metadata, serialized into a [MetadataBuffer].
-    ///
-    /// If a type implements multiple FFI traits, `TYPE_ID_META` must be the same for all of them.
     const TYPE_ID_META: MetadataBuffer;
 }
 
@@ -214,8 +216,6 @@ pub unsafe trait Lift<UT>: Sized {
             n => bail!("junk data left in buffer after lifting (count: {n})",),
         }
     }
-
-    const TYPE_ID_META: MetadataBuffer;
 }
 
 /// Lower Rust values to pass them to the foreign code
@@ -246,8 +246,6 @@ pub unsafe trait Lower<UT>: Sized {
         Self::write(obj, &mut buf);
         RustBuffer::from_vec(buf)
     }
-
-    const TYPE_ID_META: MetadataBuffer;
 }
 
 /// Return Rust values to the foreign code
@@ -285,8 +283,6 @@ pub unsafe trait LowerReturn<UT>: Sized {
     fn handle_failed_lift(arg_name: &str, e: anyhow::Error) -> Self {
         panic!("Failed to convert arg '{arg_name}': {e}")
     }
-
-    const TYPE_ID_META: MetadataBuffer;
 }
 
 /// Return foreign values to Rust
@@ -325,8 +321,6 @@ pub unsafe trait LiftReturn<UT>: Sized {
     fn handle_callback_unexpected_error(e: UnexpectedUniFFICallbackError) -> Self {
         panic!("Callback interface failure: {e}")
     }
-
-    const TYPE_ID_META: MetadataBuffer;
 }
 
 /// Lift references
@@ -347,6 +341,14 @@ pub unsafe trait LiftRef<UT> {
     type LiftType: Lift<UT> + Borrow<Self>;
 }
 
+/// Type ID metadata
+///
+/// This is used to build up more complex metadata.  For example, the `MetadataBuffer` for a
+/// nction signatures includes a copy of this metadata for each argument and return type.
+pub trait TypeId<UT> {
+    const TYPE_ID_META: MetadataBuffer;
+}
+
 pub trait ConvertError<UT>: Sized {
     fn try_convert_unexpected_callback_error(e: UnexpectedUniFFICallbackError) -> Result<Self>;
 }
@@ -378,6 +380,7 @@ macro_rules! derive_ffi_traits {
         $crate::derive_ffi_traits!(impl<UT> LiftReturn<UT> for $ty);
         $crate::derive_ffi_traits!(impl<UT> LiftRef<UT> for $ty);
         $crate::derive_ffi_traits!(impl<UT> ConvertError<UT> for $ty);
+        $crate::derive_ffi_traits!(impl<UT> TypeId<UT> for $ty);
     };
 
     (local $ty:ty) => {
@@ -387,6 +390,7 @@ macro_rules! derive_ffi_traits {
         $crate::derive_ffi_traits!(impl LiftReturn<crate::UniFfiTag> for $ty);
         $crate::derive_ffi_traits!(impl LiftRef<crate::UniFfiTag> for $ty);
         $crate::derive_ffi_traits!(impl ConvertError<crate::UniFfiTag> for $ty);
+        $crate::derive_ffi_traits!(impl TypeId<crate::UniFfiTag> for $ty);
     };
 
     (impl $(<$($generic:ident),*>)? $(::uniffi::)? Lower<$ut:path> for $ty:ty $(where $($where:tt)*)?) => {
@@ -401,8 +405,6 @@ macro_rules! derive_ffi_traits {
             fn write(obj: Self, buf: &mut ::std::vec::Vec<u8>) {
                 <Self as $crate::FfiConverter<$ut>>::write(obj, buf)
             }
-
-            const TYPE_ID_META: $crate::MetadataBuffer = <Self as $crate::FfiConverter<$ut>>::TYPE_ID_META;
         }
     };
 
@@ -418,8 +420,6 @@ macro_rules! derive_ffi_traits {
             fn try_read(buf: &mut &[u8]) -> $crate::deps::anyhow::Result<Self> {
                 <Self as $crate::FfiConverter<$ut>>::try_read(buf)
             }
-
-            const TYPE_ID_META: $crate::MetadataBuffer = <Self as $crate::FfiConverter<$ut>>::TYPE_ID_META;
         }
     };
 
@@ -431,8 +431,6 @@ macro_rules! derive_ffi_traits {
             fn lower_return(obj: Self) -> $crate::deps::anyhow::Result<Self::ReturnType, $crate::RustBuffer> {
                 Ok(<Self as $crate::Lower<$ut>>::lower(obj))
             }
-
-            const TYPE_ID_META: $crate::MetadataBuffer =<Self as $crate::Lower<$ut>>::TYPE_ID_META;
         }
     };
 
@@ -443,8 +441,6 @@ macro_rules! derive_ffi_traits {
                 <Self as $crate::Lift<$ut>>::try_lift_from_rust_buffer(buf)
                     .expect("Error reading callback interface result")
             }
-
-            const TYPE_ID_META: $crate::MetadataBuffer = <Self as $crate::Lift<$ut>>::TYPE_ID_META;
         }
     };
 
@@ -463,4 +459,11 @@ macro_rules! derive_ffi_traits {
             }
         }
     };
+
+    (impl $(<$($generic:ident),*>)? $(::uniffi::)? TypeId<$ut:path> for $ty:ty $(where $($where:tt)*)?) => {
+        impl $(<$($generic),*>)* $crate::TypeId<$ut> for $ty $(where $($where)*)*
+        {
+            const TYPE_ID_META: $crate::MetadataBuffer = <Self as $crate::FfiConverter<$ut>>::TYPE_ID_META;
+        }
+    };
 }
diff --git a/uniffi_core/src/lib.rs b/uniffi_core/src/lib.rs
@@ -46,6 +46,7 @@ pub mod metadata;
 pub use ffi::*;
 pub use ffi_converter_traits::{
     ConvertError, FfiConverter, FfiConverterArc, Lift, LiftRef, LiftReturn, Lower, LowerReturn,
+    TypeId,
 };
 pub use metadata::*;
 

diff --git a/uniffi_macros/src/custom.rs b/uniffi_macros/src/custom.rs
@@ -45,7 +45,7 @@ pub(crate) fn expand_ffi_converter_custom_type(
             const TYPE_ID_META: ::uniffi::MetadataBuffer = ::uniffi::MetadataBuffer::from_code(::uniffi::metadata::codes::TYPE_CUSTOM)
                 .concat_str(#mod_path)
                 .concat_str(#name)
-                .concat(<#builtin as ::uniffi::Lower<crate::UniFfiTag>>::TYPE_ID_META);
+                .concat(<#builtin as ::uniffi::TypeId<crate::UniFfiTag>>::TYPE_ID_META);
         }
 
         #derive_ffi_traits

diff --git a/uniffi_macros/src/enum_.rs b/uniffi_macros/src/enum_.rs
@@ -210,7 +210,7 @@ pub(crate) fn enum_meta_static_var(
     };
     metadata_expr.extend(match discr_type {
         None => quote! { .concat_bool(false) },
-        Some(t) => quote! { .concat_bool(true).concat(<#t as ::uniffi::Lower<crate::UniFfiTag>>::TYPE_ID_META) }
+        Some(t) => quote! { .concat_bool(true).concat(<#t as ::uniffi::TypeId<crate::UniFfiTag>>::TYPE_ID_META) }
     });
     metadata_expr.extend(variant_metadata(enum_)?);
     metadata_expr.extend(quote! {
@@ -287,7 +287,7 @@ pub fn variant_metadata(enum_: &DataEnum) -> syn::Result<Vec<TokenStream>> {
                         .concat_value(#fields_len)
                             #(
                                 .concat_str(#field_names)
-                                .concat(<#field_types as ::uniffi::Lower<crate::UniFfiTag>>::TYPE_ID_META)
+                                .concat(<#field_types as ::uniffi::TypeId<crate::UniFfiTag>>::TYPE_ID_META)
                                 // field defaults not yet supported for enums
                                 .concat_bool(false)
                                 .concat_long_str(#field_docstrings)