Re: [PATCH for-6.2 06/10] docs: qom: Remove unnecessary class typedefs from example

[Peter Maydell <peter.maydell@linaro.org>]

On Fri, 30 Jul 2021 at 09:18, Markus Armbruster <armbru@redhat.com> wrote:
>
> Eduardo Habkost <ehabkost@redhat.com> writes:
>
> > When there's no specific class struct used for a QOM type, we
> > normally don't define a typedef for it.  Remove the typedef from
> > the minimal example, as it is unnecessary.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> >  docs/devel/qom.rst | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
> > index 05d045bf570..dee60a64c0a 100644
> > --- a/docs/devel/qom.rst
> > +++ b/docs/devel/qom.rst
> > @@ -20,9 +20,6 @@ features:
> >
> >     #define TYPE_MY_DEVICE "my-device"
> >
> > -   // No new virtual functions: we can reuse the typedef for the
> > -   // superclass.
> > -   typedef DeviceClass MyDeviceClass;
> >     typedef struct MyDevice
> >     {
> >         DeviceState parent;
>
> Documenting existing practice makes sense, but I'm not sure the existing
> practice to elide this typedef makes sense.

The QOMConventions page on the wiki
https://wiki.qemu.org/Documentation/QOMConventions
makes what I think is a reasonable distinction:

# If your class (a) will be subclassed or (b) has member fields it needs
# to put in its class struct then you should write all of:
#
# a FOO_CLASS macro
# a FOO_GET_CLASS macro
# a FooClass structure definition containing at least the parent class field
#
# and your TypeInfo for this class should set the .class_size field to
sizeof(FooClass).
#
# These ensure that nothing in future should need changing if new fields are
# added to your class struct, and that any subclasses have the correct typenames
# available so they won't need to change either even if your
implementation changes.
#
# If your class meets neither of the above requirements (ie it is a
simple leaf class) then:
#
# don't provide FOO_CLASS or FOO_GET_CLASS
# don't provide a FooClass structure
# leave the TypeInfo's .class_size field unset.
#
# If a change means a class which didn't provide these macros/types now needs to
# provide them, then your change should add all of them (ie move the class from
# the latter category to the former).

By those principles, we should never do "typedef DeviceClass MyDeviceClass" --
either we have a real MyDeviceClass which contains at least the parent
class field (ie is not a mere typedef), or we don't provide MyDeviceClass
at all.

I would say the rationale for the wiki's distinction is that we don't
want to require unnecessary boilerplate for leaf classes without
methods (which are by far the most common kind of class), but we don't
want a free-for-all regarding how you write things either. So we define
a standard pattern for leaves and a standard pattern for everything else.

-- PMM