public inbox for pdm-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pdm-devel] [PATCH proxmox-api-types] generator: Remove needless borrows
@ 2025-02-19 14:06 Maximiliano Sandoval
  2025-02-19 14:20 ` Wolfgang Bumiller
  0 siblings, 1 reply; 2+ messages in thread
From: Maximiliano Sandoval @ 2025-02-19 14:06 UTC (permalink / raw)
  To: pdm-devel

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, &params).await?");
+        my $call = return_expr($def, "self.0.${method}(url, &params).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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [pdm-devel] [PATCH proxmox-api-types] generator: Remove needless borrows
  2025-02-19 14:06 [pdm-devel] [PATCH proxmox-api-types] generator: Remove needless borrows Maximiliano Sandoval
@ 2025-02-19 14:20 ` Wolfgang Bumiller
  0 siblings, 0 replies; 2+ messages in thread
From: Wolfgang Bumiller @ 2025-02-19 14:20 UTC (permalink / raw)
  To: Maximiliano Sandoval; +Cc: pdm-devel

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, &params).await?");
> +        my $call = return_expr($def, "self.0.${method}(url, &params).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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-02-19 14:20 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-19 14:06 [pdm-devel] [PATCH proxmox-api-types] generator: Remove needless borrows Maximiliano Sandoval
2025-02-19 14:20 ` Wolfgang Bumiller

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