Merge pull request #1479 from matrix-org/jplatte/record-field-ffi-tag

Improve UniFFI tag handling in macros

fixtures/uitests/tests/ui/fieldless_errors_used_in_callbacks_cant_have_fields.stderr

@@ -1,7 +1,11 @@
 error[E0533]: expected unit struct, unit variant or constant, found struct variant `Self::DivisionByZero`
  --> $OUT_DIR[uniffi_uitests]/errors.uniffi.rs
   |
-  | #[::uniffi::ffi_converter_error(crate::UniFfiTag)]
-  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+  | / #[::uniffi::ffi_converter_error(
+  | |     tag = crate::UniFfiTag,
+  | |     flat_error,
+  | |     with_try_read,
+  | | )]
+  | |__^
   |
   = note: this error originates in the attribute macro `::uniffi::ffi_converter_error` (in Nightly builds, run with -Z macro-backtrace for more info)

fixtures/uitests/tests/ui/interface_cannot_use_mut_self.stderr

@@ -1,8 +1,8 @@
 error[E0308]: mismatched types
-   --> $OUT_DIR[uniffi_uitests]/counter.uniffi.rs
-    |
-    |             Ok(ref val) => val,
-    |                            ^^^ types differ in mutability
-    |
-    = note: expected mutable reference `&mut Counter`
-                       found reference `&Arc<Counter>`
+ --> $OUT_DIR[uniffi_uitests]/counter.uniffi.rs
+  |
+  |             Ok(ref val) => val,
+  |                            ^^^ types differ in mutability
+  |
+  = note: expected mutable reference `&mut Counter`
+                     found reference `&Arc<Counter>`

fixtures/uitests/tests/ui/interface_not_sync_and_send.stderr

@@ -1,8 +1,8 @@
 error[E0277]: `Cell<u32>` cannot be shared between threads safely
  --> $OUT_DIR[uniffi_uitests]/counter.uniffi.rs
   |
-  | #[::uniffi::ffi_converter_interface(crate::UniFfiTag)]
-  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Cell<u32>` cannot be shared between threads safely
+  | #[::uniffi::ffi_converter_interface(tag = crate::UniFfiTag)]
+  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `Cell<u32>` cannot be shared between threads safely
   |
   = help: within `Counter`, the trait `Sync` is not implemented for `Cell<u32>`
 note: required because it appears within the type `Counter`

uniffi_bindgen/src/scaffolding/templates/EnumTemplate.rs

@@ -7,7 +7,7 @@
 // public so other crates can refer to it via an `[External='crate'] typedef`
 #}
 
-#[::uniffi::ffi_converter_enum(crate::UniFfiTag)]
+#[::uniffi::ffi_converter_enum(tag = crate::UniFfiTag)]
 enum r#{{ e.name() }} {
     {%- for variant in e.variants() %}
     r#{{ variant.name() }} {

uniffi_bindgen/src/scaffolding/templates/ErrorTemplate.rs

@@ -7,10 +7,15 @@
 // public so other crates can refer to it via an `[External='crate'] typedef`
 #}
 
-#[::uniffi::ffi_converter_error(crate::UniFfiTag)]
-{%- if e.is_flat() %}
-#[uniffi(flat_error{% if ci.should_generate_error_read(e) %},with_try_read{% endif %})]
-{%- endif %}
+#[::uniffi::ffi_converter_error(
+    tag = crate::UniFfiTag,
+    {% if e.is_flat() -%}
+    flat_error,
+    {% if ci.should_generate_error_read(e) -%}
+    with_try_read,
+    {%- endif %}
+    {%- endif %}
+)]
 enum r#{{ e.name() }} {
     {%- for variant in e.variants() %}
     r#{{ variant.name() }} {

uniffi_bindgen/src/scaffolding/templates/ObjectTemplate.rs

@@ -24,7 +24,7 @@
 fn uniffi_note_threadsafe_deprecation_{{ obj.name() }}() {}
 {% endif %}
 
-#[::uniffi::ffi_converter_interface(crate::UniFfiTag)]
+#[::uniffi::ffi_converter_interface(tag = crate::UniFfiTag)]
 struct r#{{ obj.name() }} { }
 
 // All Object structs must be `Sync + Send`. The generated scaffolding will fail to compile

uniffi_bindgen/src/scaffolding/templates/RecordTemplate.rs

@@ -8,7 +8,7 @@
 // public so other crates can refer to it via an `[External='crate'] typedef`
 #}
 
-#[::uniffi::ffi_converter_record(crate::UniFfiTag)]
+#[::uniffi::ffi_converter_record(tag = crate::UniFfiTag)]
 struct r#{{ rec.name() }} {
     {%- for field in rec.fields() %}
     r#{{ field.name() }}: {{ field.type_()|type_rs }},

uniffi_macros/src/enum_.rs

@@ -1,29 +1,32 @@
 use proc_macro2::{Ident, Span, TokenStream};
 use quote::quote;
 use syn::{
-    punctuated::Punctuated, AttributeArgs, Data, DataEnum, DeriveInput, Field, Index, Path, Token,
-    Variant,
+    punctuated::Punctuated, Data, DataEnum, DeriveInput, Field, Index, Path, Token, Variant,
 };
 use uniffi_meta::{EnumMetadata, FieldMetadata, VariantMetadata};
 
 use crate::{
     export::metadata::convert::convert_type,
-    util::{assert_type_eq, create_metadata_static_var, try_read_field, FfiConverterTagHandler},
+    util::{
+        assert_type_eq, create_metadata_static_var, tagged_impl_header, try_read_field,
+        AttributeSliceExt, CommonAttr,
+    },
 };
 
-pub fn expand_enum(input: DeriveInput, module_path: Vec<String>) -> TokenStream {
+pub fn expand_enum(input: DeriveInput, module_path: Vec<String>) -> syn::Result<TokenStream> {
     let enum_ = match input.data {
         Data::Enum(e) => e,
         _ => {
-            return syn::Error::new(Span::call_site(), "This derive must only be used on enums")
-                .into_compile_error()
+            return Err(syn::Error::new(
+                Span::call_site(),
+                "This derive must only be used on enums",
+            ));
         }
     };
 
     let ident = &input.ident;
-
-    let ffi_converter_impl =
-        enum_ffi_converter_impl(ident, &enum_, FfiConverterTagHandler::generic_impl());
+    let attr = input.attrs.parse_uniffi_attributes::<CommonAttr>()?;
+    let ffi_converter_impl = enum_ffi_converter_impl(ident, &enum_, attr.tag.as_ref());
 
     let meta_static_var = {
         match enum_metadata(ident, enum_.variants, module_path) {
@@ -34,20 +37,16 @@ pub fn expand_enum(input: DeriveInput, module_path: Vec<String>) -> TokenStream
 
     let type_assertion = assert_type_eq(ident, quote! { crate::uniffi_types::#ident });
 
-    quote! {
+    Ok(quote! {
         #ffi_converter_impl
         #meta_static_var
         #type_assertion
-    }
+    })
 }
 
-pub fn expand_enum_ffi_converter(attrs: AttributeArgs, input: DeriveInput) -> TokenStream {
-    let tag_handler = match FfiConverterTagHandler::try_from(attrs) {
-        Ok(tag_handler) => tag_handler,
-        Err(e) => return e.into_compile_error(),
-    };
+pub(crate) fn expand_enum_ffi_converter(attr: CommonAttr, input: DeriveInput) -> TokenStream {
     match input.data {
-        Data::Enum(e) => enum_ffi_converter_impl(&input.ident, &e, tag_handler),
+        Data::Enum(e) => enum_ffi_converter_impl(&input.ident, &e, attr.tag.as_ref()),
         _ => syn::Error::new(
             proc_macro2::Span::call_site(),
             "This attribute must only be used on enums",
@@ -59,14 +58,14 @@ pub fn expand_enum_ffi_converter(attrs: AttributeArgs, input: DeriveInput) -> To
 pub(crate) fn enum_ffi_converter_impl(
     ident: &Ident,
     enum_: &DataEnum,
-    tag_handler: FfiConverterTagHandler,
+    tag: Option<&Path>,
 ) -> TokenStream {
-    let (impl_spec, tag) = tag_handler.into_impl_and_tag_path("FfiConverter", ident);
+    let impl_spec = tagged_impl_header("FfiConverter", ident, tag);
     let write_match_arms = enum_.variants.iter().enumerate().map(|(i, v)| {
         let v_ident = &v.ident;
         let fields = v.fields.iter().map(|f| &f.ident);
         let idx = Index::from(i + 1);
-        let write_fields = v.fields.iter().map(|f| write_field(f, &tag));
+        let write_fields = v.fields.iter().map(write_field);
 
         quote! {
             Self::#v_ident { #(#fields),* } => {
@@ -82,7 +81,7 @@ pub(crate) fn enum_ffi_converter_impl(
     let try_read_match_arms = enum_.variants.iter().enumerate().map(|(i, v)| {
         let idx = Index::from(i + 1);
         let v_ident = &v.ident;
-        let try_read_fields = v.fields.iter().map(|f| try_read_field(f, &tag));
+        let try_read_fields = v.fields.iter().map(try_read_field);
 
         quote! {
             #idx => Self::#v_ident { #(#try_read_fields)* },
@@ -101,7 +100,7 @@ pub(crate) fn enum_ffi_converter_impl(
     quote! {
         #[automatically_derived]
         unsafe #impl_spec {
-            ::uniffi::ffi_converter_rust_buffer_lift_and_lower!(#tag);
+            ::uniffi::ffi_converter_rust_buffer_lift_and_lower!(crate::UniFfiTag);
 
             fn write(obj: Self, buf: &mut ::std::vec::Vec<u8>) {
                 #write_impl
@@ -161,11 +160,11 @@ fn field_metadata(f: &Field, v: &Variant) -> syn::Result<FieldMetadata> {
     })
 }
 
-fn write_field(f: &Field, uniffi_tag: &Path) -> TokenStream {
+fn write_field(f: &Field) -> TokenStream {
     let ident = &f.ident;
     let ty = &f.ty;
 
     quote! {
-        <#ty as ::uniffi::FfiConverter<#uniffi_tag>>::write(#ident, buf);
+        <#ty as ::uniffi::FfiConverter<crate::UniFfiTag>>::write(#ident, buf);
     }
 }