public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: "Max R. Carrara" <m.carrara@proxmox.com>
Cc: pve-devel@lists.proxmox.com
Subject: Re: [PATCH perlmod 3/4] perlmod, macro: add #[hash] parameter
Date: Thu, 23 Jul 2026 09:54:19 +0200	[thread overview]
Message-ID: <su3rbxq3zxgoc3smgnylww5vh4unug27zqzcz5cf3jc6gmkg2x@x7m367yroage> (raw)
In-Reply-To: <20260720152541.524463-4-m.carrara@proxmox.com>

On Mon, Jul 20, 2026 at 05:25:39PM +0200, Max R. Carrara wrote:
> Like `#[list]`, `#[hash]` allows us to support subs for which any
> number of additional key-value pairs may be passed, which is the same
> as having an `;%` at the end of the Perl prototype.
> 
> The `ParamsIter` type is added as a helper for pairwise iteration that
> makes use of the fact that `ffi::StackIter` is now an
> `ExactSizeIterator`, returning an error if the number of values on the
> stack is odd on creation. It is then directly passed on to
> serde's `MapDeserializer`, mirroring how `#[list]` works with
> `SeqDeserializer`.
> 
> That way we can support additional Perl APIs without new parameters
> breaking the Rust side of the API, just like we can with `#[list]`.
> 
> Also document the `#[hash]` parameter and add corresponding tests
> along the way.
> 
> Signed-off-by: Max R. Carrara <m.carrara@proxmox.com>
> ---
>  perlmod-macro/src/function.rs | 70 +++++++++++++++++++++++++++++++++--
>  perlmod/src/de.rs             | 44 ++++++++++++++++++++++
>  perlmod/src/lib.rs            |  5 +++
>  testlib-tests/01-hello.t      | 46 ++++++++++++++++++++++-
>  testlib/src/lib.rs            | 24 +++++++++++-
>  5 files changed, 183 insertions(+), 6 deletions(-)
> 
> diff --git a/perlmod-macro/src/function.rs b/perlmod-macro/src/function.rs
> index 8b5d689..8388f43 100644
> --- a/perlmod-macro/src/function.rs
> +++ b/perlmod-macro/src/function.rs
> @@ -29,6 +29,11 @@ struct ArgumentAttrs {
>      /// Slurp the remaining arguments (like an `@rest` at the end).
>      /// This requires the parameter to implement `FromIterator<T: Deserialize>`.
>      list: Option<Span>,
> +
> +    /// Slurp the remaining arguments (like a `%hash` at the end).
> +    /// This requires the parameter to implement `FromIterator<(K, V)> where K: Deserialize, V:
> +    /// Deserialize`.
> +    hash: Option<Span>,
>  }
>  
>  impl ArgumentAttrs {
> @@ -41,6 +46,8 @@ impl ArgumentAttrs {
>              self.cv = Some(path.span());
>          } else if path.is_ident("list") {
>              self.list = Some(path.span());
> +        } else if path.is_ident("hash") {
> +            self.hash = Some(path.span());
>          } else {
>              return false;
>          }
> @@ -64,11 +71,12 @@ impl ArgumentAttrs {
>              + self.try_from_ref as usize
>              + self.cv.is_some() as usize
>              + self.list.is_some() as usize
> +            + self.hash.is_some() as usize
>              > 1
>          {
>              bail!(
>                  span,
> -                "`raw` and `try_from_ref`, `cv` and `list` attributes are mutually exclusive"
> +                "`raw`, `try_from_ref`, `cv`, `list`, and `hash` attributes are mutually exclusive"
>              );
>          }
>          Ok(())
> @@ -97,6 +105,10 @@ impl ArgumentAttrs {
>              quote_spanned! { span=>
>                  let #extracted_name = args.map(::perlmod::Value::from);
>              }
> +        } else if self.hash.is_some() {
> +            quote_spanned! { span=>
> +                let #extracted_name = args.map(::perlmod::Value::from);
> +            }
>          } else {
>              quote_spanned! { span=>
>                  let #extracted_name: ::perlmod::Value = match args.next() {
> @@ -148,6 +160,33 @@ impl ArgumentAttrs {
>                      }
>                  };
>              }
> +        } else if self.hash.is_some() {
> +            quote_spanned! { span=>
> +                let #deserialized_name = {
> +                    let _guard = ::perlmod::__private__::InParameterDeserialization::guard();
> +                    match  ::perlmod::de::ParamsIter::new(#extracted_name) {
> +                        Ok(params_iter) => {
> +                            match <#arg_type as ::perlmod::__private__::serde::Deserialize>::deserialize(
> +                                ::perlmod::__private__::serde::de::value::MapDeserializer::new(
> +                                    params_iter
> +                                )
> +                            ) {
> +                                Ok(map) => map,
> +                                Err(err) => {
> +                                    return Err(::perlmod::Value::new_string(&format!("{err:#}\n"))
> +                                        .into_mortal()
> +                                        .into_raw());
> +                                }
> +                            }
> +                        },
> +                        Err(err) => {
> +                            return Err(::perlmod::Value::new_string(&format!("{err:#}\n"))
> +                                .into_mortal()
> +                                .into_raw());
> +                        }
> +                    }
> +                };
> +            }
>          } else {
>              quote_spanned! { span=>
>                  let #deserialized_name: #arg_type =
> @@ -213,6 +252,7 @@ pub fn handle_function(
>      let mut passed_arguments = TokenStream::new();
>      let mut cv_arg_param = TokenStream::new();
>      let mut had_list_param = false;
> +    let mut had_hash_param = false;
>      for arg in &mut func.sig.inputs {
>          let (argument_attrs, pat_ty) = ArgumentAttrs::for_input(arg)?;
>  
> @@ -224,6 +264,14 @@ pub fn handle_function(
>              had_list_param = argument_attrs.list.is_some();
>          }
>  
> +        if had_hash_param {
> +            if let Some(span) = argument_attrs.hash {
> +                bail!(span, "only 1 #[hash] parameter allowed");
> +            }
> +        } else {
> +            had_hash_param = argument_attrs.hash.is_some();
> +        }

This should also check the list param, since trailing @rest and trailing
%rest are mutually exclusive, for nicer error handling.
This could end up as an enum `{ None, List, Hash }` so that
`gen_prototype` doesn't also have 2 independent booleans and matches on
the rest type instead.

Otherwise LGTM.

> +
>          let arg_name = match &*pat_ty.pat {
>              syn::Pat::Ident(ident) => {
>                  if ident.by_ref.is_some() {
> @@ -266,6 +314,8 @@ pub fn handle_function(
>              quote_spanned! { span=> ::perlmod::Value::new_undef(), }
>          } else if argument_attrs.list.is_some() {
>              TokenStream::new()
> +        } else if argument_attrs.hash.is_some() {
> +            TokenStream::new()
>          } else {
>              // only count the trailing options;
>              trailing_options = 0;
> @@ -319,7 +369,7 @@ pub fn handle_function(
>          },
>      };
>  
> -    let finalize_arguments = if !had_list_param {
> +    let finalize_arguments = if !had_list_param && !had_hash_param {
>          let too_many_args_error = syn::LitStr::new(
>              &format!(
>                  "too many parameters for function '{}', (expected {})\n",
> @@ -405,13 +455,20 @@ pub fn handle_function(
>                  func.sig.inputs.len(),
>                  trailing_options,
>                  had_list_param,
> +                had_hash_param,
>              ))
>          }),
>      })
>  }
>  
> -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(
> +    arg_count: usize,
> +    trailing_options: usize,
> +    had_list_param: bool,
> +    had_hash_param: bool,
> +) -> String {
> +    let arg_count =
> +        arg_count - trailing_options - (had_list_param as usize) - (had_hash_param as usize);
>  
>      let mut proto = String::with_capacity(arg_count + trailing_options + 1);
>  
> @@ -426,8 +483,13 @@ fn gen_prototype(arg_count: usize, trailing_options: usize, had_list_param: bool
>          if had_list_param {
>              proto.push('@');
>          }
> +        if had_hash_param {
> +            proto.push('%');
> +        }
>      } else if had_list_param {
>          proto.push_str(";@");
> +    } else if had_hash_param {
> +        proto.push_str(";%");
>      }
>      proto
>  }
> diff --git a/perlmod/src/de.rs b/perlmod/src/de.rs
> index cf1601f..1355d0c 100644
> --- a/perlmod/src/de.rs
> +++ b/perlmod/src/de.rs
> @@ -782,3 +782,47 @@ impl<'de> MapAccess<'de> for RawDeserializer<'_> {
>          }
>      }
>  }
> +
> +pub struct ParamsIter<I> {
> +    inner: I,
> +}
> +
> +impl<I> ParamsIter<I>
> +where
> +    I: Iterator<Item = Value> + ExactSizeIterator,
> +{
> +    pub fn new<T>(into_iter: T) -> Result<Self, Error>
> +    where
> +        T: IntoIterator<Item = I::Item, IntoIter = I>,
> +    {
> +        let iter = into_iter.into_iter();
> +
> +        if iter.len() % 2 != 0 {
> +            Err(Error::new(
> +                "odd number of elements for parameter hash - must be even",
> +            ))
> +        } else {
> +            Ok(Self { inner: iter })
> +        }
> +    }
> +}
> +
> +impl<I> Iterator for ParamsIter<I>
> +where
> +    I: Iterator<Item = Value> + ExactSizeIterator,
> +{
> +    type Item = (Value, Value);
> +
> +    fn next(&mut self) -> Option<Self::Item> {
> +        if let Some(key) = self.inner.next() {
> +            let value = self
> +                .inner
> +                .next()
> +                .expect("expected value for key - odd number of elements in params iterator");
> +
> +            Some((key, value))
> +        } else {
> +            None
> +        }
> +    }
> +}
> diff --git a/perlmod/src/lib.rs b/perlmod/src/lib.rs
> index 83b9d8b..2dd1eb2 100644
> --- a/perlmod/src/lib.rs
> +++ b/perlmod/src/lib.rs
> @@ -119,6 +119,11 @@ pub use perlmod_macro::package;
>  ///   `serde::de::value::SeqDeserializer`.) This causes the prototype to end with `;@` (behaves
>  ///   correctly with `Option` parameters in front of it).
>  ///
> +/// * `#[hash]`: Like `#[list]`, but instead of deserializing from a sequence, the final parameter
> +///   needs to be able to deserialize from a map. (Technically just adapts Perl's argument stack
> +///   into a `serde::de::value::MapDeserializer`.) This causes the prototype to end with `;%`
> +///   (behaves correctly with `Option` parameters in front of it).
> +///
>  /// For an example on making blessed objects, see [`Value::bless_box`](Value::bless_box()).
>  pub use perlmod_macro::export;
>  
> diff --git a/testlib-tests/01-hello.t b/testlib-tests/01-hello.t
> index 8af0d5d..5c757ea 100644
> --- a/testlib-tests/01-hello.t
> +++ b/testlib-tests/01-hello.t
> @@ -1,6 +1,6 @@
>  use v5.36;
>  
> -use Test::More tests => 16;
> +use Test::More tests => 24;
>  
>  use TestLib::Hello;
>  
> @@ -47,3 +47,47 @@ is(
>      'final option unset, empty list',
>  );
>  is(TestLib::Hello::sum_list(50, 51, 52), 153, 'pass a deserialized list');
> +
> +is(
> +    TestLib::Hello::trailing_hash(60, 61 => 62),
> +    'first=60, rest is {61: 62}',
> +    'collecting 1 trailing key-value pair',
> +);
> +is(
> +    TestLib::Hello::trailing_hash_ordered(63, 64 => 65, 66 => 67),
> +    'first=63, rest is {64: 65, 66: 67}',
> +    'collecting 2 trailing key-value pairs',
> +);
> +is(
> +    TestLib::Hello::trailing_hash(67),
> +    'first=67, rest is {}',
> +    'collecting 0 trailing key-value pairs',
> +);
> +
> +is(
> +    eval { TestLib::Hello::trailing_hash(68, 69) } // $@,
> +    "error: odd number of elements for parameter hash - must be even\n",
> +    'failing to collect odd number of trailing hash parameters',
> +);
> +
> +is(
> +    TestLib::Hello::trailing_hash_and_options(80),
> +    'first=80, second=None, rest is {}',
> +    'final option unset, collecting 0 trailing key-value pairs',
> +);
> +is(
> +    TestLib::Hello::trailing_hash_and_options(81, 82),
> +    'first=81, second=Some(82), rest is {}',
> +    'final option set, collecting 0 trailing key-value pairs',
> +);
> +is(
> +    TestLib::Hello::trailing_hash_and_options(81, 82, 83 => 84),
> +    'first=81, second=Some(82), rest is {83: 84}',
> +    'final option set, collecting 1 trailing key-value pairs',
> +);
> +
> +is(
> +    eval { TestLib::Hello::trailing_hash_and_options(85, 86, 87) } // $@,
> +    "error: odd number of elements for parameter hash - must be even\n",
> +    'final option set, collecting 1 trailing key-value pairs',
> +);
> diff --git a/testlib/src/lib.rs b/testlib/src/lib.rs
> index ac3bfde..81879cc 100644
> --- a/testlib/src/lib.rs
> +++ b/testlib/src/lib.rs
> @@ -12,7 +12,10 @@ mod main_lib {}
>  
>  #[perlmod::package(name = "TestLib::Hello", lib = "testlib", boot = "loaded")]
>  mod export {
> -    use std::sync::atomic::{AtomicBool, Ordering};
> +    use std::{
> +        collections::{BTreeMap, HashMap},
> +        sync::atomic::{AtomicBool, Ordering},
> +    };
>  
>      use anyhow::{Error, bail};
>      use serde::{Deserialize, Serialize};
> @@ -137,4 +140,23 @@ mod export {
>      fn sum_list(#[list] rest: Vec<u32>) -> u32 {
>          rest.into_iter().sum()
>      }
> +
> +    #[export]
> +    fn trailing_hash(first: u32, #[hash] rest: HashMap<u32, u32>) -> String {
> +        format!("first={first}, rest is {rest:?}")
> +    }
> +
> +    #[export]
> +    fn trailing_hash_ordered(first: u32, #[hash] rest: BTreeMap<u32, u32>) -> String {
> +        format!("first={first}, rest is {rest:?}")
> +    }
> +
> +    #[export]
> +    fn trailing_hash_and_options(
> +        first: u32,
> +        second: Option<u32>,
> +        #[hash] rest: HashMap<u32, u32>,
> +    ) -> String {
> +        format!("first={first}, second={second:?}, rest is {rest:?}")
> +    }
>  }
> -- 
> 2.47.3




  reply	other threads:[~2026-07-23  7:54 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-20 15:25 [PATCH perlmod 0/4] perlmod: add #[hash] parameter Max R. Carrara
2026-07-20 15:25 ` [PATCH perlmod 1/4] ffi: implement ExactSizeIterator for StackIter Max R. Carrara
2026-07-20 15:25 ` [PATCH perlmod 2/4] value: implement serde::de::IntoDeserializer on Value Max R. Carrara
2026-07-23  7:34   ` Wolfgang Bumiller
2026-07-23  7:50     ` partially-applied: " Wolfgang Bumiller
2026-07-20 15:25 ` [PATCH perlmod 3/4] perlmod, macro: add #[hash] parameter Max R. Carrara
2026-07-23  7:54   ` Wolfgang Bumiller [this message]
2026-07-20 15:25 ` [PATCH perlmod 4/4] testlib: run perltidy Max R. Carrara
2026-07-23  7:55   ` partially-applied: " Wolfgang Bumiller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=su3rbxq3zxgoc3smgnylww5vh4unug27zqzcz5cf3jc6gmkg2x@x7m367yroage \
    --to=w.bumiller@proxmox.com \
    --cc=m.carrara@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal