From 11fc8d33ad0e051aa46a84be1ae452167c4e140c Mon Sep 17 00:00:00 2001 From: B_head Date: Sun, 6 Mar 2022 01:46:27 +0900 Subject: [PATCH 1/6] Refactoring export attribute - Enable to use Path syntax. - Remove code that only accepts [export = "wrong"]. --- gdnative-derive/src/methods.rs | 141 +++++++++++++++------------------ 1 file changed, 64 insertions(+), 77 deletions(-) diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index 730ff2f22..bcc4a1082 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -198,120 +198,107 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { if let Some("export") = last_seg.as_deref() { let _export_args = export_args.get_or_insert_with(ExportArgs::default); - if !attr.tokens.is_empty() { - use syn::{Meta, MetaNameValue, NestedMeta}; - - let meta = match attr.parse_meta() { - Ok(val) => val, + use syn::{punctuated::Punctuated, Lit, Meta, NestedMeta}; + let nested_meta_iter = match attr.parse_meta() { Err(err) => { errors.push(err); return false; } - }; - - let pairs: Vec<_> = match meta { - Meta::List(list) => list - .nested - .into_pairs() - .filter_map(|p| { - let span = p.span(); - match p.into_value() { - NestedMeta::Meta(Meta::NameValue(pair)) => { - Some(pair) - } - unexpected => { - let msg = format!( - "unexpected argument in list: {}", - unexpected.into_token_stream() - ); + Ok(Meta::NameValue(name_value)) => { + let span = name_value.span(); + let msg = "NameValue syntax is not valid"; errors.push(syn::Error::new(span, msg)); - None + return false; } + Ok(Meta::Path(_)) => { + Punctuated::::new().into_iter() } - }) - .collect(), - Meta::NameValue(pair) => vec![pair], - meta => { - let span = meta.span(); - let msg = format!( - "unexpected attribute argument: {}", - meta.into_token_stream() - ); + Ok(Meta::List(list)) => list.nested.into_iter(), + }; + for nested_meta in nested_meta_iter { + let (path, lit) = match &nested_meta { + NestedMeta::Lit(param) => { + let span = param.span(); + let msg = "Literal item is not valid"; errors.push(syn::Error::new(span, msg)); - return false; + continue; } - }; - - for MetaNameValue { path, lit, .. } in pairs { - let last = match path.segments.last() { - Some(val) => val, - None => { - errors.push(syn::Error::new( - path.span(), - "the path should not be empty", - )); - return false; + NestedMeta::Meta(param) => match param { + Meta::List(list) => { + let span = list.span(); + let msg = "List item is not valid"; + errors.push(syn::Error::new(span, msg)); + continue; } + Meta::Path(path) => (path, None), + Meta::NameValue(name_value) => { + (&name_value.path, Some(&name_value.lit)) + } + }, }; - let path = last.ident.to_string(); - - // Match optional export arguments - match path.as_str() { + if path.is_ident("rpc") { // rpc mode - "rpc" => { - let value = if let syn::Lit::Str(lit_str) = lit { - lit_str.value() - } else { + match lit { + None => { errors.push(syn::Error::new( - last.span(), - "unexpected type for rpc value, expected Str", + nested_meta.span(), + "name parameter requires string value", )); - return false; - }; - + } + Some(Lit::Str(str)) => { + let value = str.value(); if let Some(mode) = RpcMode::parse(value.as_str()) { if rpc.replace(mode).is_some() { errors.push(syn::Error::new( - last.span(), + nested_meta.span(), "rpc mode was set more than once", )); - return false; } } else { errors.push(syn::Error::new( - last.span(), + nested_meta.span(), format!("unexpected value for rpc: {}", value), )); - return false; } } + _ => { + errors.push(syn::Error::new( + nested_meta.span(), + "unexpected type for rpc value, expected string", + )); + } + } + } else if path.is_ident("name") { // name override - "name" => { - let value = if let syn::Lit::Str(lit_str) = lit { - lit_str.value() - } else { + match lit { + None => { errors.push(syn::Error::new( - last.span(), - "unexpected type for name value, expected Str", + nested_meta.span(), + "name parameter requires string value", )); - return false; - }; - - if name_override.replace(value).is_some() { + } + Some(Lit::Str(str)) => { + if name_override.replace(str.value()).is_some() { errors.push(syn::Error::new( - last.span(), + nested_meta.span(), "name was set more than once", )); - return false; } } _ => { - let msg = - format!("unknown option for export: `{}`", path); - errors.push(syn::Error::new(last.span(), msg)); - return false; + errors.push(syn::Error::new( + nested_meta.span(), + "unexpected type for name value, expected string", + )); } } + } + } else { + let msg = format!( + "unknown option for export: `{}`", + path.to_token_stream() + ); + errors.push(syn::Error::new(nested_meta.span(), msg)); } } return false; From ff852f92ae1ef7a7a60cf52ad2c2c65b2c45ebb6 Mon Sep 17 00:00:00 2001 From: B_head Date: Sun, 6 Mar 2022 01:46:51 +0900 Subject: [PATCH 2/6] Add deref_return parameter for export argument --- gdnative-core/src/export/macros.rs | 32 +++++++++++++++++++++++++++++- gdnative-derive/src/methods.rs | 19 ++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/gdnative-core/src/export/macros.rs b/gdnative-core/src/export/macros.rs index 92b3bea0f..5fc1708e9 100644 --- a/gdnative-core/src/export/macros.rs +++ b/gdnative-core/src/export/macros.rs @@ -11,11 +11,23 @@ macro_rules! godot_wrap_method_parameter_count { } } +#[doc(hidden)] +#[macro_export] +macro_rules! godot_wrap_method_if_deref { + (true, $ret:expr) => { + std::ops::Deref::deref(&$ret) + }; + (false, $ret:expr) => { + $ret + }; +} + #[doc(hidden)] #[macro_export] macro_rules! godot_wrap_method_inner { ( $type_name:ty, + $is_deref_return:ident, $map_method:ident, fn $method_name:ident( $self:ident, @@ -56,7 +68,9 @@ macro_rules! godot_wrap_method_inner { $($pname,)* $($opt_pname,)* ); - gdnative::core_types::OwnedToVariant::owned_to_variant(ret) + gdnative::core_types::OwnedToVariant::owned_to_variant( + $crate::godot_wrap_method_if_deref!($is_deref_return, ret) + ) } }) .unwrap_or_else(|err| { @@ -83,6 +97,7 @@ macro_rules! godot_wrap_method { // mutable ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( &mut $self:ident, $owner:ident : $owner_ty:ty @@ -93,6 +108,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method_inner!( $type_name, + $is_deref_return, map_mut, fn $method_name( $self, @@ -105,6 +121,7 @@ macro_rules! godot_wrap_method { // immutable ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( & $self:ident, $owner:ident : $owner_ty:ty @@ -115,6 +132,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method_inner!( $type_name, + $is_deref_return, map, fn $method_name( $self, @@ -127,6 +145,7 @@ macro_rules! godot_wrap_method { // owned ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( mut $self:ident, $owner:ident : $owner_ty:ty @@ -137,6 +156,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method_inner!( $type_name, + $is_deref_return, map_owned, fn $method_name( $self, @@ -149,6 +169,7 @@ macro_rules! godot_wrap_method { // owned ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( $self:ident, $owner:ident : $owner_ty:ty @@ -159,6 +180,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method_inner!( $type_name, + $is_deref_return, map_owned, fn $method_name( $self, @@ -171,6 +193,7 @@ macro_rules! godot_wrap_method { // mutable without return type ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( &mut $self:ident, $owner:ident : $owner_ty:ty @@ -181,6 +204,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method!( $type_name, + $is_deref_return, fn $method_name( &mut $self, $owner: $owner_ty @@ -192,6 +216,7 @@ macro_rules! godot_wrap_method { // immutable without return type ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( & $self:ident, $owner:ident : $owner_ty:ty @@ -202,6 +227,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method!( $type_name, + $is_deref_return, fn $method_name( & $self, $owner: $owner_ty @@ -213,6 +239,7 @@ macro_rules! godot_wrap_method { // owned without return type ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( mut $self:ident, $owner:ident : $owner_ty:ty @@ -223,6 +250,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method!( $type_name, + $is_deref_return, fn $method_name( $self, $owner: $owner_ty @@ -234,6 +262,7 @@ macro_rules! godot_wrap_method { // owned without return type ( $type_name:ty, + $is_deref_return:ident, fn $method_name:ident( $self:ident, $owner:ident : $owner_ty:ty @@ -244,6 +273,7 @@ macro_rules! godot_wrap_method { ) => { $crate::godot_wrap_method!( $type_name, + $is_deref_return, fn $method_name( $self, $owner: $owner_ty diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index bcc4a1082..c4d264cff 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -66,6 +66,7 @@ pub(crate) struct ExportArgs { pub(crate) optional_args: Option, pub(crate) rpc_mode: RpcMode, pub(crate) name_override: Option, + pub(crate) is_deref_return: bool, } pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { @@ -116,6 +117,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { }; let rpc = args.rpc_mode; + let is_deref_return = args.is_deref_return; let args = sig.inputs.iter().enumerate().map(|(n, arg)| { let span = arg.span(); @@ -130,6 +132,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { { let method = ::gdnative::export::godot_wrap_method!( #class_name, + #is_deref_return, fn #name ( #( #args )* ) -> #ret_ty ); @@ -183,6 +186,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { let mut export_args = None; let mut rpc = None; let mut name_override = None; + let mut is_deref_return = false; let mut errors = vec![]; @@ -292,6 +296,20 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { )); } } + } else if path.is_ident("deref_return") { + // deref return value + if lit.is_some() { + errors.push(syn::Error::new( + nested_meta.span(), + "value for deref_return parameter is not valid", + )); + } else if is_deref_return { + errors.push(syn::Error::new( + nested_meta.span(), + "deref_return was apply more than once", + )); + } else { + is_deref_return = true; } } else { let msg = format!( @@ -350,6 +368,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { export_args.optional_args = optional_args; export_args.rpc_mode = rpc.unwrap_or(RpcMode::Disabled); export_args.name_override = name_override; + export_args.is_deref_return = is_deref_return; methods_to_export.push(ExportMethod { sig: method.sig.clone(), From bdd12883a6d77b0c853176937f2214f5c7b40aab Mon Sep 17 00:00:00 2001 From: B_head Date: Sun, 6 Mar 2022 09:07:31 +0900 Subject: [PATCH 3/6] Deprecated return by reference --- gdnative-core/src/export/macros.rs | 7 +++++ gdnative-core/src/export/mod.rs | 2 ++ gdnative-derive/src/lib.rs | 2 +- gdnative-derive/src/methods.rs | 42 +++++++++++++++++++----------- 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/gdnative-core/src/export/macros.rs b/gdnative-core/src/export/macros.rs index 5fc1708e9..8e5d76d46 100644 --- a/gdnative-core/src/export/macros.rs +++ b/gdnative-core/src/export/macros.rs @@ -22,6 +22,13 @@ macro_rules! godot_wrap_method_if_deref { }; } +#[doc(hidden)] +#[macro_export] +#[deprecated = "This function does not actually pass by reference to the Godot engine. You can clarify by writing #[export(deref_return)]."] +macro_rules! deprecated_reference_return { + () => {}; +} + #[doc(hidden)] #[macro_export] macro_rules! godot_wrap_method_inner { diff --git a/gdnative-core/src/export/mod.rs b/gdnative-core/src/export/mod.rs index 70fdd1437..a59619d7c 100644 --- a/gdnative-core/src/export/mod.rs +++ b/gdnative-core/src/export/mod.rs @@ -26,6 +26,8 @@ pub(crate) mod type_tag; pub mod user_data; +#[allow(deprecated)] +pub use crate::deprecated_reference_return; pub use crate::godot_wrap_method; pub use class::*; pub use class_builder::*; diff --git a/gdnative-derive/src/lib.rs b/gdnative-derive/src/lib.rs index 612a0df54..e37c3c006 100644 --- a/gdnative-derive/src/lib.rs +++ b/gdnative-derive/src/lib.rs @@ -50,7 +50,7 @@ mod variant; /// impl gdnative::export::NativeClassMethods for Foo { /// fn register(builder: &ClassBuilder) { /// use gdnative::export::*; -/// builder.method("foo", gdnative::export::godot_wrap_method!(Foo, fn foo(&self, _owner: &Reference, bar: i64) -> i64)) +/// builder.method("foo", gdnative::export::godot_wrap_method!(Foo, false, fn foo(&self, _owner: &Reference, bar: i64) -> i64)) /// .with_rpc_mode(RpcMode::Disabled) /// .done_stateless(); /// } diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index c4d264cff..1aa2e07a4 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -86,7 +86,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { let name = sig.ident; let name_string = args.name_override.unwrap_or_else(|| name.to_string()); let ret_span = sig.output.span(); - let ret_ty = match sig.output { + let ret_ty = match &sig.output { syn::ReturnType::Default => quote_spanned!(ret_span => ()), syn::ReturnType::Type(_, ty) => quote_spanned!( ret_span => #ty ), }; @@ -128,6 +128,16 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { } }); + let deprecated = if let syn::ReturnType::Type(_, ty) = &sig.output { + if !is_deref_return && matches!(**ty, syn::Type::Reference(_)) { + quote_spanned!(ret_span=> ::gdnative::export::deprecated_reference_return!();) + } else { + quote_spanned!(ret_span=>) + } + } else { + quote_spanned!(ret_span=>) + }; + quote_spanned!( sig_span=> { let method = ::gdnative::export::godot_wrap_method!( @@ -139,6 +149,8 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { #builder.method(#name_string, method) .with_rpc_mode(#rpc) .done_stateless(); + + #deprecated } ) }) @@ -204,19 +216,19 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { let _export_args = export_args.get_or_insert_with(ExportArgs::default); use syn::{punctuated::Punctuated, Lit, Meta, NestedMeta}; let nested_meta_iter = match attr.parse_meta() { - Err(err) => { - errors.push(err); - return false; - } + Err(err) => { + errors.push(err); + return false; + } Ok(Meta::NameValue(name_value)) => { let span = name_value.span(); let msg = "NameValue syntax is not valid"; - errors.push(syn::Error::new(span, msg)); + errors.push(syn::Error::new(span, msg)); return false; - } + } Ok(Meta::Path(_)) => { Punctuated::::new().into_iter() - } + } Ok(Meta::List(list)) => list.nested.into_iter(), }; for nested_meta in nested_meta_iter { @@ -239,15 +251,15 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { (&name_value.path, Some(&name_value.lit)) } }, - }; + }; if path.is_ident("rpc") { - // rpc mode + // rpc mode match lit { None => { - errors.push(syn::Error::new( + errors.push(syn::Error::new( nested_meta.span(), "name parameter requires string value", - )); + )); } Some(Lit::Str(str)) => { let value = str.value(); @@ -273,13 +285,13 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { } } } else if path.is_ident("name") { - // name override + // name override match lit { None => { - errors.push(syn::Error::new( + errors.push(syn::Error::new( nested_meta.span(), "name parameter requires string value", - )); + )); } Some(Lit::Str(str)) => { if name_override.replace(str.value()).is_some() { From 9e78ce73cafab825da09e00f2920456e8e79a17a Mon Sep 17 00:00:00 2001 From: B_head Date: Wed, 9 Mar 2022 06:49:05 +0900 Subject: [PATCH 4/6] Refactoring: Clarify code responsibility --- gdnative-derive/src/methods.rs | 40 ++++++++++++++-------------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index 1aa2e07a4..6671ac1d6 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -58,13 +58,13 @@ pub(crate) struct ClassMethodExport { #[derive(Clone, Eq, PartialEq, Hash, Debug)] pub(crate) struct ExportMethod { pub(crate) sig: Signature, - pub(crate) args: ExportArgs, + pub(crate) export_args: ExportArgs, + pub(crate) optional_args: Option, } #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Default)] pub(crate) struct ExportArgs { - pub(crate) optional_args: Option, - pub(crate) rpc_mode: RpcMode, + pub(crate) rpc_mode: Option, pub(crate) name_override: Option, pub(crate) is_deref_return: bool, } @@ -80,11 +80,11 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { let methods = export .methods .into_iter() - .map(|ExportMethod { sig, args }| { + .map(|ExportMethod { sig, export_args , optional_args}| { let sig_span = sig.ident.span(); let name = sig.ident; - let name_string = args.name_override.unwrap_or_else(|| name.to_string()); + let name_string = export_args.name_override.unwrap_or_else(|| name.to_string()); let ret_span = sig.output.span(); let ret_ty = match &sig.output { syn::ReturnType::Default => quote_spanned!(ret_span => ()), @@ -101,7 +101,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { .to_compile_error(); } - let optional_args = match args.optional_args { + let optional_args = match optional_args { Some(count) => { let max_optional = arg_count - 2; // self and owner if count > max_optional { @@ -116,8 +116,8 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { None => 0, }; - let rpc = args.rpc_mode; - let is_deref_return = args.is_deref_return; + let rpc = export_args.rpc_mode.unwrap_or(RpcMode::Disabled); + let is_deref_return = export_args.is_deref_return; let args = sig.inputs.iter().enumerate().map(|(n, arg)| { let span = arg.span(); @@ -196,10 +196,6 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { let items = match func { ImplItem::Method(mut method) => { let mut export_args = None; - let mut rpc = None; - let mut name_override = None; - let mut is_deref_return = false; - let mut errors = vec![]; // only allow the "outer" style, aka #[thing] item. @@ -213,7 +209,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { .map(|i| i.ident.to_string()); if let Some("export") = last_seg.as_deref() { - let _export_args = export_args.get_or_insert_with(ExportArgs::default); + let mut _export_args = export_args.get_or_insert_with(ExportArgs::default); use syn::{punctuated::Punctuated, Lit, Meta, NestedMeta}; let nested_meta_iter = match attr.parse_meta() { Err(err) => { @@ -264,7 +260,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { Some(Lit::Str(str)) => { let value = str.value(); if let Some(mode) = RpcMode::parse(value.as_str()) { - if rpc.replace(mode).is_some() { + if _export_args.rpc_mode.replace(mode).is_some() { errors.push(syn::Error::new( nested_meta.span(), "rpc mode was set more than once", @@ -294,7 +290,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { )); } Some(Lit::Str(str)) => { - if name_override.replace(str.value()).is_some() { + if _export_args.name_override.replace(str.value()).is_some() { errors.push(syn::Error::new( nested_meta.span(), "name was set more than once", @@ -315,13 +311,13 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { nested_meta.span(), "value for deref_return parameter is not valid", )); - } else if is_deref_return { + } else if _export_args.is_deref_return { errors.push(syn::Error::new( nested_meta.span(), "deref_return was apply more than once", )); } else { - is_deref_return = true; + _export_args.is_deref_return = true; } } else { let msg = format!( @@ -338,7 +334,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { true }); - if let Some(mut export_args) = export_args.take() { + if let Some(export_args) = export_args.take() { let mut optional_args = None; for (n, arg) in method.sig.inputs.iter_mut().enumerate() { @@ -377,14 +373,10 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { } } - export_args.optional_args = optional_args; - export_args.rpc_mode = rpc.unwrap_or(RpcMode::Disabled); - export_args.name_override = name_override; - export_args.is_deref_return = is_deref_return; - methods_to_export.push(ExportMethod { sig: method.sig.clone(), - args: export_args, + export_args, + optional_args, }); } From 3724c3638d80ff93d10cef067b12a239add8883c Mon Sep 17 00:00:00 2001 From: B_head Date: Wed, 9 Mar 2022 09:01:12 +0900 Subject: [PATCH 5/6] Refactoring of macros.rs --- gdnative-core/src/export/macros.rs | 189 +++++++++-------------------- 1 file changed, 54 insertions(+), 135 deletions(-) diff --git a/gdnative-core/src/export/macros.rs b/gdnative-core/src/export/macros.rs index 8e5d76d46..df11a9372 100644 --- a/gdnative-core/src/export/macros.rs +++ b/gdnative-core/src/export/macros.rs @@ -37,9 +37,9 @@ macro_rules! godot_wrap_method_inner { $is_deref_return:ident, $map_method:ident, fn $method_name:ident( - $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* + $self:ident + , $owner:ident : $owner_ty:ty + $(, $pname:ident : $pty:ty)* $(, #[opt] $opt_pname:ident : $opt_pty:ty)* ) -> $retty:ty ) => { @@ -97,6 +97,17 @@ macro_rules! godot_wrap_method_inner { }; } +#[doc(hidden)] +#[macro_export] +macro_rules! godot_wrap_method_return_type { + () => { + () + }; + ($retty:ty) => { + $retty: ty + }; +} + /// Convenience macro to wrap an object's method into a function pointer /// that can be passed to the engine when registering a class. #[macro_export] @@ -106,23 +117,23 @@ macro_rules! godot_wrap_method { $type_name:ty, $is_deref_return:ident, fn $method_name:ident( - &mut $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* + &mut $self:ident + , $owner:ident : $owner_ty:ty + $(, $pname:ident : $pty:ty)* + $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? - ) -> $retty:ty + ) $(-> $retty:ty)? ) => { $crate::godot_wrap_method_inner!( $type_name, $is_deref_return, map_mut, fn $method_name( - $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> $retty + $self + , $owner : $owner_ty + $(, $pname : $pty)* + $(, #[opt] $opt_pname : $opt_pty)* + ) -> godot_wrap_method_return_type!($($retty)?) ) }; // immutable @@ -130,23 +141,23 @@ macro_rules! godot_wrap_method { $type_name:ty, $is_deref_return:ident, fn $method_name:ident( - & $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* + & $self:ident + , $owner:ident : $owner_ty:ty + $(, $pname:ident : $pty:ty)* + $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? - ) -> $retty:ty + ) $(-> $retty:ty)? ) => { $crate::godot_wrap_method_inner!( $type_name, $is_deref_return, map, fn $method_name( - $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> $retty + $self + , $owner : $owner_ty + $(, $pname : $pty)* + $(, #[opt] $opt_pname : $opt_pty)* + ) -> godot_wrap_method_return_type!($($retty)?) ) }; // owned @@ -154,23 +165,23 @@ macro_rules! godot_wrap_method { $type_name:ty, $is_deref_return:ident, fn $method_name:ident( - mut $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* + mut $self:ident + , $owner:ident : $owner_ty:ty + $(, $pname:ident : $pty:ty)* + $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? - ) -> $retty:ty + ) $(-> $retty:ty)? ) => { $crate::godot_wrap_method_inner!( $type_name, $is_deref_return, map_owned, fn $method_name( - $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> $retty + $self + , $owner : $owner_ty + $(, $pname : $pty)* + $(, #[opt] $opt_pname : $opt_pty)* + ) -> godot_wrap_method_return_type!($($retty)?) ) }; // owned @@ -178,115 +189,23 @@ macro_rules! godot_wrap_method { $type_name:ty, $is_deref_return:ident, fn $method_name:ident( - $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* + $self:ident + , $owner:ident : $owner_ty:ty + $(, $pname:ident : $pty:ty)* + $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? - ) -> $retty:ty + ) $(-> $retty:ty)? ) => { $crate::godot_wrap_method_inner!( $type_name, $is_deref_return, map_owned, fn $method_name( - $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> $retty - ) - }; - // mutable without return type - ( - $type_name:ty, - $is_deref_return:ident, - fn $method_name:ident( - &mut $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* - $(,)? - ) - ) => { - $crate::godot_wrap_method!( - $type_name, - $is_deref_return, - fn $method_name( - &mut $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> () - ) - }; - // immutable without return type - ( - $type_name:ty, - $is_deref_return:ident, - fn $method_name:ident( - & $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* - $(,)? - ) - ) => { - $crate::godot_wrap_method!( - $type_name, - $is_deref_return, - fn $method_name( - & $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> () - ) - }; - // owned without return type - ( - $type_name:ty, - $is_deref_return:ident, - fn $method_name:ident( - mut $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* - $(,)? - ) - ) => { - $crate::godot_wrap_method!( - $type_name, - $is_deref_return, - fn $method_name( - $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> () - ) - }; - // owned without return type - ( - $type_name:ty, - $is_deref_return:ident, - fn $method_name:ident( - $self:ident, - $owner:ident : $owner_ty:ty - $(,$pname:ident : $pty:ty)* - $(,#[opt] $opt_pname:ident : $opt_pty:ty)* - $(,)? - ) - ) => { - $crate::godot_wrap_method!( - $type_name, - $is_deref_return, - fn $method_name( - $self, - $owner: $owner_ty - $(,$pname : $pty)* - $(,#[opt] $opt_pname : $opt_pty)* - ) -> () + $self + , $owner : $owner_ty + $(, $pname : $pty)* + $(, #[opt] $opt_pname : $opt_pty)* + ) -> godot_wrap_method_return_type!($($retty)?) ) }; } From e113a6b970a03ebca6faa2bda33a9460b5f254c4 Mon Sep 17 00:00:00 2001 From: B_head Date: Sat, 30 Apr 2022 19:37:56 +0900 Subject: [PATCH 6/6] Add method attribute Owner argument can be omitted. --- gdnative-core/src/export/macros.rs | 36 +++++++++---- gdnative-derive/src/lib.rs | 2 +- gdnative-derive/src/methods.rs | 83 ++++++++++++++++++++++++------ 3 files changed, 93 insertions(+), 28 deletions(-) diff --git a/gdnative-core/src/export/macros.rs b/gdnative-core/src/export/macros.rs index df11a9372..083cd9e40 100644 --- a/gdnative-core/src/export/macros.rs +++ b/gdnative-core/src/export/macros.rs @@ -22,6 +22,12 @@ macro_rules! godot_wrap_method_if_deref { }; } +// The ways of emit warnings is a terrible hack. +// This is because there is no way to emit warnings from macros in stable Rust. +// +// Follow these steps to emit warnings. +// - Detect whether reference types are used in gdnative-derive::methods::derive_methods(). +// - Expand the call to the deprecated_reference_return!() macro to user code. #[doc(hidden)] #[macro_export] #[deprecated = "This function does not actually pass by reference to the Godot engine. You can clarify by writing #[export(deref_return)]."] @@ -29,6 +35,14 @@ macro_rules! deprecated_reference_return { () => {}; } +#[doc(hidden)] +#[macro_export] +macro_rules! godot_wrap_method_void { + ($ident:ident, $void:tt) => { + $ident + }; +} + #[doc(hidden)] #[macro_export] macro_rules! godot_wrap_method_inner { @@ -38,7 +52,7 @@ macro_rules! godot_wrap_method_inner { $map_method:ident, fn $method_name:ident( $self:ident - , $owner:ident : $owner_ty:ty + $(, #[base] $base:ident : $base_ty:ty)? $(, $pname:ident : $pty:ty)* $(, #[opt] $opt_pname:ident : $opt_pty:ty)* ) -> $retty:ty @@ -67,11 +81,11 @@ macro_rules! godot_wrap_method_inner { Args { $($pname,)* $($opt_pname,)* }: Args, ) -> $crate::core_types::Variant { this - .$map_method(|__rust_val, $owner| { + .$map_method(|__rust_val, __base| { #[allow(unused_unsafe)] unsafe { let ret = __rust_val.$method_name( - OwnerArg::from_safe_ref($owner), + $(OwnerArg::from_safe_ref($crate::godot_wrap_method_void!(__base,$base)),)? $($pname,)* $($opt_pname,)* ); @@ -118,7 +132,7 @@ macro_rules! godot_wrap_method { $is_deref_return:ident, fn $method_name:ident( &mut $self:ident - , $owner:ident : $owner_ty:ty + $(, #[base] $base:ident : $base_ty:ty)? $(, $pname:ident : $pty:ty)* $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? @@ -130,7 +144,7 @@ macro_rules! godot_wrap_method { map_mut, fn $method_name( $self - , $owner : $owner_ty + $(, #[base] $base : $base_ty)? $(, $pname : $pty)* $(, #[opt] $opt_pname : $opt_pty)* ) -> godot_wrap_method_return_type!($($retty)?) @@ -142,7 +156,7 @@ macro_rules! godot_wrap_method { $is_deref_return:ident, fn $method_name:ident( & $self:ident - , $owner:ident : $owner_ty:ty + $(, #[base] $base:ident : $base_ty:ty)? $(, $pname:ident : $pty:ty)* $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? @@ -154,7 +168,7 @@ macro_rules! godot_wrap_method { map, fn $method_name( $self - , $owner : $owner_ty + $(, #[base] $base : $base_ty)? $(, $pname : $pty)* $(, #[opt] $opt_pname : $opt_pty)* ) -> godot_wrap_method_return_type!($($retty)?) @@ -166,7 +180,7 @@ macro_rules! godot_wrap_method { $is_deref_return:ident, fn $method_name:ident( mut $self:ident - , $owner:ident : $owner_ty:ty + $(, #[base] $base:ident : $base_ty:ty)? $(, $pname:ident : $pty:ty)* $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? @@ -178,7 +192,7 @@ macro_rules! godot_wrap_method { map_owned, fn $method_name( $self - , $owner : $owner_ty + $(, #[base] $base : $base_ty)? $(, $pname : $pty)* $(, #[opt] $opt_pname : $opt_pty)* ) -> godot_wrap_method_return_type!($($retty)?) @@ -190,7 +204,7 @@ macro_rules! godot_wrap_method { $is_deref_return:ident, fn $method_name:ident( $self:ident - , $owner:ident : $owner_ty:ty + $(, #[base] $base:ident : $base_ty:ty)? $(, $pname:ident : $pty:ty)* $(, #[opt] $opt_pname:ident : $opt_pty:ty)* $(,)? @@ -202,7 +216,7 @@ macro_rules! godot_wrap_method { map_owned, fn $method_name( $self - , $owner : $owner_ty + $(, #[base] $base : $base_ty)? $(, $pname : $pty)* $(, #[opt] $opt_pname : $opt_pty)* ) -> godot_wrap_method_return_type!($($retty)?) diff --git a/gdnative-derive/src/lib.rs b/gdnative-derive/src/lib.rs index e37c3c006..c3b2e3a7d 100644 --- a/gdnative-derive/src/lib.rs +++ b/gdnative-derive/src/lib.rs @@ -50,7 +50,7 @@ mod variant; /// impl gdnative::export::NativeClassMethods for Foo { /// fn register(builder: &ClassBuilder) { /// use gdnative::export::*; -/// builder.method("foo", gdnative::export::godot_wrap_method!(Foo, false, fn foo(&self, _owner: &Reference, bar: i64) -> i64)) +/// builder.method("foo", gdnative::export::godot_wrap_method!(Foo, false, fn foo(&self, #[base] _owner: &Reference, bar: i64) -> i64)) /// .with_rpc_mode(RpcMode::Disabled) /// .done_stateless(); /// } diff --git a/gdnative-derive/src/methods.rs b/gdnative-derive/src/methods.rs index 6671ac1d6..84470e1ed 100644 --- a/gdnative-derive/src/methods.rs +++ b/gdnative-derive/src/methods.rs @@ -60,10 +60,12 @@ pub(crate) struct ExportMethod { pub(crate) sig: Signature, pub(crate) export_args: ExportArgs, pub(crate) optional_args: Option, + pub(crate) exist_base_arg: bool, } #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Debug, Default)] pub(crate) struct ExportArgs { + pub(crate) is_old_syntax: bool, pub(crate) rpc_mode: Option, pub(crate) name_override: Option, pub(crate) is_deref_return: bool, @@ -80,7 +82,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { let methods = export .methods .into_iter() - .map(|ExportMethod { sig, export_args , optional_args}| { + .map(|ExportMethod { sig, export_args, optional_args, exist_base_arg}| { let sig_span = sig.ident.span(); let name = sig.ident; @@ -93,10 +95,18 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { let arg_count = sig.inputs.len(); - if arg_count < 2 { + if arg_count == 0 { return syn::Error::new( sig_span, - "exported methods must take self and owner as arguments", + "exported methods must take self parameter", + ) + .to_compile_error(); + } + + if export_args.is_old_syntax && !exist_base_arg { + return syn::Error::new( + sig_span, + "exported methods must take second parameter", ) .to_compile_error(); } @@ -106,7 +116,7 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { let max_optional = arg_count - 2; // self and owner if count > max_optional { let message = format!( - "there can be at most {} optional arguments, got {}", + "there can be at most {} optional parameters, got {}", max_optional, count, ); return syn::Error::new(sig_span, message).to_compile_error(); @@ -121,13 +131,17 @@ pub(crate) fn derive_methods(item_impl: ItemImpl) -> TokenStream2 { let args = sig.inputs.iter().enumerate().map(|(n, arg)| { let span = arg.span(); - if n < arg_count - optional_args { + if exist_base_arg && n == 1 { + quote_spanned!(span => #[base] #arg ,) + } + else if n < arg_count - optional_args { quote_spanned!(span => #arg ,) } else { quote_spanned!(span => #[opt] #arg ,) } }); + // See gdnative-core::export::deprecated_reference_return!() let deprecated = if let syn::ReturnType::Type(_, ty) = &sig.output { if !is_deref_return && matches!(**ty, syn::Type::Reference(_)) { quote_spanned!(ret_span=> ::gdnative::export::deprecated_reference_return!();) @@ -208,9 +222,23 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { .last() .map(|i| i.ident.to_string()); - if let Some("export") = last_seg.as_deref() { - let mut _export_args = export_args.get_or_insert_with(ExportArgs::default); + let (is_export, is_old_syntax) = if let Some("export") = last_seg.as_deref() + { + (true, true) + } else if let Some("method") = last_seg.as_deref() { + (true, false) + } else { + (false, false) + }; + + if is_export { use syn::{punctuated::Punctuated, Lit, Meta, NestedMeta}; + let mut export_args = + export_args.get_or_insert_with(ExportArgs::default); + export_args.is_old_syntax = is_old_syntax; + + // Codes like #[macro(path, name = "value")] are accepted. + // Codes like #[path], #[name = "value"] or #[macro("lit")] are not accepted. let nested_meta_iter = match attr.parse_meta() { Err(err) => { errors.push(err); @@ -260,7 +288,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { Some(Lit::Str(str)) => { let value = str.value(); if let Some(mode) = RpcMode::parse(value.as_str()) { - if _export_args.rpc_mode.replace(mode).is_some() { + if export_args.rpc_mode.replace(mode).is_some() { errors.push(syn::Error::new( nested_meta.span(), "rpc mode was set more than once", @@ -290,7 +318,11 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { )); } Some(Lit::Str(str)) => { - if _export_args.name_override.replace(str.value()).is_some() { + if export_args + .name_override + .replace(str.value()) + .is_some() + { errors.push(syn::Error::new( nested_meta.span(), "name was set more than once", @@ -311,13 +343,13 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { nested_meta.span(), "value for deref_return parameter is not valid", )); - } else if _export_args.is_deref_return { + } else if export_args.is_deref_return { errors.push(syn::Error::new( nested_meta.span(), "deref_return was apply more than once", )); } else { - _export_args.is_deref_return = true; + export_args.is_deref_return = true; } } else { let msg = format!( @@ -336,6 +368,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { if let Some(export_args) = export_args.take() { let mut optional_args = None; + let mut exist_base_arg = false; for (n, arg) in method.sig.inputs.iter_mut().enumerate() { let attrs = match arg { @@ -344,32 +377,49 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { }; let mut is_optional = false; + let mut is_base = false; attrs.retain(|attr| { if attr.path.is_ident("opt") { is_optional = true; false + } else if attr.path.is_ident("base") { + is_base = true; + false } else { true } }); + // In the old syntax, the second parameter is always the base parameter. + if export_args.is_old_syntax && n == 1 { + is_base = true; + } + if is_optional { if n < 2 { errors.push(syn::Error::new( arg.span(), "self or owner cannot be optional", )); - continue; + } else { + *optional_args.get_or_insert(0) += 1; } - - *optional_args.get_or_insert(0) += 1; } else if optional_args.is_some() { errors.push(syn::Error::new( arg.span(), "cannot add required parameters after optional ones", )); - continue; + } + + if is_base { + exist_base_arg = true; + if n != 1 { + errors.push(syn::Error::new( + arg.span(), + "base must be the second parameter.", + )); + } } } @@ -377,6 +427,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { sig: method.sig.clone(), export_args, optional_args, + exist_base_arg, }); } @@ -423,7 +474,7 @@ fn impl_gdnative_expose(ast: ItemImpl) -> (ItemImpl, ClassMethodExport) { continue; } - // remove "mut" from arguments. + // remove "mut" from parameters. // give every wildcard a (hopefully) unique name. method .sig