[PATCH perlmod v3 1/2] macro: function: move signature inputs handling into helper struct

["Max R. Carrara" <m.carrara@proxmox.com>]

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 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 <m.carrara@proxmox.com>
---
 perlmod-macro/src/function.rs | 308 +++++++++++++++++++++-------------
 1 file changed, 195 insertions(+), 113 deletions(-)

diff --git a/perlmod-macro/src/function.rs b/perlmod-macro/src/function.rs
index 3ca7bd0..5bad3a6 100644
--- a/perlmod-macro/src/function.rs
+++ b/perlmod-macro/src/function.rs
@@ -210,6 +210,168 @@ 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<TrailingType>,
+}
+
+impl FnInputs {
+    fn from_signature(
+        signature: &mut syn::Signature,
+        arguments_name: &Ident,
+    ) -> Result<Self, Error> {
+        #[derive(Default)]
+        struct State {
+            trailing_options: usize,
+            extract_arguments: TokenStream,
+            deserialized_arguments: TokenStream,
+            passed_arguments: TokenStream,
+            cv_arg_param: TokenStream,
+            trailing_type: Option<TrailingType>,
+        }
+
+        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 = match arg_attr.pat_type.pat.as_ref() {
+                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
+                }
+                pattern => 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 +417,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 +440,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 +467,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 +483,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,35 +524,42 @@ 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('$');
         }
-        if had_list_param {
-            proto.push('@');
+
+        match trailing_type {
+            Some(TrailingType::List) => proto.push('@'),
+            None => {}
+        }
+    } else {
+        match trailing_type {
+            Some(TrailingType::List) => proto.push_str(";@"),
+            None => {}
         }
-    } else if had_list_param {
-        proto.push_str(";@");
     }
+
     proto
 }
 
@@ -492,16 +572,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<ReturnHandling, Error> {
     let span = name.span();
 
+    let passed_arguments = &inputs.passed_arguments;
+
     let return_type;
     let mut handle_return;
     let wrapper_func;
@@ -511,7 +592,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