From c6ab048efada5e75a9e7056ed48e3193e268426e Mon Sep 17 00:00:00 2001 From: Mikko Rantanen Date: Fri, 1 May 2020 00:02:59 +0300 Subject: [PATCH] Implement parameter reordering --- intercom-attributes/src/lib.rs | 15 +++ intercom-cli/src/generators/idl.rs | 13 +- .../src/attributes/com_interface.rs | 20 +-- intercom-common/src/methodinfo.rs | 55 +++++++- intercom-common/src/model/cominterface.rs | 126 ++++++++++++++++-- intercom-common/src/model/comsignature.rs | 53 ++++++++ intercom-common/src/model/mod.rs | 5 + intercom-common/src/utils.rs | 88 ------------ intercom/src/prelude.rs | 3 +- test/cpp-raw/CMakeLists.txt | 1 + test/cpp-raw/parameter_order.cpp | 44 ++++++ test/testlib/src/lib.rs | 2 + test/testlib/src/parameter_order.rs | 27 ++++ 13 files changed, 335 insertions(+), 117 deletions(-) create mode 100644 intercom-common/src/model/comsignature.rs create mode 100644 test/cpp-raw/parameter_order.cpp create mode 100644 test/testlib/src/parameter_order.rs diff --git a/intercom-attributes/src/lib.rs b/intercom-attributes/src/lib.rs index 76d6738..1ad340d 100644 --- a/intercom-attributes/src/lib.rs +++ b/intercom-attributes/src/lib.rs @@ -125,6 +125,21 @@ pub fn com_library(args: TokenStream) -> TokenStream } } +/// Metadata for method signature. +/// +/// ```rust,ignore +/// #[com_interface] +/// pub trait IFoo { +/// #[signature(OUT, a, b, c)] +/// fn foo(&self, a: u32, b: u32, c: u32) -> ComResult; +/// } +/// ``` +#[proc_macro_attribute] +pub fn com_signature(_attr: TokenStream, tokens: TokenStream) -> TokenStream +{ + tokens +} + /// Derives the implementation of the trait ForeignType for a type. #[proc_macro_derive(ForeignType)] pub fn named_type_derive(input: proc_macro::TokenStream) -> proc_macro::TokenStream diff --git a/intercom-cli/src/generators/idl.rs b/intercom-cli/src/generators/idl.rs index 949c381..284ab10 100644 --- a/intercom-cli/src/generators/idl.rs +++ b/intercom-cli/src/generators/idl.rs @@ -160,7 +160,11 @@ impl IdlMethod args: method .parameters .iter() - .map(|arg| IdlArg::try_from(arg, opts, ctx)) + .enumerate() + .map(|(idx, arg)| { + let is_last = idx == method.parameters.len() - 1; + IdlArg::try_from(arg, opts, is_last, ctx) + }) .collect::, _>>()?, }) } @@ -171,6 +175,7 @@ impl IdlArg fn try_from( arg: &Arg, opts: &TypeSystemOptions, + is_last: bool, ctx: &LibraryContext, ) -> Result { @@ -180,7 +185,11 @@ impl IdlArg Direction::Out => attrs.push("out"), Direction::Retval => { attrs.push("out"); - attrs.push("retval"); + + // Only the last parameter in the IDL is allowed [[retval]] + if is_last { + attrs.push("retval"); + } } Direction::Return => { return Err("Direction::Return is invalid direction for arguments" diff --git a/intercom-common/src/attributes/com_interface.rs b/intercom-common/src/attributes/com_interface.rs index 6129afb..33d9a71 100644 --- a/intercom-common/src/attributes/com_interface.rs +++ b/intercom-common/src/attributes/com_interface.rs @@ -70,7 +70,7 @@ pub fn expand_com_interface( process_itf_variant(&itf, *ts, itf_variant, &mut output, &mut itf_output); } - if itf.item_type == utils::InterfaceType::Trait { + if itf.item_type == model::InterfaceType::Trait { let mut impls = vec![]; for (_, method) in itf_output.method_impls.iter() { let method_rust_ident = &method.info.name; @@ -146,7 +146,7 @@ pub fn expand_com_interface( // Implement the ComInterface for the trait. let iid_arms = itf_output.iid_arms; - let (deref_impl, deref_ret) = if itf.item_type == utils::InterfaceType::Trait { + let (deref_impl, deref_ret) = if itf.item_type == model::InterfaceType::Trait { ( quote_spanned!(itf.span => com_itf), quote_spanned!(itf.span => &( dyn #itf_path + 'static ) ), @@ -313,9 +313,9 @@ fn process_itf_variant( // Create the vtable. We've already gathered all the vtable method // pointer fields so defining the struct is simple enough. let itf_bound = match itf.item_type { - utils::InterfaceType::Struct => quote!(), - utils::InterfaceType::Trait if itf.implemented_by.is_some() => quote!(), - utils::InterfaceType::Trait => quote!(+ #itf_ident), + model::InterfaceType::Struct => quote!(), + model::InterfaceType::Trait if itf.implemented_by.is_some() => quote!(), + model::InterfaceType::Trait => quote!(+ #itf_ident), }; if itf.vtable_of.is_none() { output.push(quote_spanned!(itf.span => @@ -472,8 +472,8 @@ fn format_method_vtable_entries( let method_impl_ident = idents::com_to_rust_method_impl(itf_ident, method_ident, ts); let ret_ty = method_info.returnhandler.com_ty(); let generics = match itf.item_type { - utils::InterfaceType::Struct => quote!(), - utils::InterfaceType::Trait => quote!(::), + model::InterfaceType::Struct => quote!(), + model::InterfaceType::Trait => quote!(::), }; let params = method_info.get_parameters_tokenstream(); @@ -543,8 +543,8 @@ fn create_virtual_method( // anywya, we won't use generic parameters on struct impl based implicit // interfaces. let (generics, bounds, s_ref, i_ref) = match itf.item_type { - utils::InterfaceType::Struct => (quote!(), quote!(), quote!(#itf_path), quote!(#itf_path)), - utils::InterfaceType::Trait => ( + model::InterfaceType::Struct => (quote!(), quote!(), quote!(#itf_path), quote!(#itf_path)), + model::InterfaceType::Trait => ( quote!(), quote!( where I: ?Sized, @@ -629,7 +629,7 @@ fn create_get_typeinfo_function(itf: &model::ComInterface) -> Result #[allow(non_snake_case)] diff --git a/intercom-common/src/methodinfo.rs b/intercom-common/src/methodinfo.rs index 02367e8..42986ff 100644 --- a/intercom-common/src/methodinfo.rs +++ b/intercom-common/src/methodinfo.rs @@ -1,9 +1,12 @@ use crate::prelude::*; use proc_macro2::Span; use std::rc::Rc; -use syn::{spanned::Spanned, FnArg, PathArguments, Receiver, ReturnType, Signature, Type}; +use syn::{ + spanned::Spanned, Attribute, FnArg, PathArguments, Receiver, ReturnType, Signature, Type, +}; use crate::ast_converters::*; +use crate::model::{ComSignature, SignatureItem}; use crate::returnhandlers::{get_return_handler, ReturnHandler}; use crate::tyhandlers::{get_ty_handler, Direction, ModelTypeSystem, TypeContext, TypeHandler}; use crate::utils; @@ -14,6 +17,7 @@ pub enum ComMethodInfoError TooFewArguments, BadSelfArg, BadArg(Box), + Signature, BadReturnType, } @@ -63,6 +67,7 @@ impl RustArg } } +#[derive(Clone)] pub struct ComArg { /// Name of the argument. @@ -168,6 +173,11 @@ pub struct ComMethodInfo /// Is the method infallible. pub infallible: bool, + + /// Parameter map for the COM parameters. + /// + /// For the Nth COM-parameter, args[param_map[N]] is the respective RustArg. + pub param_map: Vec, } impl PartialEq for ComMethodInfo @@ -189,6 +199,7 @@ impl ComMethodInfo /// Constructs new COM method info from a Rust method signature. pub fn new( decl: &Signature, + attrs: &[Attribute], type_system: ModelTypeSystem, ) -> Result { @@ -209,7 +220,7 @@ impl ComMethodInfo }; // Process other arguments. - let args = iter + let args: Vec = iter .map(|arg| { let ty = arg .get_ty() @@ -240,6 +251,37 @@ impl ComMethodInfo let returnhandler = get_return_handler(&retval_type, &return_type, retval_span, type_system) .or(Err(ComMethodInfoError::BadReturnType))?; + + let signature = attrs + .iter() + .find(|attr| match attr.path.get_ident() { + Some(ident) => ident == "com_signature", + None => false, + }) + .map(|attr| ComSignature::from_ast(attr.tokens.clone(), &decl.ident)) + .transpose() + .map_err(|_| ComMethodInfoError::Signature)?; + let param_mapping_indices = signature + .map(|sig| { + sig.params + .iter() + .map(|p| match p { + SignatureItem::Input(ident) => args + .iter() + .enumerate() + .find(|(_, arg)| ident == &arg.name) + .map(|(idx, _)| idx) + .ok_or_else(|| ComMethodInfoError::Signature), + SignatureItem::Output(idx) => Ok(args.len() + *idx as usize), + }) + .collect::, _>>() + }) + .transpose()? + .unwrap_or_else(|| { + let range = 0..(args.len() + returnhandler.com_out_args().len()); + range.collect::>() + }); + Ok(ComMethodInfo { name: n, infallible: returnhandler.is_infallible(), @@ -253,6 +295,7 @@ impl ComMethodInfo args, is_unsafe: unsafety, type_system, + param_map: param_mapping_indices, }) } @@ -264,7 +307,11 @@ impl ComMethodInfo .map(|ca| ComArg::from_rustarg(ca.clone(), Direction::In, self.type_system)); let out_args = self.returnhandler.com_out_args(); - in_args.chain(out_args).collect() + let all_args: Vec<_> = in_args.chain(out_args).collect(); + self.param_map + .iter() + .map(|&idx| all_args[idx].clone()) + .collect() } pub fn get_parameters_tokenstream(&self) -> TokenStream @@ -405,6 +452,6 @@ mod tests Item::Fn(ref f) => &f.sig, _ => panic!("Code isn't function"), }; - ComMethodInfo::new(sig, ts).unwrap() + ComMethodInfo::new(sig, &vec![], ts).unwrap() } } diff --git a/intercom-common/src/model/cominterface.rs b/intercom-common/src/model/cominterface.rs index 971ddbb..98f859f 100644 --- a/intercom-common/src/model/cominterface.rs +++ b/intercom-common/src/model/cominterface.rs @@ -11,7 +11,10 @@ use crate::tyhandlers::ModelTypeSystem; use indexmap::IndexMap; use proc_macro2::Span; use std::iter::FromIterator; -use syn::{Ident, LitStr, Path, TypePath, Visibility}; +use syn::{ + Attribute, Ident, ImplItem, Item, ItemImpl, ItemTrait, LitStr, Path, Signature, TraitItem, + Type, TypePath, Visibility, +}; intercom_attribute!( ComInterfaceAttr< ComInterfaceAttrParam, NoParams > { @@ -42,7 +45,7 @@ pub struct ComInterface pub visibility: Visibility, pub base_interface: Option, pub variants: IndexMap, - pub item_type: crate::utils::InterfaceType, + pub item_type: InterfaceType, pub span: Span, pub is_unsafe: bool, pub itf_ref: TokenStream, @@ -80,13 +83,12 @@ impl ComInterface // Get the interface details. As [com_interface] can be applied to both // impls and traits this handles both of those. - let (path, fns, itf_type, unsafety) = - crate::utils::get_ident_and_fns(&item).ok_or_else(|| { - ParseError::ComInterface( - item.get_ident().unwrap().to_string(), - "Unsupported associated item".into(), - ) - })?; + let (path, fns, itf_type, unsafety) = get_ident_and_fns(&item).ok_or_else(|| { + ParseError::ComInterface( + item.get_ident().unwrap().to_string(), + "Unsupported associated item".into(), + ) + })?; let ident = path.get_some_ident().ok_or_else(|| { ParseError::ComInterface( @@ -151,7 +153,7 @@ impl ComInterface // something smarter. let methods = fns .iter() - .map(|sig| ComMethodInfo::new(sig, ts)) + .map(|data| ComMethodInfo::new(data.sig, data.attrs, ts)) .filter_map(Result::ok) .collect::>(); @@ -168,8 +170,8 @@ impl ComInterface ); let itf_ref = match itf_type { - crate::utils::InterfaceType::Trait => quote_spanned!(ident.span() => dyn #path), - crate::utils::InterfaceType::Struct => quote_spanned!(ident.span() => #path), + InterfaceType::Trait => quote_spanned!(ident.span() => dyn #path), + InterfaceType::Struct => quote_spanned!(ident.span() => #path), }; Ok(ComInterface { @@ -209,6 +211,106 @@ impl ComInterface } } +#[derive(PartialEq, Debug, Clone, Copy)] +pub enum InterfaceType +{ + Trait, + Struct, +} + +pub struct MethodData<'a> +{ + attrs: &'a Vec, + sig: &'a Signature, +} + +pub type InterfaceData<'a> = ( + Path, + Vec>, + InterfaceType, + Option, +); + +pub type ImplData<'a> = (Option, Path, Vec>); + +fn get_ident_and_fns(item: &Item) -> Option +{ + match *item { + Item::Impl(ItemImpl { + ref unsafety, + ref trait_, + ref self_ty, + ref items, + .. + }) => { + let (_, struct_ident, items) = get_impl_data_raw(trait_, self_ty, items); + Some((struct_ident, items, InterfaceType::Struct, *unsafety)) + } + Item::Trait(ItemTrait { + ref ident, + unsafety, + ref items, + .. + }) => { + let methods: Option> = + items.iter().map(|i| get_trait_method(i)).collect(); + let path = syn::Path::from(ident.clone()); + + match methods { + Some(m) => Some((path, m, InterfaceType::Trait, unsafety)), + None => None, + } + } + _ => None, + } +} + +fn get_impl_data_raw<'a>( + trait_ref: &'a Option<(Option, Path, Token!(for))>, + struct_ty: &'a Type, + items: &'a [ImplItem], +) -> ImplData<'a> +{ + let struct_path = match struct_ty { + syn::Type::Path(typepath) => { + if typepath.qself.is_some() { + panic!("#[com_interface] cannot use associated types"); + } + typepath.path.clone() + } + _ => panic!("#[com_interface] must be defined for Path"), + }; + + let trait_path = trait_ref.as_ref().map(|(_, path, _)| path.clone()); + + let methods_opt: Option> = items.iter().map(|i| get_impl_method(i)).collect(); + let methods = methods_opt.unwrap_or_else(Vec::new); + + (trait_path, struct_path, methods) +} + +fn get_impl_method(i: &ImplItem) -> Option +{ + match *i { + ImplItem::Method(ref item) => Some(MethodData { + sig: &item.sig, + attrs: &item.attrs, + }), + _ => None, + } +} + +fn get_trait_method(i: &TraitItem) -> Option +{ + match *i { + TraitItem::Method(ref item) => Some(MethodData { + sig: &item.sig, + attrs: &item.attrs, + }), + _ => None, + } +} + #[cfg(test)] mod test { diff --git a/intercom-common/src/model/comsignature.rs b/intercom-common/src/model/comsignature.rs new file mode 100644 index 0000000..a4a9949 --- /dev/null +++ b/intercom-common/src/model/comsignature.rs @@ -0,0 +1,53 @@ +use super::*; +use crate::prelude::*; + +use syn::{Ident, LitInt, LitStr}; + +#[derive(Debug, Clone)] +pub enum SignatureItem +{ + Input(Ident), + Output(u32), +} + +impl syn::parse::Parse for SignatureItem +{ + fn parse(input: syn::parse::ParseStream) -> syn::parse::Result + { + let ident: syn::Ident = input.parse()?; + match ident { + out if out == "OUT" => { + let content; + bracketed!(content in input); + let value: LitInt = content.parse()?; + Ok(SignatureItem::Output(value.base10_parse()?)) + } + ident => Ok(SignatureItem::Input(ident)), + } + } +} + +intercom_attribute!(ComSignatureAttr < ComSignatureAttrParam, SignatureItem > { + com_iid : LitStr, +}); + +#[derive(Debug)] +pub struct ComSignature +{ + pub params: Vec, +} + +impl ComSignature +{ + /// Creates ComInterface from AST elements. + pub fn from_ast(attr: TokenStream, item: &Ident) -> ParseResult + { + let attr: ComSignatureAttr = ::syn::parse2(attr).map_err(|_| { + ParseError::ComSignature(item.to_string(), "Attribute syntax error".into()) + })?; + + Ok(ComSignature { + params: attr.args().into_iter().cloned().collect(), + }) + } +} diff --git a/intercom-common/src/model/mod.rs b/intercom-common/src/model/mod.rs index 5d01cf1..06708ad 100644 --- a/intercom-common/src/model/mod.rs +++ b/intercom-common/src/model/mod.rs @@ -19,6 +19,9 @@ pub enum ParseError #[fail(display = "Parsing [com_interface] item {} failed: {}", _0, _1)] ComInterface(String, String), + #[fail(display = "Parsing [com_signature] item {} failed: {}", _0, _1)] + ComSignature(String, String), + #[fail(display = "Processing crate failed: {}", _0)] ComCrate(String), @@ -41,3 +44,5 @@ mod comclass; pub use self::comclass::*; mod cominterface; pub use self::cominterface::*; +mod comsignature; +pub use self::comsignature::*; diff --git a/intercom-common/src/utils.rs b/intercom-common/src/utils.rs index d550c9f..1d1ab78 100644 --- a/intercom-common/src/utils.rs +++ b/intercom-common/src/utils.rs @@ -5,94 +5,6 @@ use syn::*; use super::*; use proc_macro2::Span; -#[derive(PartialEq, Debug, Clone, Copy)] -pub enum InterfaceType -{ - Trait, - Struct, -} - -pub type InterfaceData<'a> = ( - Path, - Vec<&'a Signature>, - InterfaceType, - Option, -); - -pub fn get_ident_and_fns(item: &Item) -> Option -{ - match *item { - Item::Impl(ItemImpl { - ref unsafety, - ref trait_, - ref self_ty, - ref items, - .. - }) => { - let (_, struct_ident, items) = get_impl_data_raw(trait_, self_ty, items); - Some((struct_ident, items, InterfaceType::Struct, *unsafety)) - } - Item::Trait(ItemTrait { - ref ident, - unsafety, - ref items, - .. - }) => { - let methods: Option> = - items.iter().map(|i| get_trait_method(i)).collect(); - let path = syn::Path::from(ident.clone()); - - match methods { - Some(m) => Some((path, m, InterfaceType::Trait, unsafety)), - None => None, - } - } - _ => None, - } -} - -pub type ImplData<'a> = (Option, Path, Vec<&'a Signature>); - -fn get_impl_data_raw<'a>( - trait_ref: &'a Option<(Option, Path, Token!(for))>, - struct_ty: &'a Type, - items: &'a [ImplItem], -) -> ImplData<'a> -{ - let struct_path = match struct_ty { - syn::Type::Path(typepath) => { - if typepath.qself.is_some() { - panic!("#[com_interface] cannot use associated types"); - } - typepath.path.clone() - } - _ => panic!("#[com_interface] must be defined for Path"), - }; - - let trait_path = trait_ref.as_ref().map(|(_, path, _)| path.clone()); - - let methods_opt: Option> = items.iter().map(|i| get_impl_method(i)).collect(); - let methods = methods_opt.unwrap_or_else(Vec::new); - - (trait_path, struct_path, methods) -} - -pub fn get_impl_method(i: &ImplItem) -> Option<&Signature> -{ - match *i { - ImplItem::Method(ref itm) => Some(&itm.sig), - _ => None, - } -} - -pub fn get_trait_method(i: &TraitItem) -> Option<&Signature> -{ - match *i { - TraitItem::Method(ref tim) => Some(&tim.sig), - _ => None, - } -} - const AUTO_GUID_BASE: guid::GUID = guid::GUID { data1: 0x4449_494C, data2: 0xDE1F, diff --git a/intercom/src/prelude.rs b/intercom/src/prelude.rs index f6ebdee..1e85309 100644 --- a/intercom/src/prelude.rs +++ b/intercom/src/prelude.rs @@ -1,3 +1,4 @@ pub use crate::{ - com_class, com_interface, com_library, com_module, ComBox, ComError, ComItf, ComRc, ComResult, + com_class, com_interface, com_library, com_module, com_signature, ComBox, ComError, ComItf, + ComRc, ComResult, }; diff --git a/test/cpp-raw/CMakeLists.txt b/test/cpp-raw/CMakeLists.txt index 28cb9c6..0d80710 100644 --- a/test/cpp-raw/CMakeLists.txt +++ b/test/cpp-raw/CMakeLists.txt @@ -24,6 +24,7 @@ ${PROJECT_SOURCE_DIR}/type_system_callbacks.cpp ${PROJECT_SOURCE_DIR}/variant.cpp ${PROJECT_SOURCE_DIR}/nullable_parameters.cpp ${PROJECT_SOURCE_DIR}/output_memory.cpp +${PROJECT_SOURCE_DIR}/parameter_order.cpp ) include_directories("${PROJECT_BINARY_DIR}") diff --git a/test/cpp-raw/parameter_order.cpp b/test/cpp-raw/parameter_order.cpp new file mode 100644 index 0000000..48214e7 --- /dev/null +++ b/test/cpp-raw/parameter_order.cpp @@ -0,0 +1,44 @@ + +#include "../cpp-utility/os.hpp" +#include "../dependencies/catch.hpp" + +#include "testlib.hpp" + +TEST_CASE( "parameter_order" ) +{ + // Initialize COM. + InitializeRuntime(); + + IParameterOrderTests_Automation* pTests = nullptr; + intercom::HRESULT hr = CreateInstance( + CLSID_ParameterOrderTests, + IID_IParameterOrderTests_Automation, + &pTests ); + REQUIRE( hr == intercom::SC_OK ); + REQUIRE( pTests != nullptr ); + + SECTION( "normal_order" ) + { + double result; + pTests->Reciprocal(123, &result); + REQUIRE( result == 1.0/123 ); + } + SECTION( "reversed" ) + { + double result; + pTests->ReciprocalReversed(&result, 123); + REQUIRE( result == 1.0/123 ); + } + SECTION( "interleaved" ) + { + double result1, result2; + pTests->ReciprocalTwo(123, &result1, -100, &result2); + REQUIRE( result1 == 1.0/123 ); + REQUIRE( result2 == -1.0/100 ); + } + + REQUIRE( pTests->Release() == 0 ); + + UninitializeRuntime(); +} + diff --git a/test/testlib/src/lib.rs b/test/testlib/src/lib.rs index c8337f1..e49be3f 100644 --- a/test/testlib/src/lib.rs +++ b/test/testlib/src/lib.rs @@ -11,6 +11,7 @@ pub mod error_info; pub mod interface_params; pub mod nullable_parameters; pub mod output_memory; +pub mod parameter_order; pub mod primitive; pub mod result; pub mod return_interfaces; @@ -37,4 +38,5 @@ com_library! { class variant::VariantImpl, class unicode::UnicodeConversion, class output_memory::OutputMemoryTests, + class parameter_order::ParameterOrderTests, } diff --git a/test/testlib/src/parameter_order.rs b/test/testlib/src/parameter_order.rs new file mode 100644 index 0000000..bd658f6 --- /dev/null +++ b/test/testlib/src/parameter_order.rs @@ -0,0 +1,27 @@ +use intercom::prelude::*; + +#[com_class(Self)] +#[derive(Default)] +pub struct ParameterOrderTests; + +#[com_interface] +impl ParameterOrderTests +{ + #[com_signature(value, OUT[0])] + pub fn reciprocal(&mut self, value: u32) -> ComResult + { + Ok(1.0 / f64::from(value)) + } + + #[com_signature(OUT[0], value)] + pub fn reciprocal_reversed(&mut self, value: u32) -> ComResult + { + Ok(1.0 / f64::from(value)) + } + + #[com_signature(v1, OUT[0], v2, OUT[1])] + pub fn reciprocal_two(&mut self, v1: u32, v2: u32) -> ComResult<(f64, f64)> + { + Ok((1.0 / f64::from(v1), 1.0 / f64::from(v2))) + } +}