From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id DE4201FF0E0 for ; Thu, 23 Jul 2026 09:54:25 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 05C0E21493; Thu, 23 Jul 2026 09:54:25 +0200 (CEST) Date: Thu, 23 Jul 2026 09:54:19 +0200 From: Wolfgang Bumiller To: "Max R. Carrara" Subject: Re: [PATCH perlmod 3/4] perlmod, macro: add #[hash] parameter Message-ID: References: <20260720152541.524463-1-m.carrara@proxmox.com> <20260720152541.524463-4-m.carrara@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260720152541.524463-4-m.carrara@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784793230207 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.306 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: JAK2VLT32C3325ZLTRTMM6NYE462JBBR X-Message-ID-Hash: JAK2VLT32C3325ZLTRTMM6NYE462JBBR X-MailFrom: w.bumiller@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 CC: pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 > --- > 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`. > list: Option, > + > + /// 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, > } > > 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 { > + inner: I, > +} > + > +impl ParamsIter > +where > + I: Iterator + ExactSizeIterator, > +{ > + pub fn new(into_iter: T) -> Result > + where > + T: IntoIterator, > + { > + 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 Iterator for ParamsIter > +where > + I: Iterator + ExactSizeIterator, > +{ > + type Item = (Value, Value); > + > + fn next(&mut self) -> Option { > + 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 { > rest.into_iter().sum() > } > + > + #[export] > + fn trailing_hash(first: u32, #[hash] rest: HashMap) -> String { > + format!("first={first}, rest is {rest:?}") > + } > + > + #[export] > + fn trailing_hash_ordered(first: u32, #[hash] rest: BTreeMap) -> String { > + format!("first={first}, rest is {rest:?}") > + } > + > + #[export] > + fn trailing_hash_and_options( > + first: u32, > + second: Option, > + #[hash] rest: HashMap, > + ) -> String { > + format!("first={first}, second={second:?}, rest is {rest:?}") > + } > } > -- > 2.47.3