From: "Max R. Carrara" <m.carrara@proxmox.com>
To: "Wolfgang Bumiller" <w.bumiller@proxmox.com>
Cc: pve-devel@lists.proxmox.com
Subject: Re: [PATCH perlmod v2 4/5] macro: function: move signature inputs handling into helper struct
Date: Thu, 30 Jul 2026 12:56:00 +0200 [thread overview]
Message-ID: <DKBUNLFD9F9N.1YXTI137097PP@proxmox.com> (raw)
In-Reply-To: <178540701947.33538.12090262967135005646.b4-review@b4>
On Thu Jul 30, 2026 at 12:23 PM CEST, Wolfgang Bumiller wrote:
> On Fri, 24 Jul 2026 16:49:06 +0200, Max R. Carrara <m.carrara@proxmox.com> wrote:
> > 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(
> > [ ... skip 44 lines ... ]
> > + 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();
>
> This binding is unnecessary - let's drop the enclosing block and
> indentation and match directly…
Looking at this again, yeah I agree -- will fix in v3. Thanks!
>
> > + 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"),
>
> … ↑ with `s/_/pattern/` here.
>
> > @@ -451,36 +527,48 @@ pub fn handle_function(
> > [ ... skip 30 lines ... ]
> > - proto.push(';');
> > - for _ in 0..trailing_options {
> > - proto.push('$');
> > +
> > + match (trailing_options, trailing_type) {
> > + (1.., ty) => {
>
> Not too happy about this - the `if` previously was sufficient, and the
> `match` now doesn't do any matching on `trailing_type` - it just rebinds
> it to `ty`, so just more indentation for no gain?
Same as above here; looks odd now that I look at it again. Will also fix
in v3, thanks!
next prev parent reply other threads:[~2026-07-30 10:56 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-24 14:49 [PATCH perlmod v2 0/5] perlmod: add #[hash] parameter attribute Max R. Carrara
2026-07-24 14:49 ` [PATCH perlmod v2 1/5] macro: function: make argument code generation helpers standalone fns Max R. Carrara
2026-07-24 14:49 ` [PATCH perlmod v2 2/5] macro: function: make argument attribute handling more extendable Max R. Carrara
2026-07-24 14:49 ` [PATCH perlmod v2 3/5] macro: function: explicitly pass identifier for generated arg iter Max R. Carrara
2026-07-24 14:49 ` [PATCH perlmod v2 4/5] macro: function: move signature inputs handling into helper struct Max R. Carrara
2026-07-30 10:23 ` Wolfgang Bumiller
2026-07-30 10:56 ` Max R. Carrara [this message]
2026-07-24 14:49 ` [PATCH perlmod v2 5/5] perlmod, macro: add #[hash] parameter attribute Max R. Carrara
2026-07-30 10:23 ` Wolfgang Bumiller
2026-07-30 10:26 ` partially-applied: [PATCH perlmod v2 0/5] perlmod: " Wolfgang Bumiller
2026-07-30 11:12 ` superseded: " Max R. Carrara
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=DKBUNLFD9F9N.1YXTI137097PP@proxmox.com \
--to=m.carrara@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
--cc=w.bumiller@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