From: Wolfgang Bumiller <w.bumiller@proxmox.com>
To: Maximiliano Sandoval <m.sandoval@proxmox.com>
Cc: pdm-devel@lists.proxmox.com
Subject: Re: [pdm-devel] [PATCH proxmox-api-types] generator: Remove needless borrows
Date: Wed, 19 Feb 2025 15:20:34 +0100 [thread overview]
Message-ID: <jzcop4innwagfh6zj55fsjmniqu75xe6ormts7oz42l4vr7ep5@33jooabb7bvx> (raw)
In-Reply-To: <20250219140630.368105-1-m.sandoval@proxmox.com>
NAK:
So this changes
let url = "some &str";
self.0.get(&url);
into
let url = "some &str";
self.0.get(url);
I mentioned an alternative off-list which is a simpler change to the
generator which we can do once edition 2024 lands (which will hit stable
tomorrow...), that is, unconditionally change the use of `url` to `&url`
and turn the `format!` calls into borrows by prepending `&`. In the new
edition, the temporary will be kept alive, which is IMO a very awkward
and unexpected thing to do, but apparently I'm wrong in that way of
thinking 😄
Iow. we'd change:
let url = format!("some url{query}");
self.0.get(&url);
and
let url = "some &str";
self.0.get(&url);
into
let url = &format!("some url{query}");
self.0.get(url);
and
let url = "some &str";
self.0.get(url);
(Note how the `.get(url)` is the same in both cases.)
This way we don't need to keep track of whether the url is owned, and
the generator is already quite messy to look at, so I'd prefer that over
this patch.
On Wed, Feb 19, 2025 at 03:06:30PM +0100, Maximiliano Sandoval wrote:
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> pve-api-types/generator-lib/Schema2Rust.pm | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/pve-api-types/generator-lib/Schema2Rust.pm b/pve-api-types/generator-lib/Schema2Rust.pm
> index 84f36f6..173d66f 100644
> --- a/pve-api-types/generator-lib/Schema2Rust.pm
> +++ b/pve-api-types/generator-lib/Schema2Rust.pm
> @@ -453,6 +453,7 @@ my sub print_method_without_body : prototype($$$$$) {
> }
>
> my $input;
> + my $is_url_owned = 0;
> if (defined($input = $def->{input_type})) {
> print {$out} " params: $input,\n";
> print {$out} ") -> Result<$def->{output_type}, Error> {\n";
> @@ -486,6 +487,7 @@ my sub print_method_without_body : prototype($$$$$) {
> print {$out} " add_query_arg(&mut query, &mut sep, \"$name\", &p_$rust_name);\n";
> }
> }
> + $is_url_owned = 1;
> print {$out} " let url = format!(\"/api2/extjs$def->{url}\{query}\");\n";
> } elsif (defined($input = $def->{input})) {
> for my $arg ($input->@*) {
> @@ -510,6 +512,7 @@ my sub print_method_without_body : prototype($$$$$) {
> print {$out} " add_query_arg(&mut query, &mut sep, \"$name\", &$rust_name);\n";
> }
> }
> + $is_url_owned = 1;
> print {$out} " let url = format!(\"/api2/extjs$def->{url}\{query}\");\n";
> } else {
> print {$out} " let url = \"/api2/extjs$def->{url}\";\n";
> @@ -523,7 +526,8 @@ my sub print_method_without_body : prototype($$$$$) {
> print {$out} " let url = \"/api2/extjs$def->{url}\";\n";
> }
>
> - my $call = return_expr($def, "self.0.$method(&url).await?");
> + my $maybe_borrowed_url = $is_url_owned ? '&url' : 'url';
> + my $call = return_expr($def, "self.0.$method($maybe_borrowed_url).await?");
> print {$out} " $call\n";
>
> print {$out} "}\n\n";
> @@ -554,7 +558,7 @@ my sub print_method_with_body : prototype($$$$$) {
> }
> # print {$out} " // self.login().await?;\n";
> print {$out} " let url = \"/api2/extjs$def->{url}\";\n";
> - my $call = return_expr($def, "self.0.${method}(&url, ¶ms).await?");
> + my $call = return_expr($def, "self.0.${method}(url, ¶ms).await?");
> print {$out} " $call\n";
> print {$out} "}\n\n";
> }
> --
> 2.39.5
_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel
prev parent reply other threads:[~2025-02-19 14:20 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-02-19 14:06 Maximiliano Sandoval
2025-02-19 14:20 ` Wolfgang Bumiller [this message]
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=jzcop4innwagfh6zj55fsjmniqu75xe6ormts7oz42l4vr7ep5@33jooabb7bvx \
--to=w.bumiller@proxmox.com \
--cc=m.sandoval@proxmox.com \
--cc=pdm-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