diff --git a/data_model/src/account.rs b/data_model/src/account.rs index 86b38fbf1cb..75b246e2f52 100644 --- a/data_model/src/account.rs +++ b/data_model/src/account.rs @@ -147,7 +147,6 @@ ffi_item! { IntoSchema, )] #[id(type = "::Id")] - #[allow(clippy::multiple_inherent_impl)] #[display(fmt = "[{id}]")] pub struct NewAccount { /// Identification @@ -184,6 +183,11 @@ impl HasMetadata for NewAccount { } } +#[cfg_attr( + all(feature = "ffi_export", not(feature = "ffi_import")), + iroha_ffi::ffi_export +)] +#[cfg_attr(feature = "ffi_import", iroha_ffi::ffi_import)] impl NewAccount { fn new( id: ::Id, @@ -200,14 +204,7 @@ impl NewAccount { pub(crate) fn id(&self) -> &::Id { &self.id } -} -#[cfg_attr( - all(feature = "ffi_export", not(feature = "ffi_import")), - iroha_ffi::ffi_export -)] -#[cfg_attr(feature = "ffi_import", iroha_ffi::ffi_import)] -impl NewAccount { /// Add [`Metadata`] to the account replacing previously defined #[must_use] pub fn with_metadata(mut self, metadata: Metadata) -> Self { diff --git a/data_model/src/asset.rs b/data_model/src/asset.rs index 32286e0bdd0..17f518ebc9b 100644 --- a/data_model/src/asset.rs +++ b/data_model/src/asset.rs @@ -426,7 +426,6 @@ ffi_item! { )] #[id(type = "::Id")] #[display(fmt = "{id} {mintable}{value_type}")] - #[allow(clippy::multiple_inherent_impl)] pub struct NewAssetDefinition { id: ::Id, value_type: AssetValueType, @@ -457,6 +456,11 @@ impl HasMetadata for NewAssetDefinition { } } +#[cfg_attr( + all(feature = "ffi_export", not(feature = "ffi_import")), + iroha_ffi::ffi_export +)] +#[cfg_attr(feature = "ffi_import", iroha_ffi::ffi_import)] impl NewAssetDefinition { /// Create a [`NewAssetDefinition`], reserved for internal use. fn new(id: ::Id, value_type: AssetValueType) -> Self { @@ -473,14 +477,7 @@ impl NewAssetDefinition { pub(crate) fn id(&self) -> &::Id { &self.id } -} -#[cfg_attr( - all(feature = "ffi_export", not(feature = "ffi_import")), - iroha_ffi::ffi_export -)] -#[cfg_attr(feature = "ffi_import", iroha_ffi::ffi_import)] -impl NewAssetDefinition { /// Set mintability to [`Mintable::Once`] #[inline] #[must_use] diff --git a/data_model/src/domain.rs b/data_model/src/domain.rs index d02f34168e0..92d75713fdd 100644 --- a/data_model/src/domain.rs +++ b/data_model/src/domain.rs @@ -89,7 +89,6 @@ ffi_item! { IntoSchema, )] #[id(type = "::Id")] - #[allow(clippy::multiple_inherent_impl)] #[display(fmt = "[{id}]")] pub struct NewDomain { /// The identification associated with the domain builder. @@ -125,6 +124,11 @@ impl HasMetadata for NewDomain { } } +#[cfg_attr( + all(feature = "ffi_export", not(feature = "ffi_import")), + iroha_ffi::ffi_export +)] +#[cfg_attr(feature = "ffi_import", iroha_ffi::ffi_import)] impl NewDomain { /// Create a [`NewDomain`], reserved for internal use. #[must_use] @@ -140,14 +144,7 @@ impl NewDomain { pub(crate) fn id(&self) -> &::Id { &self.id } -} -#[cfg_attr( - all(feature = "ffi_export", not(feature = "ffi_import")), - iroha_ffi::ffi_export -)] -#[cfg_attr(feature = "ffi_import", iroha_ffi::ffi_import)] -impl NewDomain { /// Add [`logo`](IpfsPath) to the domain replacing previously defined value #[must_use] pub fn with_logo(mut self, logo: IpfsPath) -> Self { diff --git a/data_model/src/metadata.rs b/data_model/src/metadata.rs index 07dd680ae16..fa412800b35 100644 --- a/data_model/src/metadata.rs +++ b/data_model/src/metadata.rs @@ -139,9 +139,7 @@ impl Metadata { pub fn iter(&self) -> impl ExactSizeIterator { self.map.iter() } -} -impl Metadata { /// Get the `Some(&Value)` associated to `key`. Return `None` if not found. #[inline] pub fn get(&self, key: &K) -> Option<&Value> diff --git a/data_model/src/name.rs b/data_model/src/name.rs index 03854c3489e..8053138c7b9 100644 --- a/data_model/src/name.rs +++ b/data_model/src/name.rs @@ -88,6 +88,11 @@ impl AsRef for Name { } } +#[cfg_attr( + all(feature = "ffi_export", not(feature = "ffi_import")), + iroha_ffi::ffi_export +)] +#[cfg_attr(feature = "ffi_import", iroha_ffi::ffi_import)] impl FromStr for Name { type Err = ParseError; @@ -96,48 +101,6 @@ impl FromStr for Name { } } -/// FFI function equivalent of [`Name::from_str`] -/// -/// # Safety -/// -/// All of the given pointers must be valid -#[no_mangle] -#[allow(non_snake_case, unsafe_code)] -#[cfg(all(feature = "ffi_export", not(feature = "ffi_import")))] -pub unsafe extern "C" fn Name__from_str<'itm>( - candidate: <&'itm str as iroha_ffi::TryFromReprC<'itm>>::Source, - out_ptr: <::Target as iroha_ffi::Output>::OutPtr, -) -> iroha_ffi::FfiReturn { - let res = std::panic::catch_unwind(|| { - // False positive - doesn't compile otherwise - #[allow(clippy::let_unit_value)] - let fn_body = || { - let mut store = Default::default(); - let candidate: &str = iroha_ffi::TryFromReprC::try_from_repr_c(candidate, &mut store)?; - let method_res = iroha_ffi::IntoFfi::into_ffi( - // TODO: Implement error handling (https://github.com/hyperledger/iroha/issues/2252) - Name::from_str(candidate).map_err(|_e| iroha_ffi::FfiReturn::ExecutionFail)?, - ); - iroha_ffi::OutPtrOf::write(out_ptr, method_res)?; - Ok(()) - }; - - if let Err(err) = fn_body() { - return err; - } - - iroha_ffi::FfiReturn::Ok - }); - - match res { - Ok(res) => res, - Err(_) => { - // TODO: Implement error handling (https://github.com/hyperledger/iroha/issues/2252) - iroha_ffi::FfiReturn::UnrecoverableError - } - } -} - impl<'de> Deserialize<'de> for Name { fn deserialize(deserializer: D) -> Result where @@ -194,32 +157,4 @@ mod tests { assert!(name.is_err()); } } - - #[test] - #[allow(unsafe_code)] - #[cfg(all(feature = "ffi_export", not(feature = "ffi_import")))] - fn ffi_name_from_str() -> Result<(), ParseError> { - use iroha_ffi::Handle; - let candidate = "Name"; - - unsafe { - let mut name = core::mem::MaybeUninit::new(core::ptr::null_mut()); - - assert_eq!( - iroha_ffi::FfiReturn::Ok, - Name__from_str(candidate.into_ffi(), name.as_mut_ptr()) - ); - - let name = name.assume_init(); - assert_ne!(core::ptr::null_mut(), name); - assert_eq!(Name::from_str(candidate)?, *name); - - assert_eq!( - iroha_ffi::FfiReturn::Ok, - crate::ffi::__drop(Name::ID, name.cast()) - ); - } - - Ok(()) - } } diff --git a/data_model/src/role.rs b/data_model/src/role.rs index 3e43aba0c8d..8c698add8a8 100644 --- a/data_model/src/role.rs +++ b/data_model/src/role.rs @@ -122,7 +122,6 @@ impl Registered for Role { TryFromReprC, IntoSchema, )] -#[allow(clippy::multiple_inherent_impl)] #[id(type = "::Id")] pub struct NewRole { inner: Role, @@ -144,16 +143,6 @@ impl crate::Registrable for NewRole { iroha_ffi::ffi_export )] #[cfg_attr(feature = "ffi_import", iroha_ffi::ffi_import)] -impl NewRole { - /// Add permission to the [`Role`] - #[must_use] - #[inline] - pub fn add_permission(mut self, perm: impl Into) -> Self { - self.inner.permissions.insert(perm.into()); - self - } -} - impl NewRole { /// Constructor #[must_use] @@ -172,6 +161,14 @@ impl NewRole { pub(crate) fn id(&self) -> &::Id { &self.inner.id } + + /// Add permission to the [`Role`] + #[must_use] + #[inline] + pub fn add_permission(mut self, perm: impl Into) -> Self { + self.inner.permissions.insert(perm.into()); + self + } } /// The prelude re-exports most commonly used traits, structs and macros from this module. diff --git a/ffi/derive/src/ffi_fn.rs b/ffi/derive/src/ffi_fn.rs index ad7a36db10a..aee72b2615c 100644 --- a/ffi/derive/src/ffi_fn.rs +++ b/ffi/derive/src/ffi_fn.rs @@ -5,14 +5,8 @@ use syn::Ident; use crate::impl_visitor::{ffi_output_arg, Arg, FnDescriptor}; pub fn gen_declaration(fn_descriptor: &FnDescriptor) -> TokenStream { - let ffi_fn_name = gen_fn_name(fn_descriptor.self_ty_name(), &fn_descriptor.sig.ident); - - let self_ty = fn_descriptor - .self_ty - .as_ref() - .and_then(syn::Path::get_ident); - - let ffi_fn_doc = gen_doc(self_ty, &fn_descriptor.sig.ident); + let ffi_fn_name = gen_fn_name(fn_descriptor); + let ffi_fn_doc = gen_doc(fn_descriptor); let fn_signature = gen_signature(&ffi_fn_name, fn_descriptor); quote! { @@ -24,14 +18,8 @@ pub fn gen_declaration(fn_descriptor: &FnDescriptor) -> TokenStream { } pub fn gen_definition(fn_descriptor: &FnDescriptor) -> TokenStream { - let ffi_fn_name = gen_fn_name(fn_descriptor.self_ty_name(), &fn_descriptor.sig.ident); - - let self_ty = fn_descriptor - .self_ty - .as_ref() - .and_then(syn::Path::get_ident); - - let ffi_fn_doc = gen_doc(self_ty, &fn_descriptor.sig.ident); + let ffi_fn_name = gen_fn_name(fn_descriptor); + let ffi_fn_doc = gen_doc(fn_descriptor); let fn_signature = gen_signature(&ffi_fn_name, fn_descriptor); let ffi_fn_body = gen_body(fn_descriptor); @@ -61,21 +49,42 @@ pub fn gen_definition(fn_descriptor: &FnDescriptor) -> TokenStream { } } -pub fn gen_fn_name(self_ty_name: Option<&Ident>, method_name: &Ident) -> Ident { - let self_ty_name = self_ty_name.map_or_else(Default::default, ToString::to_string); +pub fn gen_fn_name(fn_descriptor: &FnDescriptor) -> Ident { + let method_name = format!("__{}", &fn_descriptor.sig.ident); + let self_ty_name = fn_descriptor + .self_ty_name() + .map_or_else(Default::default, ToString::to_string); + let trait_name = fn_descriptor + .trait_name() + .map_or_else(Default::default, |trait_name| format!("__{trait_name}")); Ident::new( - &format!("{}__{}", self_ty_name, method_name), + &format!("{}{}{}", self_ty_name, trait_name, method_name), proc_macro2::Span::call_site(), ) } -fn gen_doc(self_type: Option<&Ident>, method_name: &Ident) -> String { +fn gen_doc(fn_descriptor: &FnDescriptor) -> String { // NOTE: [#docs = "some_doc"] expands to ///some_doc, therefore the leading space + let method_name = &fn_descriptor.sig.ident; + let self_type = fn_descriptor + .self_ty + .as_ref() + .and_then(syn::Path::get_ident); + let trait_name = fn_descriptor + .trait_name + .as_ref() + .and_then(syn::Path::get_ident); let path = self_type.map_or_else( || method_name.to_string(), - |self_ty| format!("{}::{}", self_ty, method_name), + |self_ty| { + trait_name.map_or_else( + || format!("{}::{}", self_ty, method_name), + // NOTE: Fully-qualified syntax currently not supported + |trait_| format!("{}::{}", trait_, method_name), + ) + }, ); format!( @@ -134,9 +143,8 @@ fn gen_input_conversion_stmts(fn_descriptor: &FnDescriptor) -> TokenStream { let mut stmts = quote! {}; if let Some(arg) = &fn_descriptor.receiver { - let arg_name = &arg.name(); - - stmts = if matches!(arg.src_type(), syn::Type::Path(_)) { + stmts = if is_fn_consume_and_return_self(fn_descriptor) { + let arg_name = arg.name(); quote! {let __tmp_handle = #arg_name.read();} } else { crate::util::gen_arg_ffi_to_src(arg, false) @@ -153,10 +161,11 @@ fn gen_input_conversion_stmts(fn_descriptor: &FnDescriptor) -> TokenStream { fn gen_method_call_stmt(fn_descriptor: &FnDescriptor) -> TokenStream { let ident = &fn_descriptor.sig.ident; let self_type = &fn_descriptor.self_ty; + let trait_name = &fn_descriptor.trait_name; let receiver = fn_descriptor.receiver.as_ref(); let self_arg_name = receiver.map_or_else(Vec::new, |arg| { - if matches!(arg.src_type(), syn::Type::Path(_)) { + if is_fn_consume_and_return_self(fn_descriptor) { return vec![Ident::new("__tmp_handle", proc_macro2::Span::call_site())]; } @@ -164,9 +173,15 @@ fn gen_method_call_stmt(fn_descriptor: &FnDescriptor) -> TokenStream { }); let fn_arg_names = fn_descriptor.input_args.iter().map(Arg::name); - let self_ty = self_type - .as_ref() - .map_or_else(|| quote!(), |self_ty| quote! {#self_ty::}); + let self_ty = self_type.as_ref().map_or_else( + || quote!(), + |self_ty| { + trait_name.as_ref().map_or_else( + || quote! {#self_ty::}, + |trait_| quote! {<#self_ty as #trait_>::}, + ) + }, + ); let method_call = quote! {#self_ty #ident(#(#self_arg_name,)* #(#fn_arg_names),*)}; fn_descriptor.output_arg.as_ref().map_or_else( @@ -183,30 +198,30 @@ fn gen_method_call_stmt(fn_descriptor: &FnDescriptor) -> TokenStream { } fn gen_output_assignment_stmts(fn_descriptor: &FnDescriptor) -> TokenStream { - if let Some(output_arg) = &fn_descriptor.output_arg { - if let Some(receiver) = &fn_descriptor.receiver { - let arg_name = receiver.name(); - let src_type = receiver.src_type(); - - if matches!(src_type, syn::Type::Path(_)) { - return quote! { - if __out_ptr.is_null() { - return Err(iroha_ffi::FfiReturn::ArgIsNull); - } - - __out_ptr.write(#arg_name); - }; + match &fn_descriptor.output_arg { + Some(output_arg) if is_fn_consume_and_return_self(fn_descriptor) => { + let arg_name = output_arg.name(); + quote! { + if __out_ptr.is_null() { + return Err(iroha_ffi::FfiReturn::ArgIsNull); + } + + __out_ptr.write(#arg_name); } } - - let (arg_name, arg_type) = (output_arg.name(), output_arg.ffi_type_resolved(true)); - let output_arg_conversion = crate::util::gen_arg_src_to_ffi(output_arg, true); - - return quote! { - #output_arg_conversion - iroha_ffi::OutPtrOf::<#arg_type>::write(__out_ptr, #arg_name)?; - }; + Some(output_arg) => { + let (arg_name, arg_type) = (output_arg.name(), output_arg.ffi_type_resolved(true)); + let output_arg_conversion = crate::util::gen_arg_src_to_ffi(output_arg, true); + quote! { + #output_arg_conversion + iroha_ffi::OutPtrOf::<#arg_type>::write(__out_ptr, #arg_name)?; + } + } + None => quote! {}, } +} - quote! {} +fn is_fn_consume_and_return_self(fn_descriptor: &FnDescriptor) -> bool { + fn_descriptor.receiver.as_ref().map(Arg::name) + == fn_descriptor.output_arg.as_ref().map(Arg::name) } diff --git a/ffi/derive/src/impl_visitor.rs b/ffi/derive/src/impl_visitor.rs index d253a7a78fd..f865cf7cc17 100644 --- a/ffi/derive/src/impl_visitor.rs +++ b/ffi/derive/src/impl_visitor.rs @@ -10,13 +10,6 @@ pub struct Arg { type_: Type, } -pub struct ImplDescriptor<'ast> { - /// Associated types in the impl block - pub associated_types: Vec<(&'ast Ident, &'ast Type)>, - /// Functions in the impl block - pub fns: Vec, -} - impl Arg { pub fn name(&self) -> &Ident { &self.name @@ -67,12 +60,19 @@ fn resolve_src_type(self_type: Option<&syn::Path>, mut arg_type: Type) -> Type { arg_type } +pub struct ImplDescriptor { + /// Functions in the impl block + pub fns: Vec, +} + pub struct FnDescriptor { /// Resolved type of the `Self` type pub self_ty: Option, + /// Trait name + pub trait_name: Option, /// Function documentation - pub doc: syn::Attribute, + pub doc: Option, /// Original signature of the method pub sig: syn::Signature, @@ -85,9 +85,8 @@ pub struct FnDescriptor { } struct ImplVisitor<'ast> { + /// Trait name trait_name: Option<&'ast syn::Path>, - /// Associated types in the impl block - associated_types: Vec<(&'ast Ident, &'ast Type)>, /// Resolved type of the `Self` type self_ty: Option<&'ast syn::Path>, /// Collection of FFI functions @@ -97,6 +96,8 @@ struct ImplVisitor<'ast> { struct FnVisitor<'ast> { /// Resolved type of the `Self` type self_ty: Option<&'ast syn::Path>, + /// Trait name + trait_name: Option<&'ast syn::Path>, /// Function documentation doc: Option, @@ -114,35 +115,44 @@ struct FnVisitor<'ast> { curr_arg_name: Option<&'ast Ident>, } -impl<'ast> ImplDescriptor<'ast> { - pub fn from_impl(node: &'ast syn::ItemImpl) -> Self { +impl ImplDescriptor { + pub fn from_impl(node: &syn::ItemImpl) -> Self { let mut visitor = ImplVisitor::new(); visitor.visit_item_impl(node); ImplDescriptor::from_visitor(visitor) } - fn from_visitor(visitor: ImplVisitor<'ast>) -> Self { - Self { - fns: visitor.fns, - associated_types: visitor.associated_types, - } + fn from_visitor(visitor: ImplVisitor) -> Self { + Self { fns: visitor.fns } } } impl FnDescriptor { - fn from_impl_method(self_ty: &syn::Path, node: &syn::ImplItemMethod) -> Self { - let mut visitor = FnVisitor::new(Some(self_ty)); + pub fn from_impl_method( + self_ty: &syn::Path, + trait_name: Option<&syn::Path>, + node: &syn::ImplItemMethod, + ) -> Self { + let mut visitor = FnVisitor::new(Some(self_ty), trait_name); visitor.visit_impl_item_method(node); FnDescriptor::from_visitor(visitor) } + pub fn from_fn(node: &syn::ItemFn) -> Self { + let mut visitor = FnVisitor::new(None, None); + + visitor.visit_item_fn(node); + Self::from_visitor(visitor) + } + fn from_visitor(visitor: FnVisitor) -> Self { Self { self_ty: visitor.self_ty.map(Clone::clone), + trait_name: visitor.trait_name.map(Clone::clone), - doc: visitor.doc.expect_or_abort("Missing doc"), + doc: visitor.doc, sig: visitor.sig.expect_or_abort("Missing signature").clone(), receiver: visitor.receiver, @@ -154,13 +164,9 @@ impl FnDescriptor { pub fn self_ty_name(&self) -> Option<&Ident> { self.self_ty.as_ref().map(get_ident) } -} -impl From<&syn::ItemFn> for FnDescriptor { - fn from(item: &syn::ItemFn) -> Self { - let mut visitor = FnVisitor::new(None); - visitor.visit_item_fn(item); - Self::from_visitor(visitor) + pub fn trait_name(&self) -> Option<&Ident> { + self.trait_name.as_ref().map(get_ident) } } @@ -168,7 +174,6 @@ impl<'ast> ImplVisitor<'ast> { const fn new() -> Self { Self { trait_name: None, - associated_types: Vec::new(), self_ty: None, fns: vec![], } @@ -188,9 +193,13 @@ impl<'ast> ImplVisitor<'ast> { } impl<'ast> FnVisitor<'ast> { - pub const fn new(self_ty: Option<&'ast syn::Path>) -> Self { + pub const fn new( + self_ty: Option<&'ast syn::Path>, + trait_name: Option<&'ast syn::Path>, + ) -> Self { Self { self_ty, + trait_name, doc: None, sig: None, @@ -212,41 +221,33 @@ impl<'ast> FnVisitor<'ast> { )); } - /// Produces name of the return type. Name of the self argument is used for dummy - /// output type which is not present in the FFI function signature. Dummy type is - /// used to signal that the self type passes through the method being transcribed - fn gen_output_arg_name(&mut self, output_src_type: &Type) -> Ident { - if let Some(receiver) = &mut self.receiver { - let self_src_ty = &mut receiver.type_; - - if *self_src_ty == *output_src_type { - if matches!(self_src_ty, Type::Path(_)) { - // NOTE: `Self` is first consumed and then returned in the same method - let name = core::mem::replace(&mut receiver.name, parse_quote! {irrelevant}); - - *receiver = Arg::new( - self.self_ty.map(Clone::clone), - name, - parse_quote! {#self_src_ty}, - ); - } - - return receiver.name.clone(); + /// Set return type arg name to self type arg name + /// in case of function like `fn(self, ...) -> Self`, otherwise leave name unchanged + fn set_output_arg_name(&self, mut output_arg: Arg) -> Arg { + if let Some(receiver) = &self.receiver { + // NOTE: types need to be resolved here in case of `fn(self) -> Type` where `Type == Self` + // NOTE: `Self` is first consumed and then returned in the same method + if matches!(receiver.src_type(), Type::Path(_)) + && receiver.src_type_resolved() == output_arg.src_type_resolved() + { + output_arg.name = receiver.name().clone(); } } - Ident::new("__output", Span::call_site()) + output_arg } fn add_output_arg(&mut self, src_type: &'ast Type) { assert!(self.curr_arg_name.is_none()); assert!(self.output_arg.is_none()); - self.output_arg = Some(Arg::new( + let output_arg = Arg::new( self.self_ty.map(Clone::clone), - self.gen_output_arg_name(src_type), + Ident::new("__output", Span::call_site()), src_type.clone(), - )); + ); + + self.output_arg = Some(self.set_output_arg_name(output_arg)); } } @@ -261,25 +262,27 @@ impl<'ast> Visit<'ast> for ImplVisitor<'ast> { if node.unsafety.is_some() { // NOTE: Its's irrelevant } - self.trait_name = node.trait_.as_ref().map(|trait_| &trait_.1); + self.trait_name = node.trait_.as_ref().map(|(_, trait_, _)| trait_); self.visit_self_type(&*node.self_ty); - for it in &node.items { - match it { + let self_ty = self.self_ty.expect_or_abort("Defined"); + self.fns + .extend(node.items.iter().filter_map(|item| match item { syn::ImplItem::Method(method) => { - let self_ty = self.self_ty.expect_or_abort("Defined"); - self.fns - .push(FnDescriptor::from_impl_method(self_ty, method)) - } - syn::ImplItem::Type(type_) => { - self.associated_types.push((&type_.ident, &type_.ty)); + // NOTE: private methods in inherent impl are skipped + if self.trait_name.is_none() + && !matches!(method.vis, syn::Visibility::Public(_)) + { + return None; + } + Some(FnDescriptor::from_impl_method( + self_ty, + self.trait_name, + method, + )) } - _ => abort!( - node, - "Only methods or types are supported inside impl blocks" - ), - } - } + _ => None, + })); } } @@ -287,8 +290,11 @@ impl<'ast> Visit<'ast> for FnVisitor<'ast> { fn visit_impl_item_method(&mut self, node: &'ast syn::ImplItemMethod) { self.doc = find_doc_attr(&node.attrs).cloned(); - if !matches!(node.vis, syn::Visibility::Public(_)) { - abort!(node.vis, "Methods defined in the impl block must be public"); + if self.trait_name.is_none() && !matches!(node.vis, syn::Visibility::Public(_)) { + abort!( + node.vis, + "Private methods defined in an inherent `impl` block should not be exported, this is a bug in the library", + ); } self.sig = Some(&node.sig); diff --git a/ffi/derive/src/lib.rs b/ffi/derive/src/lib.rs index e2712af4689..ef7d8ddf298 100644 --- a/ffi/derive/src/lib.rs +++ b/ffi/derive/src/lib.rs @@ -173,7 +173,7 @@ pub fn ffi_export(_attr: TokenStream, item: TokenStream) -> TokenStream { abort!(item.sig.generics, "Generics are not supported"); } - let fn_descriptor = FnDescriptor::from(&item); + let fn_descriptor = FnDescriptor::from_fn(&item); let ffi_fn = ffi_fn::gen_definition(&fn_descriptor); quote! { #item @@ -192,7 +192,7 @@ pub fn ffi_import(_attr: TokenStream, item: TokenStream) -> TokenStream { match parse_macro_input!(item) { Item::Impl(item) => { let impl_descriptor = ImplDescriptor::from_impl(&item); - let ffi_fns = impl_descriptor.fns.iter().map(ffi_fn::gen_definition); + let ffi_fns = impl_descriptor.fns.iter().map(ffi_fn::gen_declaration); // TODO: Should be fixed in https://github.com/hyperledger/iroha/issues/2231 //let item = wrapper::wrap_impl_item(&impl_descriptor.fns); @@ -240,7 +240,7 @@ pub fn ffi_import(_attr: TokenStream, item: TokenStream) -> TokenStream { abort!(item.sig.generics, "Generics are not supported"); } - let fn_descriptor = FnDescriptor::from(&item); + let fn_descriptor = FnDescriptor::from_fn(&item); let ffi_fn = ffi_fn::gen_declaration(&fn_descriptor); quote! { #item diff --git a/ffi/derive/src/util.rs b/ffi/derive/src/util.rs index 7f797272237..e078652b00a 100644 --- a/ffi/derive/src/util.rs +++ b/ffi/derive/src/util.rs @@ -174,9 +174,7 @@ fn gen_derived_method(item_name: &Ident, field: &syn::Field, derive: Derive) -> let self_ty = Some(parse_quote! {#item_name}); let sig = gen_derived_method_sig(field, derive); - let doc = find_doc_attr(&field.attrs) - .cloned() - .expect_or_abort("Missing documentation"); + let doc = find_doc_attr(&field.attrs).cloned(); let field_ty = &field.ty; let field_ty = match derive { @@ -205,6 +203,7 @@ fn gen_derived_method(item_name: &Ident, field: &syn::Field, derive: Derive) -> FnDescriptor { self_ty, + trait_name: None, doc, sig, receiver: Some(receiver), diff --git a/ffi/derive/tests/ui_fail/private_method_inside_impl_are_skipped.rs b/ffi/derive/tests/ui_fail/private_method_inside_impl_are_skipped.rs new file mode 100644 index 00000000000..659abc0b25f --- /dev/null +++ b/ffi/derive/tests/ui_fail/private_method_inside_impl_are_skipped.rs @@ -0,0 +1,21 @@ +use std::alloc::alloc; + +use iroha_ffi::{ffi_export, IntoFfi, TryFromReprC}; + +/// FfiStruct +#[derive(Clone, IntoFfi, TryFromReprC)] +pub struct FfiStruct; + +#[ffi_export] +impl FfiStruct { + /// Private methods are skipped + fn private(self) {} +} + +fn main() { + let s = FfiStruct; + unsafe { + // Function not found + FfiStruct__private(IntoFfi::into_ffi(s)); + } +} \ No newline at end of file diff --git a/ffi/derive/tests/ui_fail/private_method_inside_impl_are_skipped.stderr b/ffi/derive/tests/ui_fail/private_method_inside_impl_are_skipped.stderr new file mode 100644 index 00000000000..4050366956e --- /dev/null +++ b/ffi/derive/tests/ui_fail/private_method_inside_impl_are_skipped.stderr @@ -0,0 +1,5 @@ +error[E0425]: cannot find function, tuple struct or tuple variant `FfiStruct__private` in this scope + --> tests/ui_fail/private_method_inside_impl_are_skipped.rs:19:9 + | +19 | FfiStruct__private(IntoFfi::into_ffi(s)); + | ^^^^^^^^^^^^^^^^^^ not found in this scope diff --git a/ffi/tests/ffi_export.rs b/ffi/tests/ffi_export.rs index 4e4a70908a3..8ad79e695e4 100644 --- a/ffi/tests/ffi_export.rs +++ b/ffi/tests/ffi_export.rs @@ -53,8 +53,9 @@ impl FfiStruct { } /// With params + // Note: `-> FfiStruct` used instead of `-> Self` to showcase that such signature supported by `#[ffi_export]` #[must_use] - pub fn with_params(mut self, params: impl IntoIterator) -> Self { + pub fn with_params(mut self, params: impl IntoIterator) -> FfiStruct { self.params = params.into_iter().collect(); self } @@ -95,6 +96,21 @@ pub fn simple(byte: u8) -> u8 { byte } +pub trait Target { + type Target; + + fn target(self) -> Self::Target; +} + +#[ffi_export] +impl Target for FfiStruct { + type Target = Option; + + fn target(self) -> ::Target { + self.name + } +} + fn get_new_struct() -> FfiStruct { let name = Name(String::from("X")); @@ -316,3 +332,18 @@ fn conversion_failed() { ) } } + +#[test] +fn invoke_trait_method() { + let ffi_struct = get_new_struct_with_params(); + let mut output = MaybeUninit::<*mut Name>::new(core::ptr::null_mut()); + + unsafe { + assert_eq!( + FfiReturn::Ok, + FfiStruct__Target__target(IntoFfi::into_ffi(ffi_struct), output.as_mut_ptr()) + ); + let name = TryFromReprC::try_from_repr_c(output.assume_init(), &mut ()).unwrap(); + assert_eq!(Name(String::from("X")), name); + } +} diff --git a/ffi/tests/unambiguous.rs b/ffi/tests/unambiguous.rs new file mode 100644 index 00000000000..bfa9a95ebb1 --- /dev/null +++ b/ffi/tests/unambiguous.rs @@ -0,0 +1,77 @@ +#![allow(unsafe_code, clippy::restriction, clippy::pedantic)] + +use std::{alloc::alloc, mem::MaybeUninit}; + +use iroha_ffi::{ffi_export, FfiReturn, IntoFfi, TryFromReprC}; + +#[derive(IntoFfi, TryFromReprC, PartialEq, Eq, Debug, Clone, Copy)] +#[repr(u8)] +pub enum Ambiguous { + Inherent, + AmbiguousX, + AmbiguousY, + None, +} + +/// FfiStruct +#[derive(Clone, Copy, IntoFfi, TryFromReprC)] +#[ffi_export] +pub struct FfiStruct {} + +#[ffi_export] +impl FfiStruct { + /// Ambiguous method + pub fn ambiguous() -> Ambiguous { + Ambiguous::Inherent + } +} + +pub trait AmbiguousX { + fn ambiguous() -> Ambiguous; +} + +pub trait AmbiguousY { + fn ambiguous() -> Ambiguous; +} + +#[ffi_export] +impl AmbiguousX for FfiStruct { + fn ambiguous() -> Ambiguous { + Ambiguous::AmbiguousX + } +} + +#[ffi_export] +impl AmbiguousY for FfiStruct { + fn ambiguous() -> Ambiguous { + Ambiguous::AmbiguousY + } +} + +#[test] +fn unambiguous_method_call() { + let mut output = MaybeUninit::new(Ambiguous::None as _); + + unsafe { + assert_eq!(FfiReturn::Ok, FfiStruct__ambiguous(output.as_mut_ptr())); + let inherent: Ambiguous = + TryFromReprC::try_from_repr_c(output.assume_init(), &mut ()).unwrap(); + assert_eq!(Ambiguous::Inherent, inherent); + + assert_eq!( + FfiReturn::Ok, + FfiStruct__AmbiguousX__ambiguous(output.as_mut_ptr()) + ); + let ambiguous_x: Ambiguous = + TryFromReprC::try_from_repr_c(output.assume_init(), &mut ()).unwrap(); + assert_eq!(Ambiguous::AmbiguousX, ambiguous_x); + + assert_eq!( + FfiReturn::Ok, + FfiStruct__AmbiguousY__ambiguous(output.as_mut_ptr()) + ); + let ambiguous_y: Ambiguous = + TryFromReprC::try_from_repr_c(output.assume_init(), &mut ()).unwrap(); + assert_eq!(Ambiguous::AmbiguousY, ambiguous_y); + } +}