[1/2] object: recommend a few type check macros

I sometime regret that we have to resort to long
object{_class}_dynamic_cast() calls instead of having a shorter and
more readable macros available, similar to the one recommended by
GObject (https://developer.gnome.org/gobject/stable/gtype-conventions.html).

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qom/object.h | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

On Fri, Sep 21, 2018 at 03:13:31PM +0400, Marc-André Lureau wrote:
> I sometime regret that we have to resort to long
> object{_class}_dynamic_cast() calls instead of having a shorter and
> more readable macros available, similar to the one recommended by
> GObject (https://developer.gnome.org/gobject/stable/gtype-conventions.html).
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Nice addition.

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

I wonder if we could provide a tool to help us generate and/or
validate all this boilerplate code for new and/or existing types.


> ---
>  include/qom/object.h | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index f0b0bf39cc..c16e0bc91e 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -101,24 +101,31 @@ typedef struct InterfaceInfo InterfaceInfo;
>   *   </programlisting>
>   * </example>
>   *
> - * Every type has an #ObjectClass associated with it.  #ObjectClass derivatives
> - * are instantiated dynamically but there is only ever one instance for any
> - * given type.  The #ObjectClass typically holds a table of function pointers
> - * for the virtual methods implemented by this type.
> + * Every type has an #ObjectClass associated with it.  #ObjectClass
> + * derivatives are instantiated dynamically but there is only ever one
> + * instance for any given type.  The #ObjectClass typically holds a
> + * table of function pointers for the virtual methods implemented by
> + * this type. You can cast an #ObjectClass to a subclass (or
> + * base-class) type using object_class_dynamic_cast().
>   *
> - * Using object_new(), a new #Object derivative will be instantiated.  You can
> - * cast an #Object to a subclass (or base-class) type using
> - * object_dynamic_cast().  You typically want to define macro wrappers around
> - * OBJECT_CHECK() and OBJECT_CLASS_CHECK() to make it easier to convert to a
> - * specific type:
> + * Using object_new(), a new #Object derivative will be instantiated.
> + * You can cast an #Object to a subclass (or base-class) type using
> + * object_dynamic_cast().
> + *
> + * You typically want to define macro wrappers to make it easier to
> + * handle casting:
>   *
>   * <example>
>   *   <title>Typecasting macros</title>
>   *   <programlisting>
>   *    #define MY_DEVICE_GET_CLASS(obj) \
>   *       OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
> + *    #define IS_MY_DEVICE_CLASS(klass) \
> + *       object_class_dynamic_cast(OBJECT_CLASS(klass), TYPE_MY_DEVICE)
>   *    #define MY_DEVICE_CLASS(klass) \
>   *       OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
> + *    #define IS_MY_DEVICE(obj) \
> + *       object_dynamic_cast(OBJECT(obj), TYPE_MY_DEVICE)
>   *    #define MY_DEVICE(obj) \
>   *       OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
>   *   </programlisting>
> -- 
> 2.19.0
>

On 21 September 2018 at 12:13, Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
> I sometime regret that we have to resort to long
> object{_class}_dynamic_cast() calls instead of having a shorter and
> more readable macros available, similar to the one recommended by
> GObject (https://developer.gnome.org/gobject/stable/gtype-conventions.html).
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qom/object.h | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index f0b0bf39cc..c16e0bc91e 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -101,24 +101,31 @@ typedef struct InterfaceInfo InterfaceInfo;
>   *   </programlisting>
>   * </example>
>   *
> - * Every type has an #ObjectClass associated with it.  #ObjectClass derivatives
> - * are instantiated dynamically but there is only ever one instance for any
> - * given type.  The #ObjectClass typically holds a table of function pointers
> - * for the virtual methods implemented by this type.
> + * Every type has an #ObjectClass associated with it.  #ObjectClass
> + * derivatives are instantiated dynamically but there is only ever one
> + * instance for any given type.  The #ObjectClass typically holds a
> + * table of function pointers for the virtual methods implemented by
> + * this type. You can cast an #ObjectClass to a subclass (or
> + * base-class) type using object_class_dynamic_cast().
>   *
> - * Using object_new(), a new #Object derivative will be instantiated.  You can
> - * cast an #Object to a subclass (or base-class) type using
> - * object_dynamic_cast().  You typically want to define macro wrappers around
> - * OBJECT_CHECK() and OBJECT_CLASS_CHECK() to make it easier to convert to a
> - * specific type:
> + * Using object_new(), a new #Object derivative will be instantiated.
> + * You can cast an #Object to a subclass (or base-class) type using
> + * object_dynamic_cast().
> + *
> + * You typically want to define macro wrappers to make it easier to
> + * handle casting:
>   *
>   * <example>
>   *   <title>Typecasting macros</title>
>   *   <programlisting>
>   *    #define MY_DEVICE_GET_CLASS(obj) \
>   *       OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
> + *    #define IS_MY_DEVICE_CLASS(klass) \
> + *       object_class_dynamic_cast(OBJECT_CLASS(klass), TYPE_MY_DEVICE)
>   *    #define MY_DEVICE_CLASS(klass) \
>   *       OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
> + *    #define IS_MY_DEVICE(obj) \
> + *       object_dynamic_cast(OBJECT(obj), TYPE_MY_DEVICE)
>   *    #define MY_DEVICE(obj) \
>   *       OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
>   *   </programlisting>

I think in general this is a good idea, but I have a couple of
questions:

(1) is this new macro going to become one of the "standard set"
that every QOM object type should provide, like FOO_CLASS
and FOO_GET_CLASS, as noted in
https://wiki.qemu.org/Documentation/QOMConventions
or is it an "on-demand, added to classes where somebody cares" ?

(2) Naming. IS_FOO() implies a boolean return, but
object_dynamic_cast() is "return NULL, or a suitably cast
pointer to the object". (An awful lot of our actual uses
in tree really only seem to want the bool, but I think the
idea was to avoid having the overhead of doing the cast
operation twice.)

thanks
-- PMM

Hi

On Thu, Sep 27, 2018 at 4:42 PM Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On 21 September 2018 at 12:13, Marc-André Lureau
> <marcandre.lureau@redhat.com> wrote:
> > I sometime regret that we have to resort to long
> > object{_class}_dynamic_cast() calls instead of having a shorter and
> > more readable macros available, similar to the one recommended by
> > GObject (https://developer.gnome.org/gobject/stable/gtype-conventions.html).
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  include/qom/object.h | 25 ++++++++++++++++---------
> >  1 file changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/include/qom/object.h b/include/qom/object.h
> > index f0b0bf39cc..c16e0bc91e 100644
> > --- a/include/qom/object.h
> > +++ b/include/qom/object.h
> > @@ -101,24 +101,31 @@ typedef struct InterfaceInfo InterfaceInfo;
> >   *   </programlisting>
> >   * </example>
> >   *
> > - * Every type has an #ObjectClass associated with it.  #ObjectClass derivatives
> > - * are instantiated dynamically but there is only ever one instance for any
> > - * given type.  The #ObjectClass typically holds a table of function pointers
> > - * for the virtual methods implemented by this type.
> > + * Every type has an #ObjectClass associated with it.  #ObjectClass
> > + * derivatives are instantiated dynamically but there is only ever one
> > + * instance for any given type.  The #ObjectClass typically holds a
> > + * table of function pointers for the virtual methods implemented by
> > + * this type. You can cast an #ObjectClass to a subclass (or
> > + * base-class) type using object_class_dynamic_cast().
> >   *
> > - * Using object_new(), a new #Object derivative will be instantiated.  You can
> > - * cast an #Object to a subclass (or base-class) type using
> > - * object_dynamic_cast().  You typically want to define macro wrappers around
> > - * OBJECT_CHECK() and OBJECT_CLASS_CHECK() to make it easier to convert to a
> > - * specific type:
> > + * Using object_new(), a new #Object derivative will be instantiated.
> > + * You can cast an #Object to a subclass (or base-class) type using
> > + * object_dynamic_cast().
> > + *
> > + * You typically want to define macro wrappers to make it easier to
> > + * handle casting:
> >   *
> >   * <example>
> >   *   <title>Typecasting macros</title>
> >   *   <programlisting>
> >   *    #define MY_DEVICE_GET_CLASS(obj) \
> >   *       OBJECT_GET_CLASS(MyDeviceClass, obj, TYPE_MY_DEVICE)
> > + *    #define IS_MY_DEVICE_CLASS(klass) \
> > + *       object_class_dynamic_cast(OBJECT_CLASS(klass), TYPE_MY_DEVICE)
> >   *    #define MY_DEVICE_CLASS(klass) \
> >   *       OBJECT_CLASS_CHECK(MyDeviceClass, klass, TYPE_MY_DEVICE)
> > + *    #define IS_MY_DEVICE(obj) \
> > + *       object_dynamic_cast(OBJECT(obj), TYPE_MY_DEVICE)
> >   *    #define MY_DEVICE(obj) \
> >   *       OBJECT_CHECK(MyDevice, obj, TYPE_MY_DEVICE)
> >   *   </programlisting>
>
> I think in general this is a good idea, but I have a couple of
> questions:
>
> (1) is this new macro going to become one of the "standard set"
> that every QOM object type should provide, like FOO_CLASS
> and FOO_GET_CLASS, as noted in
> https://wiki.qemu.org/Documentation/QOMConventions
> or is it an "on-demand, added to classes where somebody cares" ?

My initial approach was "on-demand", since it wasn't used widely. Now,
if we do whole-tree conversion (which I am slowly doing ;), then we
may want to have stricter rules.

>
> (2) Naming. IS_FOO() implies a boolean return, but
> object_dynamic_cast() is "return NULL, or a suitably cast
> pointer to the object". (An awful lot of our actual uses
> in tree really only seem to want the bool, but I think the
> idea was to avoid having the overhead of doing the cast
> operation twice.)

yes, there are these two cases.

In my pending conversion tree, I keep the object_dynamic_cast() to
avoid the double cast operation where it make sense. Otherwise, I use
the IS_FOO().

In general the double cast shouldn't be a performance concern anyway
(or we can use static casting, like what is done in CPU() of
qom/cpu.h)

Alternatively, OBJECT_CLASS_CHECK() could be changed to run
object_class_dynamic_cast_assert() only when --enable-qom-cast-debug,
and do static cast in regular builds.

>
> thanks
> -- PMM