From dd881d909b3612fbcd23e5c104559c858cfeec2f Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Wed, 23 Nov 2022 15:28:03 +0100 Subject: [PATCH 01/11] proc-macro: Add automatically_derived attribute to generated impls --- uniffi_macros/src/enum_.rs | 1 + uniffi_macros/src/record.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/uniffi_macros/src/enum_.rs b/uniffi_macros/src/enum_.rs index ad4a965458..fa5fbdc672 100644 --- a/uniffi_macros/src/enum_.rs +++ b/uniffi_macros/src/enum_.rs @@ -75,6 +75,7 @@ pub fn expand_enum(input: DeriveInput, module_path: Vec) -> TokenStream let type_assertion = assert_type_eq(ident, quote! { crate::uniffi_types::#ident }); quote! { + #[automatically_derived] impl ::uniffi::RustBufferFfiConverter for #ident { type RustType = Self; diff --git a/uniffi_macros/src/record.rs b/uniffi_macros/src/record.rs index d134d4ca5f..1a958ac431 100644 --- a/uniffi_macros/src/record.rs +++ b/uniffi_macros/src/record.rs @@ -43,6 +43,7 @@ pub fn expand_record(input: DeriveInput, module_path: Vec) -> TokenStrea let type_assertion = assert_type_eq(ident, quote! { crate::uniffi_types::#ident }); quote! { + #[automatically_derived] impl ::uniffi::RustBufferFfiConverter for #ident { type RustType = Self; From c170733713f3359c25389713061840c9d9bf6088 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 26 Sep 2022 18:14:42 +0200 Subject: [PATCH 02/11] proc-macro: Use Signature instead of ImplItemMethod in export::Method --- uniffi_macros/src/export.rs | 10 +++++----- uniffi_macros/src/export/metadata/impl_.rs | 16 ++++++++-------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/uniffi_macros/src/export.rs b/uniffi_macros/src/export.rs index 0910f1f602..dc4470f869 100644 --- a/uniffi_macros/src/export.rs +++ b/uniffi_macros/src/export.rs @@ -33,7 +33,7 @@ pub enum ExportItem { } pub struct Method { - item: syn::ImplItemMethod, + sig: syn::Signature, metadata: MethodMetadata, } @@ -60,13 +60,13 @@ pub fn expand_export(metadata: ExportItem, mod_path: &[String]) -> TokenStream { .map(|res| { res.map_or_else( syn::Error::into_compile_error, - |Method { item, metadata }| { + |Method { sig, metadata }| { let checksum = checksum(&metadata); let scaffolding = - gen_method_scaffolding(&item.sig, mod_path, checksum, &self_ident); - let type_assertions = fn_type_assertions(&item.sig); + gen_method_scaffolding(&sig, mod_path, checksum, &self_ident); + let type_assertions = fn_type_assertions(&sig); let meta_static_var = create_metadata_static_var( - &format_ident!("{}_{}", metadata.self_name, item.sig.ident), + &format_ident!("{}_{}", metadata.self_name, sig.ident), metadata.into(), ); diff --git a/uniffi_macros/src/export/metadata/impl_.rs b/uniffi_macros/src/export/metadata/impl_.rs index 33709ba692..85b23d8c10 100644 --- a/uniffi_macros/src/export/metadata/impl_.rs +++ b/uniffi_macros/src/export/metadata/impl_.rs @@ -54,8 +54,8 @@ fn gen_method_metadata( self_name: &str, mod_path: &[String], ) -> syn::Result { - let item = match it { - syn::ImplItem::Method(m) => m, + let sig = match it { + syn::ImplItem::Method(m) => m.sig, _ => { return Err(syn::Error::new_spanned( it, @@ -64,21 +64,21 @@ fn gen_method_metadata( } }; - let metadata = method_metadata(self_name, &item, mod_path)?; + let metadata = method_metadata(self_name, &sig, mod_path)?; - Ok(Method { item, metadata }) + Ok(Method { sig, metadata }) } fn method_metadata( self_name: &str, - f: &syn::ImplItemMethod, + sig: &syn::Signature, mod_path: &[String], ) -> syn::Result { Ok(MethodMetadata { module_path: mod_path.to_owned(), self_name: self_name.to_owned(), - name: f.sig.ident.to_string(), - inputs: fn_param_metadata(&f.sig.inputs)?, - return_type: return_type_metadata(&f.sig.output)?, + name: sig.ident.to_string(), + inputs: fn_param_metadata(&sig.inputs)?, + return_type: return_type_metadata(&sig.output)?, }) } From 4354ca62ec4b538b629c9b56d9146b4b7831bbf0 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Tue, 22 Nov 2022 12:29:22 +0100 Subject: [PATCH 03/11] proc-macro: Update span for associated functions error --- uniffi_macros/src/export/scaffolding.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/uniffi_macros/src/export/scaffolding.rs b/uniffi_macros/src/export/scaffolding.rs index 8d46a3aec5..61195612de 100644 --- a/uniffi_macros/src/export/scaffolding.rs +++ b/uniffi_macros/src/export/scaffolding.rs @@ -68,8 +68,11 @@ pub(super) fn gen_method_scaffolding( } _ => { assoc_fn_error = Some( - syn::Error::new_spanned(sig, "associated functions are not currently supported") - .into_compile_error(), + syn::Error::new_spanned( + &sig.ident, + "associated functions are not currently supported", + ) + .into_compile_error(), ); params_args.extend(collect_params(&sig.inputs, RECEIVER_ERROR)); quote! { #self_ident:: } From fce2614ca1072ff38f409e468887979ab29fc3f8 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Wed, 23 Nov 2022 18:46:25 +0100 Subject: [PATCH 04/11] proc-macro: Permit redundant defition of errors --- uniffi_bindgen/src/interface/error.rs | 8 +++--- uniffi_bindgen/src/interface/mod.rs | 35 ++++++++++++++++------- uniffi_bindgen/src/interface/types/mod.rs | 10 ++++--- 3 files changed, 35 insertions(+), 18 deletions(-) diff --git a/uniffi_bindgen/src/interface/error.rs b/uniffi_bindgen/src/interface/error.rs index ee1fc37e3d..c4101a9a9c 100644 --- a/uniffi_bindgen/src/interface/error.rs +++ b/uniffi_bindgen/src/interface/error.rs @@ -94,7 +94,7 @@ use super::{APIConverter, ComponentInterface}; /// they're handled in the FFI very differently. We create them in `uniffi::call_with_result()` if /// the wrapped function returns an `Err` value /// struct and assign an integer error code to each variant. -#[derive(Debug, Clone, Hash)] +#[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct Error { pub name: String, enum_: Enum, @@ -159,7 +159,7 @@ mod test { enum Testing { "one", "two", "three" }; "#; let ci = ComponentInterface::from_webidl(UDL).unwrap(); - assert_eq!(ci.error_definitions().len(), 1); + assert_eq!(ci.error_definitions().count(), 1); let error = ci.get_error_definition("Testing").unwrap(); assert_eq!( error @@ -182,7 +182,7 @@ mod test { enum Testing { "one", "two", "one" }; "#; let ci = ComponentInterface::from_webidl(UDL).unwrap(); - assert_eq!(ci.error_definitions().len(), 1); + assert_eq!(ci.error_definitions().count(), 1); assert_eq!( ci.get_error_definition("Testing").unwrap().variants().len(), 3 @@ -201,7 +201,7 @@ mod test { }; "#; let ci = ComponentInterface::from_webidl(UDL).unwrap(); - assert_eq!(ci.error_definitions().len(), 1); + assert_eq!(ci.error_definitions().count(), 1); let error: &Error = ci.get_error_definition("Testing").unwrap(); assert_eq!( error diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index a57e7b2eda..657619caa7 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -100,7 +100,7 @@ pub struct ComponentInterface { functions: Vec, objects: Vec, callback_interfaces: Vec, - errors: Vec, + errors: BTreeMap, } impl ComponentInterface { @@ -207,14 +207,13 @@ impl ComponentInterface { } /// Get the definitions for every Error type in the interface. - pub fn error_definitions(&self) -> &[Error] { - &self.errors + pub fn error_definitions(&self) -> impl Iterator { + self.errors.values() } /// Get an Error definition by name, or None if no such Error is defined. pub fn get_error_definition(&self, name: &str) -> Option<&Error> { - // TODO: probably we could store these internally in a HashMap to make this easier? - self.errors.iter().find(|e| e.name == name) + self.errors.get(name) } /// Should we generate read (and lift) functions for errors? @@ -598,9 +597,25 @@ impl ComponentInterface { } /// Called by `APIBuilder` impls to add a newly-parsed error definition to the `ComponentInterface`. - fn add_error_definition(&mut self, defn: Error) { - // Note that there will be no duplicates thanks to the previous type-finding pass. - self.errors.push(defn); + fn add_error_definition(&mut self, defn: Error) -> Result<()> { + match self.errors.entry(defn.name().to_owned()) { + Entry::Vacant(v) => { + v.insert(defn); + } + Entry::Occupied(o) => { + let existing_def = o.get(); + if defn != *existing_def { + bail!( + "Mismatching definition for error `{}`!\n\ + existing definition: {existing_def:#?},\n\ + new definition: {defn:#?}", + defn.name(), + ); + } + } + } + + Ok(()) } /// Resolve unresolved types within proc-macro function / method signatures. @@ -917,7 +932,7 @@ impl APIBuilder for weedle::Definition<'_> { let attrs = attributes::EnumAttributes::try_from(d.attributes.as_ref())?; if attrs.contains_error_attr() { let err = d.convert(ci)?; - ci.add_error_definition(err); + ci.add_error_definition(err)?; } else { let e = d.convert(ci)?; ci.add_enum_definition(e)?; @@ -934,7 +949,7 @@ impl APIBuilder for weedle::Definition<'_> { ci.add_enum_definition(e)?; } else if attrs.contains_error_attr() { let e = d.convert(ci)?; - ci.add_error_definition(e); + ci.add_error_definition(e)?; } else { let obj = d.convert(ci)?; ci.add_object_definition(obj); diff --git a/uniffi_bindgen/src/interface/types/mod.rs b/uniffi_bindgen/src/interface/types/mod.rs index ca25a0f73d..f92bffabdd 100644 --- a/uniffi_bindgen/src/interface/types/mod.rs +++ b/uniffi_bindgen/src/interface/types/mod.rs @@ -239,10 +239,12 @@ impl TypeUniverse { match self.type_definitions.entry(name.to_string()) { Entry::Occupied(o) => { let existing_def = o.get(); - if type_ == *existing_def && matches!(type_, Type::Record(_) | Type::Enum(_)) { - // UDL and proc-macro metadata are allowed to define the same record and enum - // types, if the definitions match (fields and variants are checked in - // add_record_definition and add_enum_definition) + if type_ == *existing_def + && matches!(type_, Type::Record(_) | Type::Enum(_) | Type::Error(_)) + { + // UDL and proc-macro metadata are allowed to define the same record, enum and + // error types, if the definitions match (fields and variants are checked in + // add_{record,enum,error}_definition) Ok(()) } else { bail!( From b4c5f69829271445c4f7fe771df45ed837c3f3ef Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Wed, 23 Nov 2022 18:55:52 +0100 Subject: [PATCH 05/11] proc-macro: Add Error derive macro --- uniffi/src/lib.rs | 2 +- uniffi_bindgen/src/interface/error.rs | 14 ++++++ uniffi_bindgen/src/interface/mod.rs | 2 +- uniffi_bindgen/src/macro_metadata/ci.rs | 14 +++++- uniffi_macros/src/enum_.rs | 44 ++++++++++------- uniffi_macros/src/error.rs | 63 +++++++++++++++++++++++++ uniffi_macros/src/lib.rs | 15 +++++- uniffi_meta/src/lib.rs | 15 ++++++ 8 files changed, 148 insertions(+), 21 deletions(-) create mode 100644 uniffi_macros/src/error.rs diff --git a/uniffi/src/lib.rs b/uniffi/src/lib.rs index ebd41137b0..e4fb11befa 100644 --- a/uniffi/src/lib.rs +++ b/uniffi/src/lib.rs @@ -54,7 +54,7 @@ pub mod deps { pub use static_assertions; } -pub use uniffi_macros::{export, Enum, Object, Record}; +pub use uniffi_macros::{export, Enum, Error, Object, Record}; mod panichook; diff --git a/uniffi_bindgen/src/interface/error.rs b/uniffi_bindgen/src/interface/error.rs index c4101a9a9c..f2ff9093d9 100644 --- a/uniffi_bindgen/src/interface/error.rs +++ b/uniffi_bindgen/src/interface/error.rs @@ -135,6 +135,20 @@ 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, + }, + } + } +} + impl APIConverter for weedle::EnumDefinition<'_> { fn convert(&self, ci: &mut ComponentInterface) -> Result { Ok(Error::from_enum(APIConverter::::convert(self, ci)?)) diff --git a/uniffi_bindgen/src/interface/mod.rs b/uniffi_bindgen/src/interface/mod.rs index 657619caa7..66c3cd8e01 100644 --- a/uniffi_bindgen/src/interface/mod.rs +++ b/uniffi_bindgen/src/interface/mod.rs @@ -597,7 +597,7 @@ impl ComponentInterface { } /// Called by `APIBuilder` impls to add a newly-parsed error definition to the `ComponentInterface`. - fn add_error_definition(&mut self, defn: Error) -> Result<()> { + pub(super) fn add_error_definition(&mut self, defn: Error) -> Result<()> { match self.errors.entry(defn.name().to_owned()) { Entry::Vacant(v) => { v.insert(defn); diff --git a/uniffi_bindgen/src/macro_metadata/ci.rs b/uniffi_bindgen/src/macro_metadata/ci.rs index 7bef4448ad..b6fe94dd20 100644 --- a/uniffi_bindgen/src/macro_metadata/ci.rs +++ b/uniffi_bindgen/src/macro_metadata/ci.rs @@ -2,7 +2,7 @@ * 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 crate::interface::{ComponentInterface, Enum, Record, Type}; +use crate::interface::{ComponentInterface, Enum, Error, Record, Type}; use anyhow::anyhow; use uniffi_meta::Metadata; @@ -40,6 +40,10 @@ pub fn add_to_ci( format!("object `{}`", meta.name), meta.module_path.first().unwrap(), ), + Metadata::Error(meta) => ( + format!("error `{}`", meta.name), + meta.module_path.first().unwrap(), + ), }; let ns = iface.namespace(); @@ -77,6 +81,14 @@ pub fn add_to_ci( Metadata::Object(meta) => { iface.add_object_free_fn(meta); } + Metadata::Error(meta) => { + let ty = Type::Error(meta.name.clone()); + iface.types.add_known_type(&ty)?; + iface.types.add_type_definition(&meta.name, ty)?; + + let error: Error = meta.into(); + iface.add_error_definition(error)?; + } } } diff --git a/uniffi_macros/src/enum_.rs b/uniffi_macros/src/enum_.rs index fa5fbdc672..71ddda83f6 100644 --- a/uniffi_macros/src/enum_.rs +++ b/uniffi_macros/src/enum_.rs @@ -16,7 +16,32 @@ pub fn expand_enum(input: DeriveInput, module_path: Vec) -> TokenStream let ident = &input.ident; - let (write_impl, try_read_impl) = match &variants { + let ffi_converter_impl = enum_ffi_converter_impl(&variants, ident); + + let meta_static_var = if let Some(variants) = variants { + match enum_metadata(ident, variants, module_path) { + Ok(metadata) => create_metadata_static_var(ident, metadata.into()), + Err(e) => e.into_compile_error(), + } + } else { + syn::Error::new(Span::call_site(), "This derive must only be used on enums") + .into_compile_error() + }; + + let type_assertion = assert_type_eq(ident, quote! { crate::uniffi_types::#ident }); + + quote! { + #ffi_converter_impl + #meta_static_var + #type_assertion + } +} + +pub(crate) fn enum_ffi_converter_impl( + variants: &Option>, + ident: &Ident, +) -> TokenStream { + let (write_impl, try_read_impl) = match variants { Some(variants) => { let write_match_arms = variants.iter().enumerate().map(|(i, v)| { let v_ident = &v.ident; @@ -62,18 +87,6 @@ pub fn expand_enum(input: DeriveInput, module_path: Vec) -> TokenStream } }; - let meta_static_var = if let Some(variants) = variants { - match enum_metadata(ident, variants, module_path) { - Ok(metadata) => create_metadata_static_var(ident, metadata.into()), - Err(e) => e.into_compile_error(), - } - } else { - syn::Error::new(Span::call_site(), "This derive must only be used on enums") - .into_compile_error() - }; - - let type_assertion = assert_type_eq(ident, quote! { crate::uniffi_types::#ident }); - quote! { #[automatically_derived] impl ::uniffi::RustBufferFfiConverter for #ident { @@ -87,9 +100,6 @@ pub fn expand_enum(input: DeriveInput, module_path: Vec) -> TokenStream #try_read_impl } } - - #meta_static_var - #type_assertion } } @@ -111,7 +121,7 @@ fn enum_metadata( }) } -fn variant_metadata(v: &Variant) -> syn::Result { +pub(crate) fn variant_metadata(v: &Variant) -> syn::Result { let name = v.ident.to_string(); let fields = v .fields diff --git a/uniffi_macros/src/error.rs b/uniffi_macros/src/error.rs new file mode 100644 index 0000000000..9a8b56c409 --- /dev/null +++ b/uniffi_macros/src/error.rs @@ -0,0 +1,63 @@ +use proc_macro2::{Ident, Span, TokenStream}; +use quote::quote; +use syn::{punctuated::Punctuated, Data, DeriveInput, Token, Variant}; +use uniffi_meta::ErrorMetadata; + +use crate::{ + enum_::{enum_ffi_converter_impl, variant_metadata}, + util::{assert_type_eq, create_metadata_static_var}, +}; + +pub fn expand_error(input: DeriveInput, module_path: Vec) -> TokenStream { + let variants = match input.data { + Data::Enum(e) => Some(e.variants), + _ => None, + }; + + let ident = &input.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) { + 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() + }; + + let type_assertion = assert_type_eq(ident, quote! { crate::uniffi_types::#ident }); + + quote! { + #ffi_converter_impl + + #[automatically_derived] + impl ::uniffi::FfiError for #ident {} + + #meta_static_var + #type_assertion + } +} + +fn error_metadata( + ident: &Ident, + variants: Punctuated, + module_path: Vec, +) -> syn::Result { + let name = ident.to_string(); + let variants = variants + .iter() + .map(variant_metadata) + .collect::>()?; + + Ok(ErrorMetadata { + module_path, + name, + variants, + }) +} diff --git a/uniffi_macros/src/lib.rs b/uniffi_macros/src/lib.rs index dd4ae8604f..6be03f30d2 100644 --- a/uniffi_macros/src/lib.rs +++ b/uniffi_macros/src/lib.rs @@ -15,6 +15,7 @@ use syn::{parse_macro_input, LitStr}; use util::rewrite_self_type; mod enum_; +mod error; mod export; mod object; mod record; @@ -22,7 +23,8 @@ mod test; mod util; use self::{ - enum_::expand_enum, export::expand_export, object::expand_object, record::expand_record, + enum_::expand_enum, error::expand_error, export::expand_export, object::expand_object, + record::expand_record, }; /// A macro to build testcases for a component's generated bindings. @@ -103,6 +105,17 @@ pub fn derive_object(input: TokenStream) -> TokenStream { expand_object(input, mod_path).into() } +#[proc_macro_derive(Error)] +pub fn derive_error(input: TokenStream) -> TokenStream { + let mod_path = match util::mod_path() { + Ok(p) => p, + Err(e) => return e.into_compile_error().into(), + }; + let input = parse_macro_input!(input); + + expand_error(input, mod_path).into() +} + /// A helper macro to include generated component scaffolding. /// /// This is a simple convenience macro to include the UniFFI component diff --git a/uniffi_meta/src/lib.rs b/uniffi_meta/src/lib.rs index 24eefefcf0..6029861ee3 100644 --- a/uniffi_meta/src/lib.rs +++ b/uniffi_meta/src/lib.rs @@ -121,6 +121,14 @@ 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, +} + /// Returns the last 16 bits of the value's hash as computed with [`DefaultHasher`]. /// /// To be used as a checksum of FFI symbols, as a safeguard against different UniFFI versions being @@ -144,6 +152,7 @@ pub enum Metadata { Record(RecordMetadata), Enum(EnumMetadata), Object(ObjectMetadata), + Error(ErrorMetadata), } impl From for Metadata { @@ -175,3 +184,9 @@ impl From for Metadata { Self::Object(v) } } + +impl From for Metadata { + fn from(v: ErrorMetadata) -> Self { + Self::Error(v) + } +} From e2a5c6d87ba0d9a619dc351ae41bbb35bbb80a0d Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 23 Jun 2022 15:04:58 +0200 Subject: [PATCH 06/11] proc-macro: Add support for fallible functions --- uniffi_bindgen/src/interface/attributes.rs | 12 ++++ uniffi_bindgen/src/interface/function.rs | 9 +-- uniffi_bindgen/src/interface/object.rs | 9 +-- uniffi_macros/src/export.rs | 71 +++++++++++++++---- uniffi_macros/src/export/metadata/convert.rs | 68 +++++++++++++----- uniffi_macros/src/export/metadata/function.rs | 24 ++++--- uniffi_macros/src/export/metadata/impl_.rs | 19 +++-- uniffi_macros/src/export/scaffolding.rs | 44 ++++++++---- uniffi_meta/src/lib.rs | 2 + 9 files changed, 192 insertions(+), 66 deletions(-) diff --git a/uniffi_bindgen/src/interface/attributes.rs b/uniffi_bindgen/src/interface/attributes.rs index b0763d930f..d2520e1321 100644 --- a/uniffi_bindgen/src/interface/attributes.rs +++ b/uniffi_bindgen/src/interface/attributes.rs @@ -169,6 +169,12 @@ impl FunctionAttributes { } } +impl FromIterator for FunctionAttributes { + fn from_iter>(iter: T) -> Self { + Self(Vec::from_iter(iter)) + } +} + impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for FunctionAttributes { type Error = anyhow::Error; fn try_from( @@ -346,6 +352,12 @@ impl MethodAttributes { } } +impl FromIterator for MethodAttributes { + fn from_iter>(iter: T) -> Self { + Self(Vec::from_iter(iter)) + } +} + impl TryFrom<&weedle::attribute::ExtendedAttributeList<'_>> for MethodAttributes { type Error = anyhow::Error; fn try_from( diff --git a/uniffi_bindgen/src/interface/function.rs b/uniffi_bindgen/src/interface/function.rs index c15869d0fc..76574fb977 100644 --- a/uniffi_bindgen/src/interface/function.rs +++ b/uniffi_bindgen/src/interface/function.rs @@ -36,14 +36,11 @@ use std::hash::{Hash, Hasher}; use anyhow::{bail, Result}; +use super::attributes::{ArgumentAttributes, Attribute, FunctionAttributes}; use super::ffi::{FfiArgument, FfiFunction}; use super::literal::{convert_default_value, Literal}; use super::types::{Type, TypeIterator}; -use super::{ - attributes::{ArgumentAttributes, FunctionAttributes}, - convert_type, -}; -use super::{APIConverter, ComponentInterface}; +use super::{convert_type, APIConverter, ComponentInterface}; /// Represents a standalone function. /// @@ -137,7 +134,7 @@ impl From for Function { arguments, return_type, ffi_func, - attributes: Default::default(), + attributes: meta.throws.map(Attribute::Throws).into_iter().collect(), } } } diff --git a/uniffi_bindgen/src/interface/object.rs b/uniffi_bindgen/src/interface/object.rs index 38ba4d2f9f..3440483aa6 100644 --- a/uniffi_bindgen/src/interface/object.rs +++ b/uniffi_bindgen/src/interface/object.rs @@ -63,14 +63,11 @@ use std::{collections::HashSet, iter}; use anyhow::{bail, Result}; +use super::attributes::{Attribute, ConstructorAttributes, InterfaceAttributes, MethodAttributes}; use super::ffi::{FfiArgument, FfiFunction, FfiType}; use super::function::Argument; use super::types::{Type, TypeIterator}; -use super::{ - attributes::{ConstructorAttributes, InterfaceAttributes, MethodAttributes}, - convert_type, -}; -use super::{APIConverter, ComponentInterface}; +use super::{convert_type, APIConverter, ComponentInterface}; /// An "object" is an opaque type that can be instantiated and passed around by reference, /// have methods called on it, and so on - basically your classic Object Oriented Programming @@ -445,7 +442,7 @@ impl From for Method { arguments, return_type, ffi_func, - attributes: Default::default(), + attributes: meta.throws.map(Attribute::Throws).into_iter().collect(), } } } diff --git a/uniffi_macros/src/export.rs b/uniffi_macros/src/export.rs index dc4470f869..c31b0fdce6 100644 --- a/uniffi_macros/src/export.rs +++ b/uniffi_macros/src/export.rs @@ -12,18 +12,18 @@ pub(crate) mod metadata; mod scaffolding; pub use self::metadata::gen_metadata; -use self::scaffolding::{gen_fn_scaffolding, gen_method_scaffolding}; -use crate::{ - export::metadata::convert::convert_type, - util::{assert_type_eq, create_metadata_static_var}, +use self::{ + metadata::convert::{convert_type, try_split_result}, + scaffolding::{gen_fn_scaffolding, gen_method_scaffolding}, }; +use crate::util::{assert_type_eq, create_metadata_static_var}; // TODO(jplatte): Ensure no generics, no async, … // TODO(jplatte): Aggregate errors instead of short-circuiting, whereever possible pub enum ExportItem { Function { - sig: Box, + sig: Signature, metadata: FnMetadata, }, Impl { @@ -33,10 +33,48 @@ pub enum ExportItem { } pub struct Method { - sig: syn::Signature, + sig: Signature, metadata: MethodMetadata, } +pub struct Signature { + ident: Ident, + inputs: Vec, + output: Option, +} + +impl Signature { + fn new(item: syn::Signature) -> syn::Result { + let output = match item.output { + syn::ReturnType::Default => None, + syn::ReturnType::Type(_, ty) => Some(FunctionReturn::new(ty)?), + }; + + Ok(Self { + ident: item.ident, + inputs: item.inputs.into_iter().collect(), + output, + }) + } +} + +pub struct FunctionReturn { + ty: Box, + throws: Option, +} + +impl FunctionReturn { + fn new(ty: Box) -> syn::Result { + Ok(match try_split_result(&ty)? { + Some((ok_type, throws)) => FunctionReturn { + ty: Box::new(ok_type.to_owned()), + throws: Some(throws), + }, + None => FunctionReturn { ty, throws: None }, + }) + } +} + pub fn expand_export(metadata: ExportItem, mod_path: &[String]) -> TokenStream { match metadata { ExportItem::Function { sig, metadata } => { @@ -92,7 +130,7 @@ pub fn expand_export(metadata: ExportItem, mod_path: &[String]) -> TokenStream { } } -fn fn_type_assertions(sig: &syn::Signature) -> TokenStream { +fn fn_type_assertions(sig: &Signature) -> TokenStream { // Convert uniffi_meta::Type back to a Rust type fn convert_type_back(ty: &Type) -> TokenStream { match &ty { @@ -143,10 +181,7 @@ fn fn_type_assertions(sig: &syn::Signature) -> TokenStream { _ => Some(&pat_ty.ty), }, }); - let output_type = match &sig.output { - syn::ReturnType::Default => None, - syn::ReturnType::Type(_, ty) => Some(ty), - }; + let output_type = sig.output.as_ref().map(|s| &s.ty); let type_assertions: BTreeMap<_, _> = input_types .chain(output_type) @@ -158,6 +193,18 @@ fn fn_type_assertions(sig: &syn::Signature) -> TokenStream { }) }) .collect(); + let input_output_type_assertions: TokenStream = type_assertions.into_values().collect(); + + let throws_type_assertion = sig.output.as_ref().and_then(|s| { + let ident = s.throws.as_ref()?; + Some(assert_type_eq( + ident, + quote! { crate::uniffi_types::#ident }, + )) + }); - type_assertions.into_values().collect() + quote! { + #input_output_type_assertions + #throws_type_assertion + } } diff --git a/uniffi_macros/src/export/metadata/convert.rs b/uniffi_macros/src/export/metadata/convert.rs index 2d0027b695..6d8d87acc8 100644 --- a/uniffi_macros/src/export/metadata/convert.rs +++ b/uniffi_macros/src/export/metadata/convert.rs @@ -4,12 +4,9 @@ use proc_macro2::Ident; use quote::ToTokens; -use syn::{punctuated::Punctuated, Token}; use uniffi_meta::{FnParamMetadata, Type}; -pub(super) fn fn_param_metadata( - params: &Punctuated, -) -> syn::Result> { +pub(super) fn fn_param_metadata(params: &[syn::FnArg]) -> syn::Result> { params .iter() .filter_map(|a| { @@ -37,13 +34,6 @@ pub(super) fn fn_param_metadata( .collect() } -pub(super) fn return_type_metadata(ty: &syn::ReturnType) -> syn::Result> { - Ok(match ty { - syn::ReturnType::Default => None, - syn::ReturnType::Type(_, ty) => Some(convert_type(ty)?), - }) -} - pub(crate) fn convert_type(ty: &syn::Type) -> syn::Result { let type_path = type_as_type_path(ty)?; @@ -119,11 +109,7 @@ fn convert_generic_type1(ident: &Ident, arg: &syn::GenericArgument) -> syn::Resu let arg = arg_as_type(arg)?; match ident.to_string().as_str() { "Arc" => Ok(Type::ArcObject { - object_name: type_as_type_path(arg)? - .path - .get_ident() - .ok_or_else(|| type_not_supported(arg))? - .to_string(), + object_name: type_as_type_name(arg)?.to_string(), }), "Option" => Ok(Type::Option { inner_type: convert_type(arg)?.into(), @@ -152,6 +138,13 @@ fn convert_generic_type2( } } +fn type_as_type_name(arg: &syn::Type) -> syn::Result<&Ident> { + type_as_type_path(arg)? + .path + .get_ident() + .ok_or_else(|| type_not_supported(arg)) +} + pub(super) fn type_as_type_path(ty: &syn::Type) -> syn::Result<&syn::TypePath> { match ty { syn::Type::Group(g) => type_as_type_path(&g.elem), @@ -177,3 +170,46 @@ fn type_not_supported(ty: &impl ToTokens) -> syn::Error { "this type is not currently supported by uniffi::export in this position", ) } + +pub(crate) fn try_split_result(ty: &syn::Type) -> syn::Result> { + let type_path = type_as_type_path(ty)?; + + if type_path.qself.is_some() { + return Err(syn::Error::new_spanned( + type_path, + "qualified self types are not currently supported by uniffi::export", + )); + } + + if type_path.path.segments.len() > 1 { + return Err(syn::Error::new_spanned( + type_path, + "qualified paths in types are not currently supported by uniffi::export", + )); + } + + let (ident, a) = match &type_path.path.segments.first() { + Some(seg) => match &seg.arguments { + syn::PathArguments::AngleBracketed(a) => (&seg.ident, a), + syn::PathArguments::None | syn::PathArguments::Parenthesized(_) => return Ok(None), + }, + None => return Ok(None), + }; + + let mut it = a.args.iter(); + if let Some(arg1) = it.next() { + if let Some(arg2) = it.next() { + if it.next().is_none() { + let arg1 = arg_as_type(arg1)?; + let arg2 = arg_as_type(arg2)?; + + if let "Result" = ident.to_string().as_str() { + let throws = type_as_type_name(arg2)?.to_owned(); + return Ok(Some((arg1, throws))); + } + } + } + } + + Ok(None) +} diff --git a/uniffi_macros/src/export/metadata/function.rs b/uniffi_macros/src/export/metadata/function.rs index b19e8108c6..9c4edb9869 100644 --- a/uniffi_macros/src/export/metadata/function.rs +++ b/uniffi_macros/src/export/metadata/function.rs @@ -4,23 +4,29 @@ use uniffi_meta::FnMetadata; -use super::convert::{fn_param_metadata, return_type_metadata}; -use crate::export::ExportItem; +use super::convert::{convert_type, fn_param_metadata}; +use crate::export::{ExportItem, Signature}; pub(super) fn gen_fn_metadata(sig: syn::Signature, mod_path: &[String]) -> syn::Result { + let sig = Signature::new(sig)?; let metadata = fn_metadata(&sig, mod_path)?; - - Ok(ExportItem::Function { - sig: Box::new(sig), - metadata, - }) + Ok(ExportItem::Function { sig, metadata }) } -fn fn_metadata(sig: &syn::Signature, mod_path: &[String]) -> syn::Result { +fn fn_metadata(sig: &Signature, mod_path: &[String]) -> syn::Result { + let (return_type, throws) = match &sig.output { + Some(ret) => { + let ty = convert_type(&ret.ty)?; + (Some(ty), ret.throws.as_ref().map(ToString::to_string)) + } + None => (None, None), + }; + Ok(FnMetadata { module_path: mod_path.to_owned(), name: sig.ident.to_string(), inputs: fn_param_metadata(&sig.inputs)?, - return_type: return_type_metadata(&sig.output)?, + return_type, + throws, }) } diff --git a/uniffi_macros/src/export/metadata/impl_.rs b/uniffi_macros/src/export/metadata/impl_.rs index 85b23d8c10..27c43a2bd9 100644 --- a/uniffi_macros/src/export/metadata/impl_.rs +++ b/uniffi_macros/src/export/metadata/impl_.rs @@ -4,8 +4,8 @@ use uniffi_meta::MethodMetadata; -use super::convert::{fn_param_metadata, return_type_metadata, type_as_type_path}; -use crate::export::{ExportItem, Method}; +use super::convert::{convert_type, fn_param_metadata, type_as_type_path}; +use crate::export::{ExportItem, Method, Signature}; pub(super) fn gen_impl_metadata( item: syn::ItemImpl, @@ -55,7 +55,7 @@ fn gen_method_metadata( mod_path: &[String], ) -> syn::Result { let sig = match it { - syn::ImplItem::Method(m) => m.sig, + syn::ImplItem::Method(m) => Signature::new(m.sig)?, _ => { return Err(syn::Error::new_spanned( it, @@ -71,14 +71,23 @@ fn gen_method_metadata( fn method_metadata( self_name: &str, - sig: &syn::Signature, + sig: &Signature, mod_path: &[String], ) -> syn::Result { + let (return_type, throws) = match &sig.output { + Some(ret) => { + let ty = convert_type(&ret.ty)?; + (Some(ty), ret.throws.as_ref().map(ToString::to_string)) + } + None => (None, None), + }; + Ok(MethodMetadata { module_path: mod_path.to_owned(), self_name: self_name.to_owned(), name: sig.ident.to_string(), inputs: fn_param_metadata(&sig.inputs)?, - return_type: return_type_metadata(&sig.output)?, + return_type, + throws, }) } diff --git a/uniffi_macros/src/export/scaffolding.rs b/uniffi_macros/src/export/scaffolding.rs index 61195612de..dcf9ab166f 100644 --- a/uniffi_macros/src/export/scaffolding.rs +++ b/uniffi_macros/src/export/scaffolding.rs @@ -4,7 +4,9 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::{format_ident, quote, ToTokens}; -use syn::{FnArg, Pat, ReturnType, Signature}; +use syn::{FnArg, Pat}; + +use super::{FunctionReturn, Signature}; pub(super) fn gen_fn_scaffolding( sig: &Signature, @@ -31,7 +33,7 @@ pub(super) fn gen_fn_scaffolding( } pub(super) fn gen_method_scaffolding( - sig: &syn::Signature, + sig: &Signature, mod_path: &[String], checksum: u16, self_ident: &Ident, @@ -130,6 +132,8 @@ fn collect_params<'a>( let arg_n = format_ident!("arg{i}"); let param = quote! { #arg_n: <#ty as ::uniffi::FfiConverter>::FfiType }; + // FIXME: With UDL, fallible functions use uniffi::lower_anyhow_error_or_panic instead of + // panicking unconditionally. This seems cleaner though. let panic_fmt = match name { Some(name) => format!("Failed to convert arg '{name}': {{}}"), None => format!("Failed to convert arg #{i}: {{}}"), @@ -145,7 +149,7 @@ fn collect_params<'a>( } fn gen_ffi_function( - sig: &syn::Signature, + sig: &Signature, ffi_ident: Ident, params: &[TokenStream], rust_fn_call: TokenStream, @@ -157,16 +161,34 @@ fn gen_ffi_function( // well as `()` so no different codegen is needed? let (output, return_expr); match &sig.output { - ReturnType::Default => { - output = None; - return_expr = rust_fn_call; - } - ReturnType::Type(_, ty) => { + Some(FunctionReturn { ty, throws }) => { output = Some(quote! { -> <#ty as ::uniffi::FfiConverter>::FfiType }); + + return_expr = if let Some(error_ident) = throws { + quote! { + ::uniffi::call_with_result(call_status, || { + let val = #rust_fn_call.map_err(|e| { + <#error_ident as ::uniffi::FfiConverter>::lower( + ::std::convert::Into::into(e), + ) + })?; + Ok(<#ty as ::uniffi::FfiConverter>::lower(val)) + }) + } + } else { + quote! { + ::uniffi::call_with_output(call_status, || { + <#ty as ::uniffi::FfiConverter>::lower(#rust_fn_call) + }) + } + }; + } + None => { + output = None; return_expr = quote! { - <#ty as ::uniffi::FfiConverter>::lower(#rust_fn_call) + ::uniffi::call_with_output(call_status, || #rust_fn_call) }; } } @@ -179,9 +201,7 @@ fn gen_ffi_function( call_status: &mut ::uniffi::RustCallStatus, ) #output { ::uniffi::deps::log::debug!(#name_s); - ::uniffi::call_with_output(call_status, || { - #return_expr - }) + #return_expr } } } diff --git a/uniffi_meta/src/lib.rs b/uniffi_meta/src/lib.rs index 6029861ee3..06ba0ced05 100644 --- a/uniffi_meta/src/lib.rs +++ b/uniffi_meta/src/lib.rs @@ -15,6 +15,7 @@ pub struct FnMetadata { pub name: String, pub inputs: Vec, pub return_type: Option, + pub throws: Option, } impl FnMetadata { @@ -30,6 +31,7 @@ pub struct MethodMetadata { pub name: String, pub inputs: Vec, pub return_type: Option, + pub throws: Option, } impl MethodMetadata { From de2a45252b5d48a8c78cf2dce693c0b011d50ad2 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 12 Dec 2022 14:00:41 +0100 Subject: [PATCH 07/11] proc-macro: Support explicit unit return types --- uniffi/src/lib.rs | 38 +++++++++++- uniffi_macros/src/export/metadata/convert.rs | 7 +++ uniffi_macros/src/export/metadata/function.rs | 10 +-- uniffi_macros/src/export/metadata/impl_.rs | 10 +-- uniffi_macros/src/export/scaffolding.rs | 62 ++++++++----------- 5 files changed, 80 insertions(+), 47 deletions(-) diff --git a/uniffi/src/lib.rs b/uniffi/src/lib.rs index e4fb11befa..e45b7ae8a0 100644 --- a/uniffi/src/lib.rs +++ b/uniffi/src/lib.rs @@ -211,6 +211,38 @@ pub unsafe trait FfiConverter: Sized { fn try_read(buf: &mut &[u8]) -> Result; } +/// Types that can be returned from exported functions. +/// +/// Blanket-implemented by any FfiConverter with RustType = Self, but additionally implemented by +/// the unit type. +/// +/// This helper trait is currently only used to simplify the code generated by the export macro and +/// is not part of the public interface of the library, hence its documentation is hidden. +#[doc(hidden)] +pub unsafe trait FfiReturn: Sized { + type FfiType; + fn lower(self) -> Self::FfiType; +} + +unsafe impl FfiReturn for T +where + T: FfiConverter, +{ + type FfiType = ::FfiType; + + fn lower(self) -> Self::FfiType { + ::lower(self) + } +} + +unsafe impl FfiReturn for () { + type FfiType = (); + + fn lower(self) -> Self::FfiType { + self + } +} + /// A helper function to ensure we don't read past the end of a buffer. /// /// Rust won't actually let us read past the end of a buffer, but the `Buf` trait does not support @@ -608,7 +640,7 @@ unsafe impl FfiConverter for std::sync::Arc { /// function for other types may lead to undefined behaviour. fn write(obj: Self::RustType, buf: &mut Vec) { static_assertions::const_assert!(std::mem::size_of::<*const std::ffi::c_void>() <= 8); - buf.put_u64(Self::lower(obj) as u64); + buf.put_u64(::lower(obj) as u64); } /// When reading as a field of a complex structure, we receive a "borrow" of the `Arc` @@ -639,7 +671,9 @@ where #[cfg(test)] mod test { - use super::*; + use std::time::{Duration, SystemTime}; + + use super::FfiConverter as _; #[test] fn trybuild_ui_tests() { diff --git a/uniffi_macros/src/export/metadata/convert.rs b/uniffi_macros/src/export/metadata/convert.rs index 6d8d87acc8..c19ae579c2 100644 --- a/uniffi_macros/src/export/metadata/convert.rs +++ b/uniffi_macros/src/export/metadata/convert.rs @@ -34,6 +34,13 @@ pub(super) fn fn_param_metadata(params: &[syn::FnArg]) -> syn::Result syn::Result> { + match ty { + syn::Type::Tuple(tup) if tup.elems.is_empty() => Ok(None), + _ => convert_type(ty).map(Some), + } +} + pub(crate) fn convert_type(ty: &syn::Type) -> syn::Result { let type_path = type_as_type_path(ty)?; diff --git a/uniffi_macros/src/export/metadata/function.rs b/uniffi_macros/src/export/metadata/function.rs index 9c4edb9869..a2cf6b7aa1 100644 --- a/uniffi_macros/src/export/metadata/function.rs +++ b/uniffi_macros/src/export/metadata/function.rs @@ -4,7 +4,7 @@ use uniffi_meta::FnMetadata; -use super::convert::{convert_type, fn_param_metadata}; +use super::convert::{convert_return_type, fn_param_metadata}; use crate::export::{ExportItem, Signature}; pub(super) fn gen_fn_metadata(sig: syn::Signature, mod_path: &[String]) -> syn::Result { @@ -15,10 +15,10 @@ pub(super) fn gen_fn_metadata(sig: syn::Signature, mod_path: &[String]) -> syn:: fn fn_metadata(sig: &Signature, mod_path: &[String]) -> syn::Result { let (return_type, throws) = match &sig.output { - Some(ret) => { - let ty = convert_type(&ret.ty)?; - (Some(ty), ret.throws.as_ref().map(ToString::to_string)) - } + Some(ret) => ( + convert_return_type(&ret.ty)?, + ret.throws.as_ref().map(ToString::to_string), + ), None => (None, None), }; diff --git a/uniffi_macros/src/export/metadata/impl_.rs b/uniffi_macros/src/export/metadata/impl_.rs index 27c43a2bd9..302f0bfa13 100644 --- a/uniffi_macros/src/export/metadata/impl_.rs +++ b/uniffi_macros/src/export/metadata/impl_.rs @@ -4,7 +4,7 @@ use uniffi_meta::MethodMetadata; -use super::convert::{convert_type, fn_param_metadata, type_as_type_path}; +use super::convert::{convert_return_type, fn_param_metadata, type_as_type_path}; use crate::export::{ExportItem, Method, Signature}; pub(super) fn gen_impl_metadata( @@ -75,10 +75,10 @@ fn method_metadata( mod_path: &[String], ) -> syn::Result { let (return_type, throws) = match &sig.output { - Some(ret) => { - let ty = convert_type(&ret.ty)?; - (Some(ty), ret.throws.as_ref().map(ToString::to_string)) - } + Some(ret) => ( + convert_return_type(&ret.ty)?, + ret.throws.as_ref().map(ToString::to_string), + ), None => (None, None), }; diff --git a/uniffi_macros/src/export/scaffolding.rs b/uniffi_macros/src/export/scaffolding.rs index dcf9ab166f..f66e8bf844 100644 --- a/uniffi_macros/src/export/scaffolding.rs +++ b/uniffi_macros/src/export/scaffolding.rs @@ -4,7 +4,7 @@ use proc_macro2::{Ident, Span, TokenStream}; use quote::{format_ident, quote, ToTokens}; -use syn::{FnArg, Pat}; +use syn::{parse_quote, FnArg, Pat}; use super::{FunctionReturn, Signature}; @@ -157,41 +157,33 @@ fn gen_ffi_function( let name = &sig.ident; let name_s = name.to_string(); - // FIXME(jplatte): Use an extra trait implemented for `T: FfiConverter` as - // well as `()` so no different codegen is needed? - let (output, return_expr); - match &sig.output { - Some(FunctionReturn { ty, throws }) => { - output = Some(quote! { - -> <#ty as ::uniffi::FfiConverter>::FfiType - }); - - return_expr = if let Some(error_ident) = throws { - quote! { - ::uniffi::call_with_result(call_status, || { - let val = #rust_fn_call.map_err(|e| { - <#error_ident as ::uniffi::FfiConverter>::lower( - ::std::convert::Into::into(e), - ) - })?; - Ok(<#ty as ::uniffi::FfiConverter>::lower(val)) - }) - } - } else { - quote! { - ::uniffi::call_with_output(call_status, || { - <#ty as ::uniffi::FfiConverter>::lower(#rust_fn_call) - }) - } - }; - } + let unit_slot; + let (ty, throws) = match &sig.output { + Some(FunctionReturn { ty, throws }) => (ty, throws), None => { - output = None; - return_expr = quote! { - ::uniffi::call_with_output(call_status, || #rust_fn_call) - }; + unit_slot = parse_quote! { () }; + (&unit_slot, &None) } - } + }; + + let return_expr = if let Some(error_ident) = throws { + quote! { + ::uniffi::call_with_result(call_status, || { + let val = #rust_fn_call.map_err(|e| { + <#error_ident as ::uniffi::FfiConverter>::lower( + ::std::convert::Into::into(e), + ) + })?; + Ok(<#ty as ::uniffi::FfiReturn>::lower(val)) + }) + } + } else { + quote! { + ::uniffi::call_with_output(call_status, || { + <#ty as ::uniffi::FfiReturn>::lower(#rust_fn_call) + }) + } + }; quote! { #[doc(hidden)] @@ -199,7 +191,7 @@ fn gen_ffi_function( pub extern "C" fn #ffi_ident( #(#params,)* call_status: &mut ::uniffi::RustCallStatus, - ) #output { + ) -> <#ty as ::uniffi::FfiReturn>::FfiType { ::uniffi::deps::log::debug!(#name_s); #return_expr } From 25c22054f394c77604991ce0e14b46fae43a980b Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 12 Dec 2022 14:27:07 +0100 Subject: [PATCH 08/11] python: Fix internal error branch --- uniffi_bindgen/src/bindings/python/templates/Helpers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/uniffi_bindgen/src/bindings/python/templates/Helpers.py b/uniffi_bindgen/src/bindings/python/templates/Helpers.py index 9ecfd25476..fbce501981 100644 --- a/uniffi_bindgen/src/bindings/python/templates/Helpers.py +++ b/uniffi_bindgen/src/bindings/python/templates/Helpers.py @@ -45,7 +45,7 @@ def rust_call_with_error(error_ffi_converter, fn, *args): return result elif call_status.code == RustCallStatus.CALL_ERROR: if error_ffi_converter is None: - call_status.err_buf.contents.free() + call_status.error_buf.free() raise InternalError("rust_call_with_error: CALL_ERROR, but error_ffi_converter is None") else: raise error_ffi_converter.lift(call_status.error_buf) From ff54efadc3a2d386a7605715ad4dd9287f138e35 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 12 Dec 2022 14:31:40 +0100 Subject: [PATCH 09/11] proc-macro: Add tests for fallible functions --- fixtures/proc-macro/src/lib.rs | 27 ++++++++++++++++++- .../tests/bindings/test_proc_macro.kts | 14 ++++++++++ .../tests/bindings/test_proc_macro.py | 16 +++++++++++ .../tests/bindings/test_proc_macro.swift | 14 ++++++++++ 4 files changed, 70 insertions(+), 1 deletion(-) diff --git a/fixtures/proc-macro/src/lib.rs b/fixtures/proc-macro/src/lib.rs index 2419396456..82468b7301 100644 --- a/fixtures/proc-macro/src/lib.rs +++ b/fixtures/proc-macro/src/lib.rs @@ -63,8 +63,33 @@ fn enum_identity(value: MaybeBool) -> MaybeBool { value } +#[derive(uniffi::Error)] +pub enum BasicError { + InvalidInput, + OsError, + ThisIsNotAFlatErrorType { field: u32 }, +} + +#[uniffi::export] +fn always_fails() -> Result<(), BasicError> { + Err(BasicError::OsError) +} + +#[uniffi::export] +impl Object { + fn do_stuff(&self, times: u32) -> Result<(), BasicError> { + match times { + 0 => Err(BasicError::InvalidInput), + _ => { + // do stuff + Ok(()) + } + } + } +} + include!(concat!(env!("OUT_DIR"), "/proc-macro.uniffi.rs")); mod uniffi_types { - pub use crate::{MaybeBool, NestedRecord, Object, One, Three, Two}; + pub use crate::{BasicError, 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 50dffc12df..753c09b4d3 100644 --- a/fixtures/proc-macro/tests/bindings/test_proc_macro.kts +++ b/fixtures/proc-macro/tests/bindings/test_proc_macro.kts @@ -17,3 +17,17 @@ assert(enumIdentity(MaybeBool.TRUE) == MaybeBool.TRUE) // just make sure this works / doesn't crash val three = Three(obj) + +try { + alwaysFails() + throw RuntimeException("alwaysFails should have thrown") +} catch (e: BasicException) { +} + +obj.doStuff(5u) + +try { + obj.doStuff(0u) + throw RuntimeException("doStuff should throw if its argument is 0") +} catch (e: BasicException) { +} diff --git a/fixtures/proc-macro/tests/bindings/test_proc_macro.py b/fixtures/proc-macro/tests/bindings/test_proc_macro.py index 94ed38af72..129691cd54 100644 --- a/fixtures/proc-macro/tests/bindings/test_proc_macro.py +++ b/fixtures/proc-macro/tests/bindings/test_proc_macro.py @@ -17,3 +17,19 @@ # just make sure this works / doesn't crash three = Three(obj) + +try: + always_fails() +except BasicError.OsError: + pass +else: + raise Exception("always_fails should have thrown") + +obj.do_stuff(5) + +try: + obj.do_stuff(0) +except BasicError.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 29ecd54f64..955a49f013 100644 --- a/fixtures/proc-macro/tests/bindings/test_proc_macro.swift +++ b/fixtures/proc-macro/tests/bindings/test_proc_macro.swift @@ -17,3 +17,17 @@ assert(enumIdentity(value: .true) == .true) // just make sure this works / doesn't crash let three = Three(obj: obj) + +do { + try alwaysFails() + fatalError("alwaysFails should have thrown") +} catch BasicError.OsError { +} + +try! obj.doStuff(times: 5) + +do { + try obj.doStuff(times: 0) + fatalError("doStuff should throw if its argument is 0") +} catch BasicError.InvalidInput { +} From 7a24e534d333d176cf8d53ad5a54358ec0d1ddbf Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Tue, 13 Dec 2022 17:46:54 +0100 Subject: [PATCH 10/11] 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`]. From b4f784d24a406ed1d39bc69077a96bb820be4010 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Tue, 29 Nov 2022 10:40:22 +0100 Subject: [PATCH 11/11] proc-macro: Document the Error derive --- docs/manual/src/proc_macro/index.md | 53 +++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/docs/manual/src/proc_macro/index.md b/docs/manual/src/proc_macro/index.md index 67eabdd8fd..2516fa2e8a 100644 --- a/docs/manual/src/proc_macro/index.md +++ b/docs/manual/src/proc_macro/index.md @@ -154,6 +154,59 @@ impl Foo { } ``` +## The `uniffi::Error` derive + +The `Error` derive registers a type as an error and can be used on any enum that the `Enum` derive also accepts. +By default, it exposes any variant fields to the foreign code. +This type can then be used as the `E` in a `Result` return type of an exported function or method. +The generated foreign function for an exported function with a `Result` return type +will have the result's `T` as its return type and throw the error in case the Rust call returns `Err(e)`. + +```rust +#[derive(uniffi::Error)] +pub enum MyError { + MissingInput, + IndexOutOfBounds { + index: u32, + size: u32, + } + Generic { + message: String, + } +} + +#[uniffi::export] +fn do_thing() -> Result<(), MyError> { + // ... +} +``` + +You can also use the helper attribute `#[uniffi(flat_error)]` to expose just the variants but none of the fields. +In this case the error will be serialized using Rust's `ToString` trait +and will be accessible as the only field on each of the variants. +For flat errors your variants can have unnamed fields, +and the types of the fields don't need to implement any special traits. + +```rust +#[derive(uniffi::Error)] +#[uniffi(flat_error)] +pub enum MyApiError { + Http(reqwest::Error), + Json(serde_json::Error), +} + +// ToString is not usually implemented directly, but you get it for free by implementing Display. +// This impl could also be generated by a proc-macro, for example thiserror::Error. +impl std::fmt::Display for MyApiError { + // ... +} + +#[uniffi::export] +fn do_http_request() -> Result<(), MyApiError> { + // ... +} +``` + ## Other limitations In addition to the per-item limitations of the macros presented above, there is also currently a