Re: [PATCH 5/9] rust: use MaybeUninit::zeroed() in const context

[Manos Pitsidianakis <manos.pitsidianakis@linaro.org>]

On Fri, May 2, 2025 at 3:23 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 5/2/25 13:01, Manos Pitsidianakis wrote:
> > On Fri, 02 May 2025 13:23, Paolo Bonzini <pbonzini@redhat.com> wrote:
> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> >> ---
> >> docs/devel/rust.rst              |   4 --
> >> rust/hw/timer/hpet/src/fw_cfg.rs |   6 +-
> >> rust/qemu-api/src/zeroable.rs    | 104 +++++--------------------------
> >> 3 files changed, 18 insertions(+), 96 deletions(-)
> >
> > Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
> >
> > BTW There's this TODO in qom.rs, ObjectImpl trait
> >
> >>    /// `&mut T`.  TODO: add a bound of
> >>    //[`Zeroable`](crate::zeroable::Zeroable)
> >>    /// to T; this is more easily done once Zeroable does not require a manual
> >>    /// implementation (Rust 1.75.0).
> >>    const CLASS_INIT: fn(&mut Self::Class);
>
> Yes, good point.  When I wrote the TODO, my idea here was to have some
> kind of
>
>         #[derive(Zeroable)]
>
> macro so that the compiler can "confirm" the safety of implementing
> Zeroable by hand.
>
> However, most of the time the class will be just a C-defined class
> (DeviceClass or SysBusDeviceClass), and then it's not even possible to
> add the derive attribute to the declaration.

Yes this makes sense. A benefit of having `#[derive(Zeroable)]` is
avoiding blanket unsafe impls of the trait, since the derive macro
would work only if all fields implement Zeroable (so in a sense, it's
safe as long as the individual field type impls are safe,
transitively). And we can still do manual unsafe impls for everything
from the FFI boundary.

>  So adding the bound to
> ObjectType::Class is feasible now that one can just add
>
>      unsafe impl Zeroable for bindings::ObjectClass {}
>      unsafe impl Zeroable for bindings::DeviceClass {}
>      unsafe impl Zeroable for bindings::SysBusDeviceClass {}
>      unsafe impl Zeroable for bindings::ResettableClass {}
>
> etc.; in fact it was already possible when Zhao added the impl_zeroed!
> macro in commit aaf3778baaa ("rust/zeroable: Implement Zeroable with
> const_zero macro", 2025-01-28).
>
> Paolo
>