From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id DCD7E1FF0E6 for ; Fri, 24 Jul 2026 16:50:10 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 5D3E721534; Fri, 24 Jul 2026 16:50:06 +0200 (CEST) From: "Max R. Carrara" To: pve-devel@lists.proxmox.com Subject: [PATCH perlmod v2 4/5] macro: function: move signature inputs handling into helper struct Date: Fri, 24 Jul 2026 16:49:06 +0200 Message-ID: <20260724144914.658730-5-m.carrara@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260724144914.658730-1-m.carrara@proxmox.com> References: <20260724144914.658730-1-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784904532459 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.029 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: ZHHETTCISD7OAK4D4UKD3DOUCJB4OHX5 X-Message-ID-Hash: ZHHETTCISD7OAK4D4UKD3DOUCJB4OHX5 X-MailFrom: m.carrara@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: The handling of function signature inputs adds a lot of mutable state to the body of the `handle_function()` fn, which makes it somewhat hard to track when that state is actually mutated throughout the function. Therefore, introduce the private `FnInputs` struct and heave all of the fn signature input handling into `FnInputs::from_signature()` to limit the scope where mutability occurs. Additionally, use an inline `State` struct inside `FnInputs::from_signature()` in order to make it explicit at a glance when mutable state is in fact mutated. Also introduce the `TrailingType` enum and use it inside `FnInputs` instead of the `had_list_param` boolean in order to make adding such proc macro parameter attributes a little easier in the future. Take the opportunity to improve the overall signature inputs handling logic by making use of some more pattern matching and using `break`s from labeled blocks [1] instead of an if-elseif-else expression when generating the code for the `None` handling. Adapt the `gen_prototype()` function and make it take a reference to `&FnInputs` instead, and also use pattern matching when defining the string for the Perl subroutine prototype. Avoid shadowing the `arg_count` variable too by renaming the parameter to `total_arg_count`. Adapt the `handle_return_kind()` function and make it take a reference to `&FnInputs` as well, removing obsolete parameters in the process. Adapt its body accordingly. [1] https://blog.rust-lang.org/2022/11/03/Rust-1.65.0/#break-from-labeled-blocks Signed-off-by: Max R. Carrara --- perlmod-macro/src/function.rs | 326 ++++++++++++++++++++++------------ 1 file changed, 208 insertions(+), 118 deletions(-) diff --git a/perlmod-macro/src/function.rs b/perlmod-macro/src/function.rs index 3ca7bd0..f5415c5 100644 --- a/perlmod-macro/src/function.rs +++ b/perlmod-macro/src/function.rs @@ -210,6 +210,171 @@ fn deserialized_argument_code( } } +enum TrailingType { + List, +} + +impl TrailingType { + fn as_str(&self) -> &'static str { + match self { + Self::List => "list", + } + } +} + +struct FnInputs { + trailing_options: usize, + extract_arguments: TokenStream, + deserialized_arguments: TokenStream, + passed_arguments: TokenStream, + cv_arg_param: TokenStream, + trailing_type: Option, +} + +impl FnInputs { + fn from_signature( + signature: &mut syn::Signature, + arguments_name: &Ident, + ) -> Result { + #[derive(Default)] + struct State { + trailing_options: usize, + extract_arguments: TokenStream, + deserialized_arguments: TokenStream, + passed_arguments: TokenStream, + cv_arg_param: TokenStream, + trailing_type: Option, + } + + let mut state = State::default(); + + let span = signature.ident.span(); + + for arg in &mut signature.inputs { + let arg_attr = ArgumentAttr::new_from_fn_arg(arg)?; + + let arg_type: &syn::Type = arg_attr.pat_type.ty.as_ref(); + + let arg_name = { + let pattern: &syn::Pat = arg_attr.pat_type.pat.as_ref(); + match pattern { + syn::Pat::Ident(ident) => { + if ident.by_ref.is_some() { + bail!(ident => "xsub does not support by-ref parameters"); + } + if ident.subpat.is_some() { + bail!(ident => "xsub does not support sub-patterns on parameters"); + } + &ident.ident + } + _ => bail!(pattern => "xsub does not support this kind of parameter"), + } + }; + + if let Some(ref attr_type) = arg_attr.attr_type { + match attr_type { + ArgumentAttrType::Raw => {} + ArgumentAttrType::TryFromRef => {} + ArgumentAttrType::CVPtr(cv_span) => { + if !state.cv_arg_param.is_empty() { + bail!(*cv_span, "only 1 'cv' parameter allowed"); + } + + state.cv_arg_param = quote_spanned! { span=> #arg_name: #arg_type }; + if state.passed_arguments.is_empty() { + state + .passed_arguments + .extend(quote_spanned! { span=> #arg_name }); + } else { + state + .passed_arguments + .extend(quote_spanned! { span=> , #arg_name }); + } + + continue; + } + ArgumentAttrType::TrailingList(trailing_span) => { + if let Some(trailing_type) = state.trailing_type { + bail!( + *trailing_span, + "only 1 parameter for trailing values allowed, already got '{}'", + trailing_type.as_str(), + ); + } + + state.trailing_type = Some(TrailingType::List); + } + } + } + + let extracted_name = Ident::new(&format!("extracted_arg_{arg_name}"), arg_name.span()); + let deserialized_name = + Ident::new(&format!("deserialized_arg_{arg_name}"), arg_name.span()); + + let missing_message = syn::LitStr::new( + &format!("missing required parameter: '{arg_name}'\n"), + arg_name.span(), + ); + + let none_handling = 'handling: { + if is_option_type(arg_type).is_some() { + state.trailing_options += 1; + break 'handling quote_spanned! { span=> ::perlmod::Value::new_undef(), }; + } + + if matches!(arg_attr.attr_type, Some(ArgumentAttrType::TrailingList(_))) { + break 'handling TokenStream::new(); + } + + // only count the trailing options + state.trailing_options = 0; + quote_spanned! { span=> + { + return Err(::perlmod::Value::new_string(#missing_message) + .into_mortal() + .into_raw()); + } + } + }; + + let arg_code = extract_argument_code( + &arg_attr, + span, + arguments_name, + &extracted_name, + none_handling, + ); + + let de_arg_code = deserialized_argument_code( + &arg_attr, + span, + arg_type, + &deserialized_name, + extracted_name, + ); + + let passed_arg = if state.passed_arguments.is_empty() { + quote_spanned! { span=> #deserialized_name } + } else { + quote_spanned! { span=> , #deserialized_name } + }; + + state.extract_arguments.extend(arg_code); + state.deserialized_arguments.extend(de_arg_code); + state.passed_arguments.extend(passed_arg) + } + + Ok(Self { + trailing_options: state.trailing_options, + extract_arguments: state.extract_arguments, + deserialized_arguments: state.deserialized_arguments, + passed_arguments: state.passed_arguments, + cv_arg_param: state.cv_arg_param, + trailing_type: state.trailing_type, + }) + } +} + struct Return { result: bool, value: ReturnValue, @@ -255,99 +420,7 @@ pub fn handle_function( let arguments_name = syn::Ident::new("args", name.span()); - let mut trailing_options = 0; - let mut extract_arguments = TokenStream::new(); - let mut deserialized_arguments = TokenStream::new(); - let mut passed_arguments = TokenStream::new(); - let mut cv_arg_param = TokenStream::new(); - let mut had_list_param = false; - for arg in &mut func.sig.inputs { - let arg_attr = ArgumentAttr::new_from_fn_arg(arg)?; - - if let Some(ArgumentAttrType::TrailingList(list_span)) = arg_attr.attr_type { - if had_list_param { - bail!(list_span, "only 1 #[list] parameter allowed"); - } - - had_list_param = true; - } - - let arg_name = match &*arg_attr.pat_type.pat { - syn::Pat::Ident(ident) => { - if ident.by_ref.is_some() { - bail!(ident => "xsub does not support by-ref parameters"); - } - if ident.subpat.is_some() { - bail!(ident => "xsub does not support sub-patterns on parameters"); - } - &ident.ident - } - _ => bail!(&arg_attr.pat_type.pat => "xsub does not support this kind of parameter"), - }; - - let arg_type = &*arg_attr.pat_type.ty; - - if let Some(ArgumentAttrType::CVPtr(cv_span)) = arg_attr.attr_type { - if !cv_arg_param.is_empty() { - bail!(cv_span, "only 1 'cv' parameter allowed"); - } - cv_arg_param = quote_spanned! { span=> #arg_name: #arg_type }; - if passed_arguments.is_empty() { - passed_arguments.extend(quote_spanned! { span=> #arg_name }); - } else { - passed_arguments.extend(quote_spanned! { span=> , #arg_name }); - } - continue; - } - - let extracted_name = Ident::new(&format!("extracted_arg_{arg_name}"), arg_name.span()); - let deserialized_name = - Ident::new(&format!("deserialized_arg_{arg_name}"), arg_name.span()); - - let missing_message = syn::LitStr::new( - &format!("missing required parameter: '{arg_name}'\n"), - arg_name.span(), - ); - - let none_handling = if is_option_type(arg_type).is_some() { - trailing_options += 1; - quote_spanned! { span=> ::perlmod::Value::new_undef(), } - } else if matches!(arg_attr.attr_type, Some(ArgumentAttrType::TrailingList(_))) { - TokenStream::new() - } else { - // only count the trailing options; - trailing_options = 0; - quote_spanned! { span=> - { - return Err(::perlmod::Value::new_string(#missing_message) - .into_mortal() - .into_raw()); - } - } - }; - - extract_arguments.extend(extract_argument_code( - &arg_attr, - span, - &arguments_name, - &extracted_name, - none_handling, - )); - - deserialized_arguments.extend(deserialized_argument_code( - &arg_attr, - span, - arg_type, - &deserialized_name, - extracted_name, - )); - - if passed_arguments.is_empty() { - passed_arguments.extend(quote_spanned! { span=> #deserialized_name }); - } else { - passed_arguments.extend(quote_spanned! { span=> , #deserialized_name }); - } - } + let inputs = FnInputs::from_signature(&mut func.sig, &arguments_name)?; let has_return_value = match &func.sig.output { syn::ReturnType::Default => Return { @@ -370,12 +443,12 @@ pub fn handle_function( }, }; - let finalize_arguments = if !had_list_param { + let finalize_arguments = if !inputs.trailing_type.is_some() { let too_many_args_error = syn::LitStr::new( &format!( "too many parameters for function '{}', (expected {})\n", name, - func.sig.inputs.len() - (!cv_arg_param.is_empty()) as usize + func.sig.inputs.len() - (!inputs.cv_arg_param.is_empty()) as usize ), Span::call_site(), ); @@ -397,13 +470,12 @@ pub fn handle_function( wrapper_func, } = handle_return_kind( &attr, + &inputs, has_return_value, &name, &xs_name, &impl_xs_name, - passed_arguments, export_public, - !cv_arg_param.is_empty(), )?; let visibility_action = check_visibility(&func); @@ -414,6 +486,10 @@ pub fn handle_function( #wrapper_func }; + let cv_arg_param = &inputs.cv_arg_param; + let extract_arguments = &inputs.extract_arguments; + let deserialized_arguments = &inputs.deserialized_arguments; + tokens.extend(quote_spanned! { span=> #[inline(never)] #[allow(non_snake_case)] @@ -451,36 +527,48 @@ pub fn handle_function( perl_name: attr.perl_name, xs_name, tokens, - prototype: attr.prototype.or_else(|| { - Some(gen_prototype( - func.sig.inputs.len(), - trailing_options, - had_list_param, - )) - }), + prototype: attr + .prototype + .or_else(|| Some(gen_prototype(func.sig.inputs.len(), &inputs))), }) } -fn gen_prototype(arg_count: usize, trailing_options: usize, had_list_param: bool) -> String { - let arg_count = arg_count - trailing_options - (had_list_param as usize); +fn gen_prototype(total_arg_count: usize, inputs: &FnInputs) -> String { + let trailing_options = inputs.trailing_options; + let trailing_type = inputs.trailing_type.as_ref(); + + let arg_count = total_arg_count - trailing_options - (trailing_type.is_some() as usize); let mut proto = String::with_capacity(arg_count + trailing_options + 1); for _ in 0..arg_count { proto.push('$'); } - if trailing_options > 0 { - proto.push(';'); - for _ in 0..trailing_options { - proto.push('$'); + + match (trailing_options, trailing_type) { + (1.., ty) => { + proto.push(';'); + + for _ in 0..trailing_options { + proto.push('$'); + } + + match ty { + Some(TrailingType::List) => proto.push('@'), + None => {} + } + + proto } - if had_list_param { - proto.push('@'); + (0, ty) => { + match ty { + Some(TrailingType::List) => proto.push_str(";@"), + None => {} + } + + proto } - } else if had_list_param { - proto.push_str(";@"); } - proto } struct ReturnHandling { @@ -492,16 +580,17 @@ struct ReturnHandling { #[allow(clippy::too_many_arguments)] fn handle_return_kind( attr: &FunctionAttrs, + inputs: &FnInputs, ret: Return, name: &Ident, xs_name: &Ident, impl_xs_name: &Ident, - passed_arguments: TokenStream, export_public: Option<&syn::Visibility>, - cv_arg: bool, ) -> Result { let span = name.span(); + let passed_arguments = &inputs.passed_arguments; + let return_type; let mut handle_return; let wrapper_func; @@ -511,7 +600,8 @@ fn handle_return_kind( None => quote_spanned! { span=> #[allow(non_snake_case)] }, }; - let (cv_arg_name, cv_arg_passed) = if cv_arg { + let has_cv_arg = !inputs.cv_arg_param.is_empty(); + let (cv_arg_name, cv_arg_passed) = if has_cv_arg { ( quote_spanned! { span=> cv }, quote_spanned! { span=> ::perlmod::Value::from_raw_ref(cv as *mut ::perlmod::ffi::SV) }, -- 2.47.3