From 7a24e534d333d176cf8d53ad5a54358ec0d1ddbf Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Tue, 13 Dec 2022 17:46:54 +0100 Subject: [PATCH] proc-macro: Properly support flat errors --- fixtures/proc-macro/src/lib.rs | 20 ++- .../tests/bindings/test_proc_macro.kts | 2 +- .../tests/bindings/test_proc_macro.py | 2 +- .../tests/bindings/test_proc_macro.swift | 2 +- uniffi_bindgen/src/interface/error.rs | 3 +- uniffi_macros/src/enum_.rs | 6 +- uniffi_macros/src/error.rs | 149 +++++++++++++++--- uniffi_macros/src/lib.rs | 2 +- uniffi_macros/src/record.rs | 2 +- uniffi_macros/src/util.rs | 71 ++++++++- uniffi_meta/src/lib.rs | 2 +- 11 files changed, 224 insertions(+), 37 deletions(-) diff --git a/fixtures/proc-macro/src/lib.rs b/fixtures/proc-macro/src/lib.rs index 82468b7301..bdaa9a61ad 100644 --- a/fixtures/proc-macro/src/lib.rs +++ b/fixtures/proc-macro/src/lib.rs @@ -67,7 +67,6 @@ fn enum_identity(value: MaybeBool) -> MaybeBool { pub enum BasicError { InvalidInput, OsError, - ThisIsNotAFlatErrorType { field: u32 }, } #[uniffi::export] @@ -75,11 +74,24 @@ fn always_fails() -> Result<(), BasicError> { Err(BasicError::OsError) } +#[derive(Debug, thiserror::Error, uniffi::Error)] +#[uniffi(flat_error)] +#[non_exhaustive] +pub enum FlatError { + #[error("Invalid input")] + InvalidInput, + + // Inner types that aren't FFI-convertible, as well as unnamed fields, + // are allowed for flat errors + #[error("OS error: {0}")] + OsError(std::io::Error), +} + #[uniffi::export] impl Object { - fn do_stuff(&self, times: u32) -> Result<(), BasicError> { + fn do_stuff(&self, times: u32) -> Result<(), FlatError> { match times { - 0 => Err(BasicError::InvalidInput), + 0 => Err(FlatError::InvalidInput), _ => { // do stuff Ok(()) @@ -91,5 +103,5 @@ impl Object { include!(concat!(env!("OUT_DIR"), "/proc-macro.uniffi.rs")); mod uniffi_types { - pub use crate::{BasicError, MaybeBool, NestedRecord, Object, One, Three, Two}; + pub use crate::{BasicError, FlatError, MaybeBool, NestedRecord, Object, One, Three, Two}; } diff --git a/fixtures/proc-macro/tests/bindings/test_proc_macro.kts b/fixtures/proc-macro/tests/bindings/test_proc_macro.kts index 753c09b4d3..77d84b8d87 100644 --- a/fixtures/proc-macro/tests/bindings/test_proc_macro.kts +++ b/fixtures/proc-macro/tests/bindings/test_proc_macro.kts @@ -29,5 +29,5 @@ obj.doStuff(5u) try { obj.doStuff(0u) throw RuntimeException("doStuff should throw if its argument is 0") -} catch (e: BasicException) { +} catch (e: FlatException) { } diff --git a/fixtures/proc-macro/tests/bindings/test_proc_macro.py b/fixtures/proc-macro/tests/bindings/test_proc_macro.py index 129691cd54..ad5de8a6be 100644 --- a/fixtures/proc-macro/tests/bindings/test_proc_macro.py +++ b/fixtures/proc-macro/tests/bindings/test_proc_macro.py @@ -29,7 +29,7 @@ try: obj.do_stuff(0) -except BasicError.InvalidInput: +except FlatError.InvalidInput: pass else: raise Exception("do_stuff should throw if its argument is 0") diff --git a/fixtures/proc-macro/tests/bindings/test_proc_macro.swift b/fixtures/proc-macro/tests/bindings/test_proc_macro.swift index 955a49f013..8fe4cee4e6 100644 --- a/fixtures/proc-macro/tests/bindings/test_proc_macro.swift +++ b/fixtures/proc-macro/tests/bindings/test_proc_macro.swift @@ -29,5 +29,5 @@ try! obj.doStuff(times: 5) do { try obj.doStuff(times: 0) fatalError("doStuff should throw if its argument is 0") -} catch BasicError.InvalidInput { +} catch FlatError.InvalidInput { } diff --git a/uniffi_bindgen/src/interface/error.rs b/uniffi_bindgen/src/interface/error.rs index f2ff9093d9..f1d134c1d6 100644 --- a/uniffi_bindgen/src/interface/error.rs +++ b/uniffi_bindgen/src/interface/error.rs @@ -137,13 +137,12 @@ impl Error { impl From for Error { fn from(meta: uniffi_meta::ErrorMetadata) -> Self { - let flat = meta.variants.iter().all(|v| v.fields.is_empty()); Self { name: meta.name.clone(), enum_: Enum { name: meta.name, variants: meta.variants.into_iter().map(Into::into).collect(), - flat, + flat: meta.flat, }, } } diff --git a/uniffi_macros/src/enum_.rs b/uniffi_macros/src/enum_.rs index 71ddda83f6..c4e49beb8b 100644 --- a/uniffi_macros/src/enum_.rs +++ b/uniffi_macros/src/enum_.rs @@ -16,7 +16,7 @@ pub fn expand_enum(input: DeriveInput, module_path: Vec) -> TokenStream let ident = &input.ident; - let ffi_converter_impl = enum_ffi_converter_impl(&variants, ident); + let ffi_converter_impl = enum_ffi_converter_impl(variants.as_ref(), ident); let meta_static_var = if let Some(variants) = variants { match enum_metadata(ident, variants, module_path) { @@ -38,7 +38,7 @@ pub fn expand_enum(input: DeriveInput, module_path: Vec) -> TokenStream } pub(crate) fn enum_ffi_converter_impl( - variants: &Option>, + variants: Option<&Punctuated>, ident: &Ident, ) -> TokenStream { let (write_impl, try_read_impl) = match variants { @@ -150,7 +150,7 @@ fn field_metadata(f: &Field, v: &Variant) -> syn::Result { }) } -pub fn write_field(f: &Field) -> TokenStream { +fn write_field(f: &Field) -> TokenStream { let ident = &f.ident; let ty = &f.ty; diff --git a/uniffi_macros/src/error.rs b/uniffi_macros/src/error.rs index 9a8b56c409..76c34cf78d 100644 --- a/uniffi_macros/src/error.rs +++ b/uniffi_macros/src/error.rs @@ -1,37 +1,62 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::quote; -use syn::{punctuated::Punctuated, Data, DeriveInput, Token, Variant}; -use uniffi_meta::ErrorMetadata; +use syn::{ + parse::{Parse, ParseStream}, + punctuated::Punctuated, + Data, DeriveInput, Index, Token, Variant, +}; +use uniffi_meta::{ErrorMetadata, VariantMetadata}; use crate::{ enum_::{enum_ffi_converter_impl, variant_metadata}, - util::{assert_type_eq, create_metadata_static_var}, + util::{ + assert_type_eq, chain, create_metadata_static_var, either_attribute_arg, AttributeSliceExt, + UniffiAttribute, + }, }; pub fn expand_error(input: DeriveInput, module_path: Vec) -> TokenStream { let variants = match input.data { - Data::Enum(e) => Some(e.variants), - _ => None, + Data::Enum(e) => Ok(e.variants), + _ => Err(syn::Error::new( + Span::call_site(), + "This derive currently only supports enums", + )), }; let ident = &input.ident; + let attr = input.attrs.parse_uniffi_attributes::(); + let ffi_converter_impl = match &attr { + Ok(a) if a.flat.is_some() => flat_error_ffi_converter_impl(variants.as_ref().ok(), ident), + _ => enum_ffi_converter_impl(variants.as_ref().ok(), ident), + }; - let ffi_converter_impl = enum_ffi_converter_impl(&variants, ident); - - let meta_static_var = if let Some(variants) = variants { - match error_metadata(ident, variants, module_path) { + let meta_static_var = match (&variants, &attr) { + (Ok(vs), Ok(a)) => Some(match error_metadata(ident, vs, module_path, a) { Ok(metadata) => create_metadata_static_var(ident, metadata.into()), Err(e) => e.into_compile_error(), - } - } else { - syn::Error::new( - Span::call_site(), - "This derive currently only supports enums", - ) - .into_compile_error() + }), + _ => None, }; let type_assertion = assert_type_eq(ident, quote! { crate::uniffi_types::#ident }); + let variant_errors: TokenStream = match variants { + Ok(vs) => vs + .iter() + .flat_map(|variant| { + chain( + variant.attrs.attributes_not_allowed_here(), + variant + .fields + .iter() + .flat_map(|field| field.attrs.attributes_not_allowed_here()), + ) + }) + .map(syn::Error::into_compile_error) + .collect(), + Err(e) => e.into_compile_error(), + }; + let attr_error = attr.err().map(syn::Error::into_compile_error); quote! { #ffi_converter_impl @@ -41,23 +66,105 @@ pub fn expand_error(input: DeriveInput, module_path: Vec) -> TokenStream #meta_static_var #type_assertion + #variant_errors + #attr_error + } +} + +pub(crate) fn flat_error_ffi_converter_impl( + variants: Option<&Punctuated>, + ident: &Ident, +) -> TokenStream { + let write_impl = match variants { + Some(variants) => { + let write_match_arms = variants.iter().enumerate().map(|(i, v)| { + let v_ident = &v.ident; + let idx = Index::from(i + 1); + + quote! { + Self::#v_ident { .. } => { + ::uniffi::deps::bytes::BufMut::put_i32(buf, #idx); + <::std::string::String as ::uniffi::FfiConverter>::write(error_msg, buf); + } + } + }); + let write_impl = quote! { + let error_msg = ::std::string::ToString::to_string(&obj); + match obj { #(#write_match_arms)* } + }; + + write_impl + } + None => quote! { ::std::unimplemented!() }, + }; + + quote! { + #[automatically_derived] + impl ::uniffi::RustBufferFfiConverter for #ident { + type RustType = Self; + + fn write(obj: Self, buf: &mut ::std::vec::Vec) { + #write_impl + } + + fn try_read(buf: &mut &[::std::primitive::u8]) -> ::uniffi::deps::anyhow::Result { + ::std::panic!("try_read not supported for flat errors"); + } + } } } fn error_metadata( ident: &Ident, - variants: Punctuated, + variants: &Punctuated, module_path: Vec, + attr: &ErrorAttr, ) -> syn::Result { let name = ident.to_string(); - let variants = variants - .iter() - .map(variant_metadata) - .collect::>()?; + let flat = attr.flat.is_some(); + let variants = if flat { + variants + .iter() + .map(|v| VariantMetadata { + name: v.ident.to_string(), + fields: vec![], + }) + .collect() + } else { + variants + .iter() + .map(variant_metadata) + .collect::>()? + }; Ok(ErrorMetadata { module_path, name, variants, + flat, }) } + +mod kw { + syn::custom_keyword!(flat_error); +} + +#[derive(Default)] +struct ErrorAttr { + flat: Option, +} + +impl Parse for ErrorAttr { + fn parse(input: ParseStream<'_>) -> syn::Result { + let flat = input.parse()?; + Ok(ErrorAttr { flat }) + } +} + +impl UniffiAttribute for ErrorAttr { + fn merge(self, other: Self) -> syn::Result { + Ok(Self { + flat: either_attribute_arg(self.flat, other.flat)?, + }) + } +} diff --git a/uniffi_macros/src/lib.rs b/uniffi_macros/src/lib.rs index 6be03f30d2..a38705e57d 100644 --- a/uniffi_macros/src/lib.rs +++ b/uniffi_macros/src/lib.rs @@ -105,7 +105,7 @@ pub fn derive_object(input: TokenStream) -> TokenStream { expand_object(input, mod_path).into() } -#[proc_macro_derive(Error)] +#[proc_macro_derive(Error, attributes(uniffi))] pub fn derive_error(input: TokenStream) -> TokenStream { let mod_path = match util::mod_path() { Ok(p) => p, diff --git a/uniffi_macros/src/record.rs b/uniffi_macros/src/record.rs index 1a958ac431..60d0962503 100644 --- a/uniffi_macros/src/record.rs +++ b/uniffi_macros/src/record.rs @@ -98,7 +98,7 @@ fn field_metadata(f: &Field) -> syn::Result { }) } -pub fn write_field(f: &Field) -> TokenStream { +fn write_field(f: &Field) -> TokenStream { let ident = &f.ident; let ty = &f.ty; diff --git a/uniffi_macros/src/util.rs b/uniffi_macros/src/util.rs index 242e63e8bf..e8e03b3b5d 100644 --- a/uniffi_macros/src/util.rs +++ b/uniffi_macros/src/util.rs @@ -4,7 +4,13 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::{format_ident, quote, quote_spanned, ToTokens}; -use syn::{spanned::Spanned, visit_mut::VisitMut, Item, Type}; +use syn::{ + parse::{Parse, ParseStream}, + punctuated::Punctuated, + spanned::Spanned, + visit_mut::VisitMut, + Attribute, Item, Token, Type, +}; use uniffi_meta::Metadata; #[cfg(not(feature = "nightly"))] @@ -152,3 +158,66 @@ pub fn assert_type_eq(a: impl ToTokens + Spanned, b: impl ToTokens) -> TokenStre }; } } + +pub fn chain( + a: impl IntoIterator, + b: impl IntoIterator, +) -> impl Iterator { + a.into_iter().chain(b) +} + +pub trait UniffiAttribute: Default + Parse { + fn merge(self, other: Self) -> syn::Result; +} + +#[derive(Default)] +struct AttributeNotAllowedHere; + +impl Parse for AttributeNotAllowedHere { + fn parse(input: ParseStream<'_>) -> syn::Result { + Err(syn::Error::new( + input.span(), + "UniFFI attributes are not currently recognized in this position", + )) + } +} + +impl UniffiAttribute for AttributeNotAllowedHere { + fn merge(self, _other: Self) -> syn::Result { + Ok(Self) + } +} + +pub trait AttributeSliceExt { + fn parse_uniffi_attributes(&self) -> syn::Result; + fn attributes_not_allowed_here(&self) -> Option; +} + +impl AttributeSliceExt for [Attribute] { + fn parse_uniffi_attributes(&self) -> syn::Result { + self.iter() + .filter(|attr| attr.path.is_ident("uniffi")) + .try_fold(T::default(), |res, attr| { + let list: Punctuated = + attr.parse_args_with(Punctuated::parse_terminated)?; + list.into_iter().try_fold(res, T::merge) + }) + } + + fn attributes_not_allowed_here(&self) -> Option { + self.parse_uniffi_attributes::() + .err() + } +} + +pub fn either_attribute_arg(a: Option, b: Option) -> syn::Result> { + match (a, b) { + (None, None) => Ok(None), + (Some(val), None) | (None, Some(val)) => Ok(Some(val)), + (Some(a), Some(b)) => { + let mut error = syn::Error::new_spanned(a, "redundant attribute argument"); + error.combine(syn::Error::new_spanned(b, "note: first one here")); + Err(error) + } + } +} diff --git a/uniffi_meta/src/lib.rs b/uniffi_meta/src/lib.rs index 06ba0ced05..228e1e8a0c 100644 --- a/uniffi_meta/src/lib.rs +++ b/uniffi_meta/src/lib.rs @@ -123,12 +123,12 @@ impl ObjectMetadata { } } -/// Currently a copy of `EnumMetadata`, but likely to be changed in the future. #[derive(Clone, Debug, Hash, Deserialize, Serialize)] pub struct ErrorMetadata { pub module_path: Vec, pub name: String, pub variants: Vec, + pub flat: bool, } /// Returns the last 16 bits of the value's hash as computed with [`DefaultHasher`].