Tage Johansson
2023-Aug-02 12:40 UTC
[Libguestfs] [libnbd PATCH v4 04/11] rust: Use more specific closure traits
For closures with `cb,count = CBOnce`, `FnOnce` will be used instead of
`FnMut`. Moreover, closures in synchronous commands with
`cblifetime = CBCommand` will not need to live for the static lifetime.
See [here](https://doc.rust-lang.org/std/ops/trait.FnOnce.html) for more
information about the advantages of using `FnOnce` when possible.
---
generator/Rust.ml | 44 ++++++++++++++++++++++++++++----------------
rust/src/handle.rs | 2 ++
rust/src/types.rs | 2 ++
3 files changed, 32 insertions(+), 16 deletions(-)
diff --git a/generator/Rust.ml b/generator/Rust.ml
index 3117980..d3225eb 100644
--- a/generator/Rust.ml
+++ b/generator/Rust.ml
@@ -111,7 +111,7 @@ let rust_cbarg_name : cbarg -> string = function
| CBArrayAndLen (arg, _) | CBMutable arg -> rust_arg_name arg
(* Get the Rust type for an argument. *)
-let rec rust_arg_type : arg -> string = function
+let rec rust_arg_type ?(async_kind = None) : arg -> string = function
| Bool _ -> "bool"
| Int _ -> "c_int"
| UInt _ -> "c_uint"
@@ -131,15 +131,18 @@ let rec rust_arg_type : arg -> string = function
| BytesOut _ -> "&mut [u8]"
| BytesPersistIn _ -> "&'static [u8]"
| BytesPersistOut _ -> "&'static mut [u8]"
- | Closure { cbargs } -> "impl " ^ rust_closure_trait cbargs
+ | Closure { cbargs; cbcount } -> (
+ match async_kind with
+ | Some _ -> "impl " ^ rust_closure_trait cbargs cbcount
+ | None -> "impl " ^ rust_closure_trait cbargs cbcount
~lifetime:None)
(* Get the Rust closure trait for a callback, That is `Fn*(...) -> ...)`. *)
-and rust_closure_trait ?(lifetime = Some "'static") cbargs :
string +and rust_closure_trait ?(lifetime = Some "'static") cbargs
cbcount : string let rust_cbargs = String.concat ", " (List.map
rust_cbarg_type cbargs)
- and lifetime_constraint - match lifetime with None -> "" |
Some x -> " + " ^ x
- in
- "FnMut(" ^ rust_cbargs ^ ") -> c_int + Send + Sync" ^
lifetime_constraint
+ and closure_type + match cbcount with CBOnce -> "FnOnce" |
CBMany -> "FnMut"
+ and lifetime = match lifetime with None -> "" | Some x ->
" + " ^ x in
+ sprintf "%s(%s) -> c_int + Send + Sync%s" closure_type
rust_cbargs lifetime
(* Get the Rust type for a callback argument. *)
and rust_cbarg_type : cbarg -> string = function
@@ -153,8 +156,8 @@ and rust_cbarg_type : cbarg -> string = function
| CBMutable arg -> "&mut " ^ rust_arg_type arg
(* Get the type of a rust optional argument. *)
-let rust_optarg_type : optarg -> string = function
- | OClosure x -> sprintf "Option<%s>" (rust_arg_type
(Closure x))
+let rust_optarg_type ?(async_kind = None) : optarg -> string = function
+ | OClosure x -> sprintf "Option<%s>" (rust_arg_type
(Closure x) ~async_kind)
| OFlags (name, flags, _) ->
sprintf "Option<%s>" (rust_arg_type (Flags (name,
flags)))
@@ -419,8 +422,8 @@ let ffi_ret_to_rust (call : call) closure data, and a
free function for the closure data. This struct is what
will be sent to a C function taking the closure as an argument. In fact,
the struct itself is generated by rust-bindgen. *)
-let print_rust_closure_to_raw_fn ({ cbname; cbargs } : closure) - let
closure_trait = rust_closure_trait cbargs ~lifetime:None in
+let print_rust_closure_to_raw_fn ({ cbname; cbargs; cbcount } : closure) + let
closure_trait = rust_closure_trait cbargs cbcount ~lifetime:None in
let ffi_cbargs_names = List.flatten (List.map ffi_cbarg_names cbargs) in
let ffi_cbargs_types = List.flatten (List.map ffi_cbarg_types cbargs) in
let rust_cbargs_names = List.map rust_cbarg_name cbargs in
@@ -435,16 +438,24 @@ let print_rust_closure_to_raw_fn ({ cbname; cbargs } :
closure) (List.map2 (sprintf "%s: %s") ffi_cbargs_names
ffi_cbargs_types));
pr " where F: %s\n" closure_trait;
pr " {\n";
- pr " let callback_ptr = data as *mut F;\n";
- pr " let callback = &mut *callback_ptr;\n";
+ (match cbcount with
+ | CBMany ->
+ pr " let callback_ptr = data as *mut F;\n";
+ pr " let callback = &mut *callback_ptr;\n"
+ | CBOnce ->
+ pr " let callback_ptr = data as *mut
Option<F>;\n";
+ pr " let callback_option: &mut Option<F> = &mut
*callback_ptr;\n";
+ pr " let callback: F =
callback_option.take().unwrap();\n");
List.iter ffi_cbargs_to_rust cbargs;
pr " callback(%s)\n" (String.concat ", "
rust_cbargs_names);
pr " }\n";
- pr " let callback_data = Box::into_raw(Box::new(f));\n";
+ pr " let callback_data = Box::into_raw(Box::new(%s));\n"
+ (match cbcount with CBMany -> "f" | CBOnce ->
"Some(f)");
pr " sys::nbd_%s_callback {\n" cbname;
pr " callback: Some(call_closure::<F>),\n";
pr " user_data: callback_data as *mut _,\n";
- pr " free: Some(utils::drop_data::<F>),\n";
+ pr " free: Some(utils::drop_data::<%s>),\n"
+ (match cbcount with CBMany -> "F" | CBOnce ->
"Option<F>");
pr " }\n";
pr "}\n";
pr "\n"
@@ -501,7 +512,8 @@ let print_rust_handle_method ((name, call) : string * call)
let rust_args_names List.map rust_arg_name call.args @ List.map
rust_optarg_name call.optargs
and rust_args_types - List.map rust_arg_type call.args @ List.map
rust_optarg_type call.optargs
+ List.map (rust_arg_type ~async_kind:call.async_kind) call.args
+ @ List.map (rust_optarg_type ~async_kind:call.async_kind) call.optargs
in
let rust_args String.concat ", "
diff --git a/rust/src/handle.rs b/rust/src/handle.rs
index e26755c..269d1ac 100644
--- a/rust/src/handle.rs
+++ b/rust/src/handle.rs
@@ -31,6 +31,8 @@ impl Handle {
if handle.is_null() {
Err(unsafe { Error::Fatal(ErrorKind::get_error().into()) })
} else {
+ // Set a debug callback communicating with any logging
+ // implementation as defined by the log crate.
#[allow(unused_mut)]
let mut nbd = Handle { handle };
#[cfg(feature = "log")]
diff --git a/rust/src/types.rs b/rust/src/types.rs
index eb2df06..af62140 100644
--- a/rust/src/types.rs
+++ b/rust/src/types.rs
@@ -15,4 +15,6 @@
// License along with this library; if not, write to the Free Software
// Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+/// A cookie is a 64 bit integer returned by some aio_* methods on
+/// [crate::Handle] used to identify a running command.
pub struct Cookie(pub(crate) u64);
--
2.41.0
Richard W.M. Jones
2023-Aug-02 16:32 UTC
[Libguestfs] [libnbd PATCH v4 04/11] rust: Use more specific closure traits
On Wed, Aug 02, 2023 at 12:40:49PM +0000, Tage Johansson wrote:> For closures with `cb,count = CBOnce`, `FnOnce` will be used instead ofI think there's an extra ',' here. Previous comments about use of markdown apply too.> `FnMut`. Moreover, closures in synchronous commands with > `cblifetime = CBCommand` will not need to live for the static lifetime. > See [here](https://doc.rust-lang.org/std/ops/trait.FnOnce.html) for more > information about the advantages of using `FnOnce` when possible. > --- > generator/Rust.ml | 44 ++++++++++++++++++++++++++++---------------- > rust/src/handle.rs | 2 ++ > rust/src/types.rs | 2 ++ > 3 files changed, 32 insertions(+), 16 deletions(-) > > diff --git a/generator/Rust.ml b/generator/Rust.ml > index 3117980..d3225eb 100644 > --- a/generator/Rust.ml > +++ b/generator/Rust.ml > @@ -111,7 +111,7 @@ let rust_cbarg_name : cbarg -> string = function > | CBArrayAndLen (arg, _) | CBMutable arg -> rust_arg_name arg > > (* Get the Rust type for an argument. *) > -let rec rust_arg_type : arg -> string = function > +let rec rust_arg_type ?(async_kind = None) : arg -> string = function > | Bool _ -> "bool" > | Int _ -> "c_int" > | UInt _ -> "c_uint" > @@ -131,15 +131,18 @@ let rec rust_arg_type : arg -> string = function > | BytesOut _ -> "&mut [u8]" > | BytesPersistIn _ -> "&'static [u8]" > | BytesPersistOut _ -> "&'static mut [u8]" > - | Closure { cbargs } -> "impl " ^ rust_closure_trait cbargs > + | Closure { cbargs; cbcount } -> ( > + match async_kind with > + | Some _ -> "impl " ^ rust_closure_trait cbargs cbcount > + | None -> "impl " ^ rust_closure_trait cbargs cbcount ~lifetime:None) > > (* Get the Rust closure trait for a callback, That is `Fn*(...) -> ...)`. *) > -and rust_closure_trait ?(lifetime = Some "'static") cbargs : string > +and rust_closure_trait ?(lifetime = Some "'static") cbargs cbcount : string > let rust_cbargs = String.concat ", " (List.map rust_cbarg_type cbargs) > - and lifetime_constraint > - match lifetime with None -> "" | Some x -> " + " ^ x > - in > - "FnMut(" ^ rust_cbargs ^ ") -> c_int + Send + Sync" ^ lifetime_constraint > + and closure_type > + match cbcount with CBOnce -> "FnOnce" | CBMany -> "FnMut" > + and lifetime = match lifetime with None -> "" | Some x -> " + " ^ x in > + sprintf "%s(%s) -> c_int + Send + Sync%s" closure_type rust_cbargs lifetime > > (* Get the Rust type for a callback argument. *) > and rust_cbarg_type : cbarg -> string = function > @@ -153,8 +156,8 @@ and rust_cbarg_type : cbarg -> string = function > | CBMutable arg -> "&mut " ^ rust_arg_type arg > > (* Get the type of a rust optional argument. *) > -let rust_optarg_type : optarg -> string = function > - | OClosure x -> sprintf "Option<%s>" (rust_arg_type (Closure x)) > +let rust_optarg_type ?(async_kind = None) : optarg -> string = function > + | OClosure x -> sprintf "Option<%s>" (rust_arg_type (Closure x) ~async_kind) > | OFlags (name, flags, _) -> > sprintf "Option<%s>" (rust_arg_type (Flags (name, flags))) > > @@ -419,8 +422,8 @@ let ffi_ret_to_rust (call : call) > closure data, and a free function for the closure data. This struct is what > will be sent to a C function taking the closure as an argument. In fact, > the struct itself is generated by rust-bindgen. *) > -let print_rust_closure_to_raw_fn ({ cbname; cbargs } : closure) > - let closure_trait = rust_closure_trait cbargs ~lifetime:None in > +let print_rust_closure_to_raw_fn ({ cbname; cbargs; cbcount } : closure) > + let closure_trait = rust_closure_trait cbargs cbcount ~lifetime:None in > let ffi_cbargs_names = List.flatten (List.map ffi_cbarg_names cbargs) in > let ffi_cbargs_types = List.flatten (List.map ffi_cbarg_types cbargs) in > let rust_cbargs_names = List.map rust_cbarg_name cbargs in > @@ -435,16 +438,24 @@ let print_rust_closure_to_raw_fn ({ cbname; cbargs } : closure) > (List.map2 (sprintf "%s: %s") ffi_cbargs_names ffi_cbargs_types)); > pr " where F: %s\n" closure_trait; > pr " {\n"; > - pr " let callback_ptr = data as *mut F;\n"; > - pr " let callback = &mut *callback_ptr;\n"; > + (match cbcount with > + | CBMany -> > + pr " let callback_ptr = data as *mut F;\n"; > + pr " let callback = &mut *callback_ptr;\n" > + | CBOnce -> > + pr " let callback_ptr = data as *mut Option<F>;\n"; > + pr " let callback_option: &mut Option<F> = &mut *callback_ptr;\n"; > + pr " let callback: F = callback_option.take().unwrap();\n"); > List.iter ffi_cbargs_to_rust cbargs; > pr " callback(%s)\n" (String.concat ", " rust_cbargs_names); > pr " }\n"; > - pr " let callback_data = Box::into_raw(Box::new(f));\n"; > + pr " let callback_data = Box::into_raw(Box::new(%s));\n" > + (match cbcount with CBMany -> "f" | CBOnce -> "Some(f)"); > pr " sys::nbd_%s_callback {\n" cbname; > pr " callback: Some(call_closure::<F>),\n"; > pr " user_data: callback_data as *mut _,\n"; > - pr " free: Some(utils::drop_data::<F>),\n"; > + pr " free: Some(utils::drop_data::<%s>),\n" > + (match cbcount with CBMany -> "F" | CBOnce -> "Option<F>"); > pr " }\n"; > pr "}\n"; > pr "\n" > @@ -501,7 +512,8 @@ let print_rust_handle_method ((name, call) : string * call) > let rust_args_names > List.map rust_arg_name call.args @ List.map rust_optarg_name call.optargs > and rust_args_types > - List.map rust_arg_type call.args @ List.map rust_optarg_type call.optargs > + List.map (rust_arg_type ~async_kind:call.async_kind) call.args > + @ List.map (rust_optarg_type ~async_kind:call.async_kind) call.optargs > in > let rust_args > String.concat ", " > diff --git a/rust/src/handle.rs b/rust/src/handle.rs > index e26755c..269d1ac 100644 > --- a/rust/src/handle.rs > +++ b/rust/src/handle.rs > @@ -31,6 +31,8 @@ impl Handle { > if handle.is_null() { > Err(unsafe { Error::Fatal(ErrorKind::get_error().into()) }) > } else { > + // Set a debug callback communicating with any logging > + // implementation as defined by the log crate. > #[allow(unused_mut)] > let mut nbd = Handle { handle }; > #[cfg(feature = "log")] > diff --git a/rust/src/types.rs b/rust/src/types.rs > index eb2df06..af62140 100644 > --- a/rust/src/types.rs > +++ b/rust/src/types.rs > @@ -15,4 +15,6 @@ > // License along with this library; if not, write to the Free Software > // Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA > > +/// A cookie is a 64 bit integer returned by some aio_* methods on > +/// [crate::Handle] used to identify a running command. > pub struct Cookie(pub(crate) u64);I still feel we're overspecifying the interface in patches 2, 3 & 4, just to avoid using a reference count in the Rust bindings. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW