rust: qom: get rid of ClassInitImpl

Complete the conversion from the ClassInitImpl trait to class_init() methods.
This will provide more freedom to split the qemu_api crate in separate parts.

Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

rust/hw/char/pl011/src/device.rs

@@ -19,8 +19,8 @@ use qemu_api::{
     memory::{hwaddr, MemoryRegion, MemoryRegionOps, MemoryRegionOpsBuilder},
     prelude::*,
     qdev::{Clock, ClockEvent, DeviceImpl, DeviceState, Property, ResetType, ResettablePhasesImpl},
-    qom::{ClassInitImpl, ObjectImpl, Owned, ParentField},
+    qom::{ObjectImpl, Owned, ParentField},
-    sysbus::{SysBusDevice, SysBusDeviceClass, SysBusDeviceImpl},
+    sysbus::{SysBusDevice, SysBusDeviceImpl},
     vmstate::VMStateDescription,
 };
 
@@ -145,7 +145,7 @@ trait PL011Impl: SysBusDeviceImpl + IsA<PL011State> {
 impl PL011Class {
     fn class_init<T: PL011Impl>(&mut self) {
         self.device_id = T::DEVICE_ID;
-        <T as ClassInitImpl<SysBusDeviceClass>>::class_init(&mut self.parent_class);
+        self.parent_class.class_init::<T>();
     }
 }
 

rust/hw/timer/hpet/src/hpet.rs

@@ -21,7 +21,7 @@ use qemu_api::{
     },
     prelude::*,
     qdev::{DeviceImpl, DeviceMethods, DeviceState, Property, ResetType, ResettablePhasesImpl},
-    qom::{ClassInitImpl, ObjectImpl, ObjectType, ParentField},
+    qom::{ObjectImpl, ObjectType, ParentField},
     qom_isa,
     sysbus::{SysBusDevice, SysBusDeviceImpl},
     timer::{Timer, CLOCK_VIRTUAL},
@@ -836,7 +836,7 @@ impl ObjectImpl for HPETState {
 
     const INSTANCE_INIT: Option<unsafe fn(&mut Self)> = Some(Self::init);
     const INSTANCE_POST_INIT: Option<fn(&Self)> = Some(Self::post_init);
-    const CLASS_INIT: fn(&mut Self::Class) = <Self as ClassInitImpl<Self::Class>>::class_init;
+    const CLASS_INIT: fn(&mut Self::Class) = Self::Class::class_init::<Self>;
 }
 
 // TODO: Make these properties user-configurable!

rust/qemu-api/src/qdev.rs

@@ -19,7 +19,7 @@ use crate::{
     chardev::Chardev,
     irq::InterruptSource,
     prelude::*,
-    qom::{ClassInitImpl, ObjectClass, ObjectImpl, Owned},
+    qom::{ObjectClass, ObjectImpl, Owned},
     vmstate::VMStateDescription,
 };
 
@@ -113,7 +113,7 @@ pub trait DeviceImpl: ObjectImpl + ResettablePhasesImpl + IsA<DeviceState> {
 /// # Safety
 ///
 /// This function is only called through the QOM machinery and
-/// used by the `ClassInitImpl<DeviceClass>` trait.
+/// used by `DeviceClass::class_init`.
 /// We expect the FFI user of this function to pass a valid pointer that
 /// can be downcasted to type `T`. We also expect the device is
 /// readable/writeable from one thread at any time.
@@ -127,43 +127,41 @@ unsafe impl InterfaceType for ResettableClass {
         unsafe { CStr::from_bytes_with_nul_unchecked(bindings::TYPE_RESETTABLE_INTERFACE) };
 }
 
-impl<T> ClassInitImpl<ResettableClass> for T
+impl ResettableClass {
-where
+    /// Fill in the virtual methods of `ResettableClass` based on the
-    T: ResettablePhasesImpl,
+    /// definitions in the `ResettablePhasesImpl` trait.
-{
+    pub fn class_init<T: ResettablePhasesImpl>(&mut self) {
-    fn class_init(rc: &mut ResettableClass) {
         if <T as ResettablePhasesImpl>::ENTER.is_some() {
-            rc.phases.enter = Some(rust_resettable_enter_fn::<T>);
+            self.phases.enter = Some(rust_resettable_enter_fn::<T>);
         }
         if <T as ResettablePhasesImpl>::HOLD.is_some() {
-            rc.phases.hold = Some(rust_resettable_hold_fn::<T>);
+            self.phases.hold = Some(rust_resettable_hold_fn::<T>);
         }
         if <T as ResettablePhasesImpl>::EXIT.is_some() {
-            rc.phases.exit = Some(rust_resettable_exit_fn::<T>);
+            self.phases.exit = Some(rust_resettable_exit_fn::<T>);
         }
     }
 }
 
-impl<T> ClassInitImpl<DeviceClass> for T
+impl DeviceClass {
-where
+    /// Fill in the virtual methods of `DeviceClass` based on the definitions in
-    T: ClassInitImpl<ObjectClass> + ClassInitImpl<ResettableClass> + DeviceImpl,
+    /// the `DeviceImpl` trait.
-{
+    pub fn class_init<T: DeviceImpl>(&mut self) {
-    fn class_init(dc: &mut DeviceClass) {
         if <T as DeviceImpl>::REALIZE.is_some() {
-            dc.realize = Some(rust_realize_fn::<T>);
+            self.realize = Some(rust_realize_fn::<T>);
         }
         if let Some(vmsd) = <T as DeviceImpl>::vmsd() {
-            dc.vmsd = vmsd;
+            self.vmsd = vmsd;
         }
         let prop = <T as DeviceImpl>::properties();
         if !prop.is_empty() {
             unsafe {
-                bindings::device_class_set_props_n(dc, prop.as_ptr(), prop.len());
+                bindings::device_class_set_props_n(self, prop.as_ptr(), prop.len());
             }
         }
 
-        ResettableClass::interface_init::<T, DeviceState>(dc);
+        ResettableClass::cast::<DeviceState>(self).class_init::<T>();
-        <T as ClassInitImpl<ObjectClass>>::class_init(&mut dc.parent_class);
+        self.parent_class.class_init::<T>();
     }
 }
 

rust/qemu-api/src/qom.rs

@@ -40,11 +40,6 @@
 //!   The traits have the appropriate specialization of `IsA<>` as a supertrait,
 //!   for example `IsA<DeviceState>` for `DeviceImpl`.
 //!
-//! * an implementation of [`ClassInitImpl`], for example
-//!   `ClassInitImpl<DeviceClass>`. This fills the vtable in the class struct;
-//!   the source for this is the `*Impl` trait; the associated consts and
-//!   functions if needed are wrapped to map C types into Rust types.
-//!
 //! * a trait for instance methods, for example `DeviceMethods`. This trait is
 //!   automatically implemented for any reference or smart pointer to a device
 //!   instance.  It calls into the vtable provides access across all subclasses
@@ -54,6 +49,48 @@
 //!   This provides access to class-wide functionality that doesn't depend on
 //!   instance data. Like instance methods, these are automatically inherited by
 //!   child classes.
+//!
+//! # Class structures
+//!
+//! Each QOM class that has virtual methods describes them in a
+//! _class struct_.  Class structs include a parent field corresponding
+//! to the vtable of the parent class, all the way up to [`ObjectClass`].
+//!
+//! As mentioned above, virtual methods are defined via traits such as
+//! `DeviceImpl`.  Class structs do not define any trait but, conventionally,
+//! all of them have a `class_init` method to initialize the virtual methods
+//! based on the trait and then call the same method on the superclass.
+//!
+//! ```ignore
+//! impl YourSubclassClass
+//! {
+//!     pub fn class_init<T: YourSubclassImpl>(&mut self) {
+//!         ...
+//!         klass.parent_class::class_init<T>();
+//!     }
+//! }
+//! ```
+//!
+//! If a class implements a QOM interface.  In that case, the function must
+//! contain, for each interface, an extra forwarding call as follows:
+//!
+//! ```ignore
+//! ResettableClass::cast::<Self>(self).class_init::<Self>();
+//! ```
+//!
+//! These `class_init` functions are methods on the class rather than a trait,
+//! because the bound on `T` (`DeviceImpl` in this case), will change for every
+//! class struct.  The functions are pointed to by the
+//! [`ObjectImpl::CLASS_INIT`] function pointer. While there is no default
+//! implementation, in most cases it will be enough to write it as follows:
+//!
+//! ```ignore
+//! const CLASS_INIT: fn(&mut Self::Class)> = Self::Class::class_init::<Self>;
+//! ```
+//!
+//! This design incurs a small amount of code duplication but, by not using
+//! traits, it allows the flexibility of implementing bindings in any crate,
+//! without incurring into violations of orphan rules for traits.
 
 use std::{
     ffi::CStr,
@@ -279,19 +316,25 @@ pub unsafe trait InterfaceType: Sized {
     /// for this interface.
     const TYPE_NAME: &'static CStr;
 
-    /// Initialize the vtable for the interface; the generic argument `T` is the
+    /// Return the vtable for the interface; `U` is the type that
-    /// type being initialized, while the generic argument `U` is the type that
     /// lists the interface in its `TypeInfo`.
     ///
+    /// # Examples
+    ///
+    /// This function is usually called by a `class_init` method in `U::Class`.
+    /// For example, `DeviceClass::class_init<T>` initializes its `Resettable`
+    /// interface as follows:
+    ///
+    /// ```ignore
+    /// ResettableClass::cast::<DeviceState>(self).class_init::<T>();
+    /// ```
+    ///
+    /// where `T` is the concrete subclass that is being initialized.
+    ///
     /// # Panics
     ///
     /// Panic if the incoming argument if `T` does not implement the interface.
-    fn interface_init<
+    fn cast<U: ObjectType>(klass: &mut U::Class) -> &mut Self {
-        T: ObjectType + ClassInitImpl<Self> + ClassInitImpl<U::Class>,
-        U: ObjectType,
-    >(
-        klass: &mut U::Class,
-    ) {
         unsafe {
             // SAFETY: upcasting to ObjectClass is always valid, and the
             // return type is either NULL or the argument itself
@@ -300,8 +343,7 @@ pub unsafe trait InterfaceType: Sized {
                 Self::TYPE_NAME.as_ptr(),
             )
             .cast();
-
+            result.as_mut().unwrap()
-            <T as ClassInitImpl<Self>>::class_init(result.as_mut().unwrap())
         }
     }
 }
@@ -558,87 +600,20 @@ pub trait ObjectImpl: ObjectType + IsA<Object> {
     /// the default values coming from the parent classes; the function
     /// can change them to override virtual methods of a parent class.
     ///
-    /// Usually defined as `<Self as ClassInitImpl<Self::Class>::class_init`.
+    /// Usually defined simply as `Self::Class::class_init::<Self>`;
-    const CLASS_INIT: fn(&mut Self::Class);
+    /// however a default implementation cannot be included here, because the
-}
+    /// bounds that the `Self::Class::class_init` method places on `Self` are
-
+    /// not known in advance.
-/// Internal trait used to automatically fill in a class struct.
-///
-/// Each QOM class that has virtual methods describes them in a
-/// _class struct_.  Class structs include a parent field corresponding
-/// to the vtable of the parent class, all the way up to [`ObjectClass`].
-/// Each QOM type has one such class struct; this trait takes care of
-/// initializing the `T` part of the class struct, for the type that
-/// implements the trait.
-///
-/// Each struct will implement this trait with `T` equal to each
-/// superclass.  For example, a device should implement at least
-/// `ClassInitImpl<`[`DeviceClass`](crate::qdev::DeviceClass)`>` and
-/// `ClassInitImpl<`[`ObjectClass`]`>`.  Such implementations are made
-/// in one of two ways.
-///
-/// For most superclasses, `ClassInitImpl` is provided by the `qemu-api`
-/// crate itself.  The Rust implementation of methods will come from a
-/// trait like [`ObjectImpl`] or [`DeviceImpl`](crate::qdev::DeviceImpl),
-/// and `ClassInitImpl` is provided by blanket implementations that
-/// operate on all implementors of the `*Impl`* trait.  For example:
-///
-/// ```ignore
-/// impl<T> ClassInitImpl<DeviceClass> for T
-/// where
-///     T: ClassInitImpl<ObjectClass> + DeviceImpl,
-/// ```
-///
-/// The bound on `ClassInitImpl<ObjectClass>` is needed so that,
-/// after initializing the `DeviceClass` part of the class struct,
-/// the parent [`ObjectClass`] is initialized as well.
-///
-/// The other case is when manual implementation of the trait is needed.
-/// This covers the following cases:
-///
-/// * if a class implements a QOM interface, the Rust code _has_ to define its
-///   own class struct `FooClass` and implement `ClassInitImpl<FooClass>`.
-///   `ClassInitImpl<FooClass>`'s `class_init` method will then forward to
-///   multiple other `class_init`s, for the interfaces as well as the
-///   superclass. (Note that there is no Rust example yet for using interfaces).
-///
-/// * for classes implemented outside the ``qemu-api`` crate, it's not possible
-///   to add blanket implementations like the above one, due to orphan rules. In
-///   that case, the easiest solution is to implement
-///   `ClassInitImpl<YourSuperclass>` for each subclass and not have a
-///   `YourSuperclassImpl` trait at all.
-///
-/// ```ignore
-/// impl ClassInitImpl<YourSuperclass> for YourSubclass {
-///     fn class_init(klass: &mut YourSuperclass) {
-///         klass.some_method = Some(Self::some_method);
-///         <Self as ClassInitImpl<SysBusDeviceClass>>::class_init(&mut klass.parent_class);
-///     }
-/// }
-/// ```
-///
-///   While this method incurs a small amount of code duplication,
-///   it is generally limited to the recursive call on the last line.
-///   This is because classes defined in Rust do not need the same
-///   glue code that is needed when the classes are defined in C code.
-///   You may consider using a macro if you have many subclasses.
-pub trait ClassInitImpl<T> {
-    /// Initialize `klass` to point to the virtual method implementations
-    /// for `Self`.  On entry, the virtual method pointers are set to
-    /// the default values coming from the parent classes; the function
-    /// can change them to override virtual methods of a parent class.
     ///
-    /// The virtual method implementations usually come from another
+    /// # Safety
-    /// trait, for example [`DeviceImpl`](crate::qdev::DeviceImpl)
-    /// when `T` is [`DeviceClass`](crate::qdev::DeviceClass).
     ///
-    /// On entry, `klass`'s parent class is initialized, while the other fields
+    /// While `klass`'s parent class is initialized on entry, the other fields
     /// are all zero; it is therefore assumed that all fields in `T` can be
     /// zeroed, otherwise it would not be possible to provide the class as a
     /// `&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).
-    fn class_init(klass: &mut T);
+    const CLASS_INIT: fn(&mut Self::Class);
 }
 
 /// # Safety
@@ -651,13 +626,12 @@ unsafe extern "C" fn rust_unparent_fn<T: ObjectImpl>(dev: *mut Object) {
     T::UNPARENT.unwrap()(unsafe { state.as_ref() });
 }
 
-impl<T> ClassInitImpl<ObjectClass> for T
+impl ObjectClass {
-where
+    /// Fill in the virtual methods of `ObjectClass` based on the definitions in
-    T: ObjectImpl,
+    /// the `ObjectImpl` trait.
-{
+    pub fn class_init<T: ObjectImpl>(&mut self) {
-    fn class_init(oc: &mut ObjectClass) {
         if <T as ObjectImpl>::UNPARENT.is_some() {
-            oc.unparent = Some(rust_unparent_fn::<T>);
+            self.unparent = Some(rust_unparent_fn::<T>);
         }
     }
 }

rust/qemu-api/src/sysbus.rs

@@ -14,8 +14,8 @@ use crate::{
     irq::{IRQState, InterruptSource},
     memory::MemoryRegion,
     prelude::*,
-    qdev::{DeviceClass, DeviceImpl, DeviceState},
+    qdev::{DeviceImpl, DeviceState},
-    qom::{ClassInitImpl, Owned},
+    qom::Owned,
 };
 
 unsafe impl ObjectType for SysBusDevice {
@@ -28,12 +28,11 @@ qom_isa!(SysBusDevice: DeviceState, Object);
 // TODO: add virtual methods
 pub trait SysBusDeviceImpl: DeviceImpl + IsA<SysBusDevice> {}
 
-impl<T> ClassInitImpl<SysBusDeviceClass> for T
+impl SysBusDeviceClass {
-where
+    /// Fill in the virtual methods of `SysBusDeviceClass` based on the
-    T: SysBusDeviceImpl + ClassInitImpl<DeviceClass>,
+    /// definitions in the `SysBusDeviceImpl` trait.
-{
+    pub fn class_init<T: SysBusDeviceImpl>(self: &mut SysBusDeviceClass) {
-    fn class_init(sdc: &mut SysBusDeviceClass) {
+        self.parent_class.class_init::<T>();
-        <T as ClassInitImpl<DeviceClass>>::class_init(&mut sdc.parent_class);
     }
 }
 

rust/qemu-api/tests/tests.rs

@@ -14,7 +14,7 @@ use qemu_api::{
     declare_properties, define_property,
     prelude::*,
     qdev::{DeviceImpl, DeviceState, Property, ResettablePhasesImpl},
-    qom::{ClassInitImpl, ObjectImpl, ParentField},
+    qom::{ObjectImpl, ParentField},
     sysbus::SysBusDevice,
     vmstate::VMStateDescription,
     zeroable::Zeroable,
@@ -43,7 +43,7 @@ pub struct DummyClass {
 
 impl DummyClass {
     pub fn class_init<T: DeviceImpl>(self: &mut DummyClass) {
-        <T as ClassInitImpl<DeviceClass>>::class_init(&mut self.parent_class);
+        self.parent_class.class_init::<T>();
     }
 }