[RFC,0/2] rust/qemu-api: Rethink property definition macro

Message

Zhao Liu Oct. 17, 2024, 2:32 p.m. UTC

Hi all,

This RFC is try to implement the property definition macro in Rust
style, to take advantage of Rust's language features and to eliminate
some of the extra checking burden of the C version.

This RFC would change the way property definitions are defined, making
the Rust version of the interface less like C.

Junjie and I felt that this was a small enough, as well as special
enough case, to get us thinking together about what changes Rust would
bring to the QEMU interface.

Welcome your comments!


Background
==========

Let's start by reviewing the original property definition macro in C
version.

This is the most basic one, "DEFINE_PROP":

#define DEFINE_PROP(_name, _state, _field, _prop, _type, ...) {  \
        .name      = (_name),                                    \
        .info      = &(_prop),                                   \
        .offset    = offsetof(_state, _field)                    \
            + type_check(_type, typeof_field(_state, _field)),   \
        __VA_ARGS__                                              \
        }

This macro, to ensure correctness, needs to consider two places:

 1. "_field" must be the member name of "_state".
   * checked by typeof_field().

 2. "_field", "_type" and "_prop" must be aligned.
   * Type of "field" is checked by type_check().
   * Various variants of the macro bind "_prop" to "_type".

For Rust version macro, "define_property!", performs the first check by
"offset_of!", but the second requirement is not ensured because it
doesn't check if $type matches $field. This makes the current Rust
version more unsafe than the C version. :)


Solution
========

Of course, we can add checks, such as some tricks similar to those in
patch 2 for type inference.

However, manually binding $type and $prop in macros and checking $field
feels like C style, especially since Rust has type inference and trait.

Therefore, we define a trait for types in QEMU that require properties,
and return the PropertyInfo by trait, like:

pub trait PropertyType {
    fn get_prop_info() -> *const PropertyInfo;
}

impl PropertyType for bool {
    fn get_prop_info() -> *const PropertyInfo {
        unsafe { std::ptr::addr_of!(qdev_prop_bool) }
    }
}

(This is a simplified version, and we spend the extra effort to implement
PropertyTypeImpl, to get $field type in a safer way, pls see patch 2 for
details.)

At the same time, using Rust's type inference, the PropertyInfo is
directly obtained in the macro based on the type of $field.

Therefore, from $field, we can derive $type, and thus $prop, so that
these three things can be naturally aligned, and, in turn, we can
eliminate the $type and $prop parameters, from the Rust macro.

Fewer parameter relationships mean fewer errors.

Then for Rust version, user shouldn't encode PropertyInfo in macro
directly, instead he should implement PropertyType trait.


Opens
=====

But because of the change in the way properties are defined, we need to
think about how to support custom PropertyInfo: this happens when QEMU
needs to support different PropertyInfo's for the same type, and Rust
trait can't implement it multiple times for the same type.

There're 2 options:

1. Introduce a new macro to support $prop parameter, and user could
specify their custom PropertyInfo.

This is aligned with original C version and reverts to the current code
prior to modifications.


2. Don't allow the user to override the PropertyInfo of the type. Instead
user can wrap the type in truple struct and then implement trait for the
new truple type. For example,

struct MyBool(bool);

impl PropertyType for MyBool {
    ...
}

This way is more like Rust style, but users have to add ".0" whenever
they want to access the wrapped property value.


In both cases we allow the user to specify the pointer to PropertyInfo.
But there is no way to check if the provided PropertyInfo is consistent
with the property type (afterall, we haven't implemented the property
registration mechanism in Rust yet), and we have to assume the user will
ensure the consistency. So both approaches have the same level of safety.

Which option do you think is better?

Thanks and Best Regards,
Zhao

---
Junjie Mao (1):
  rust/qemu-api: Fix fragment-specifiers in define_property macro

Zhao Liu (1):
  rust/qemu-api: Bind PropertyInfo to type

 rust/hw/char/pl011/src/device_class.rs |  4 ---
 rust/qemu-api/src/device_class.rs      | 48 +++++++++++++++++++++++---
 rust/qemu-api/src/tests.rs             |  4 ---
 3 files changed, 43 insertions(+), 13 deletions(-)