[1/2] qom: Rename Object::class into Object::klass

Commit Message

Felix Wu June 24, 2024, 8:43 p.m. UTC

From: Roman Kiryanov <[e-mail]>

'class' is a C++ keyword and it prevents from
using the QEMU headers with a C++ compiler.

Google-Bug-Id: 331190993
Change-Id: I9ab7d2d77edef654a9c7b7cb9cd01795a6ed65a2
Signed-off-by: Felix Wu <[e-mail]>
Signed-off-by: Roman Kiryanov <[e-mail]>
---
 hw/core/qdev-properties-system.c |  2 +-
 include/exec/memory.h            |  2 +-
 include/qom/object.h             |  2 +-
 qom/object.c                     | 90 ++++++++++++++++----------------
 4 files changed, 48 insertions(+), 48 deletions(-)

Comments

Philippe Mathieu-Daudé June 25, 2024, 2:20 a.m. UTC | #1

Hi Felix,

On 24/6/24 22:43, Felix Wu wrote:
> From: Roman Kiryanov <[e-mail]>
> 
> 'class' is a C++ keyword and it prevents from
> using the QEMU headers with a C++ compiler.
> 
> Google-Bug-Id: 331190993

I asked Roman twice about this tag meaning:
https://lore.kernel.org/qemu-devel/[e-mail]/
https://lore.kernel.org/qemu-devel/[e-mail]/
Since you are taking his work, do you mind clarifying?

Please include a cover letter for your series:
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#include-a-meaningful-cover-letter

Also for headers refactors, enabling scripts/git.orderfile helps
reviewers.

Since you are posting different C++ enablement cleanups,
I suggest you add a section in our docs/devel/style.rst
requesting to keep headers C++ compatible, by not using
C++ reserved keywords, etc...
In particular because the mainstream project is not build-testing
for C++, thus we will likely merge patches breaking C++ and
make your life harder. That said, a C++ header smoke-build job
in our CI could help.

> Change-Id: I9ab7d2d77edef654a9c7b7cb9cd01795a6ed65a2
> Signed-off-by: Felix Wu <[e-mail]>
> Signed-off-by: Roman Kiryanov <[e-mail]>
> ---
>   hw/core/qdev-properties-system.c |  2 +-
>   include/exec/memory.h            |  2 +-
>   include/qom/object.h             |  2 +-
>   qom/object.c                     | 90 ++++++++++++++++----------------
>   4 files changed, 48 insertions(+), 48 deletions(-)


> diff --git a/include/qom/object.h b/include/qom/object.h
> index 13d3a655dd..7afdb261a8 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -153,7 +153,7 @@ struct ObjectClass
>   struct Object
>   {
>       /* private: */
> -    ObjectClass *class;
> +    ObjectClass *klass;
>       ObjectFree *free;
>       GHashTable *properties;
>       uint32_t ref;

I suppose the OBJECT_CLASS / OBJECT_CLASS_CHECK / OBJECT_GET_CLASS
macros aren't compiled so "class" isn't a problem there.

Since it isn't worst than our INTERFACE_CLASS() use:

Reviewed-by: Philippe Mathieu-Daudé <[e-mail]>

Regards,

Phil.

Daniel P. Berrangé June 25, 2024, 7:15 a.m. UTC | #2

On Mon, Jun 24, 2024 at 08:43:59PM +0000, Felix Wu wrote:
> From: Roman Kiryanov <[e-mail]>
> 
> 'class' is a C++ keyword and it prevents from
> using the QEMU headers with a C++ compiler.
> 
> Google-Bug-Id: 331190993
> Change-Id: I9ab7d2d77edef654a9c7b7cb9cd01795a6ed65a2

Please remove both of these lines when posting patches. They are
irrelevant to QEMU's git commit history.

> Signed-off-by: Felix Wu <[e-mail]>
> Signed-off-by: Roman Kiryanov <[e-mail]>
> ---
>  hw/core/qdev-properties-system.c |  2 +-
>  include/exec/memory.h            |  2 +-
>  include/qom/object.h             |  2 +-
>  qom/object.c                     | 90 ++++++++++++++++----------------
>  4 files changed, 48 insertions(+), 48 deletions(-)

With regards,
Daniel

Peter Maydell June 25, 2024, 9:23 a.m. UTC | #3

On Tue, 25 Jun 2024 at 03:20, Philippe Mathieu-Daudé <[e-mail]> wrote:
> Since you are posting different C++ enablement cleanups,
> I suggest you add a section in our docs/devel/style.rst
> requesting to keep headers C++ compatible, by not using
> C++ reserved keywords, etc...
>
> In particular because the mainstream project is not build-testing
> for C++, thus we will likely merge patches breaking C++ and
> make your life harder. That said, a C++ header smoke-build job
> in our CI could help.

Unless there's some easy mechanism for contributors to check
that they haven't broken whatever our C++ requirement is,
I don't think we should define it in the style guide.

More generally, we specifically removed the handling we
had for being able to include our headers from C++ source
files. (cf the stuff we added in commit 875df03b221 for
extern "C" blocks and then removed again later). If we're
not bringing that back (and I don't think we should) then
we're not actually trying to have our headers be C++
compatible, so what are we aiming for?

thanks
-- PMM

Daniel P. Berrangé June 25, 2024, 9:36 a.m. UTC | #4

On Tue, Jun 25, 2024 at 10:23:54AM +0100, Peter Maydell wrote:
> On Tue, 25 Jun 2024 at 03:20, Philippe Mathieu-Daudé <[e-mail]> wrote:
> > Since you are posting different C++ enablement cleanups,
> > I suggest you add a section in our docs/devel/style.rst
> > requesting to keep headers C++ compatible, by not using
> > C++ reserved keywords, etc...
> >
> > In particular because the mainstream project is not build-testing
> > for C++, thus we will likely merge patches breaking C++ and
> > make your life harder. That said, a C++ header smoke-build job
> > in our CI could help.
> 
> Unless there's some easy mechanism for contributors to check
> that they haven't broken whatever our C++ requirement is,
> I don't think we should define it in the style guide.
> 
> More generally, we specifically removed the handling we
> had for being able to include our headers from C++ source
> files. (cf the stuff we added in commit 875df03b221 for
> extern "C" blocks and then removed again later). If we're
> not bringing that back (and I don't think we should) then
> we're not actually trying to have our headers be C++
> compatible, so what are we aiming for?

I really dislike the drip-feeding of patches fixing C++ related
problems. As maintainers we've no idea what the end state is,
is this the last patch, or are there another 100 of these patches
to trickle out one at a time. Ultimately from the QEMU maintainer
POV anything related to C++ compatibility is a distraction, given
the general consensus has turned to Rust as the future for QEMU,
not C++.

If we're going to take any C++ compat cleanups as a courtesy to
ease burden of a downstream fork, then I'd like to see a complete
series in one go, so we can sensibly evaluate whether the end
state is something desirable from QEMU's POV.

With regards,
Daniel

Patch

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index f13350b4fb..a6781841af 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -431,7 +431,7 @@  static void set_netdev(Object *obj, Visitor *v, const char *name,
         }
 
         if (peers[i]->info->check_peer_type) {
-            if (!peers[i]->info->check_peer_type(peers[i], obj->class, errp)) {
+            if (!peers[i]->info->check_peer_type(peers[i], obj->klass, errp)) {
                 goto out;
             }
         }
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 2d7c278b9f..e5bd75956e 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1808,7 +1808,7 @@  static inline IOMMUMemoryRegion *memory_region_get_iommu(MemoryRegion *mr)
 static inline IOMMUMemoryRegionClass *memory_region_get_iommu_class_nocheck(
         IOMMUMemoryRegion *iommu_mr)
 {
-    return (IOMMUMemoryRegionClass *) (((Object *)iommu_mr)->class);
+    return (IOMMUMemoryRegionClass *) (((Object *)iommu_mr)->klass);
 }
 
 #define memory_region_is_iommu(mr) (memory_region_get_iommu(mr) != NULL)
diff --git a/include/qom/object.h b/include/qom/object.h
index 13d3a655dd..7afdb261a8 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -153,7 +153,7 @@  struct ObjectClass
 struct Object
 {
     /* private: */
-    ObjectClass *class;
+    ObjectClass *klass;
     ObjectFree *free;
     GHashTable *properties;
     uint32_t ref;
diff --git a/qom/object.c b/qom/object.c
index 157a45c5f8..133cd08763 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -68,7 +68,7 @@  struct TypeImpl
     const char *parent;
     TypeImpl *parent_type;
 
-    ObjectClass *class;
+    ObjectClass *klass;
 
     int num_interfaces;
     InterfaceImpl interfaces[MAX_INTERFACES];
@@ -304,11 +304,11 @@  static void type_initialize_interface(TypeImpl *ti, TypeImpl *interface_type,
     type_initialize(iface_impl);
     g_free((char *)info.name);
 
-    new_iface = (InterfaceClass *)iface_impl->class;
-    new_iface->concrete_class = ti->class;
+    new_iface = (InterfaceClass *)iface_impl->klass;
+    new_iface->concrete_class = ti->klass;
     new_iface->interface_type = interface_type;
 
-    ti->class->interfaces = g_slist_append(ti->class->interfaces, new_iface);
+    ti->klass->interfaces = g_slist_append(ti->klass->interfaces, new_iface);
 }
 
 static void object_property_free(gpointer data)
@@ -329,7 +329,7 @@  static void type_initialize(TypeImpl *ti)
 {
     TypeImpl *parent;
 
-    if (ti->class) {
+    if (ti->klass) {
         return;
     }
 
@@ -350,7 +350,7 @@  static void type_initialize(TypeImpl *ti)
         assert(!ti->instance_finalize);
         assert(!ti->num_interfaces);
     }
-    ti->class = g_malloc0(ti->class_size);
+    ti->klass = g_malloc0(ti->class_size);
 
     parent = type_get_parent(ti);
     if (parent) {
@@ -360,10 +360,10 @@  static void type_initialize(TypeImpl *ti)
 
         g_assert(parent->class_size <= ti->class_size);
         g_assert(parent->instance_size <= ti->instance_size);
-        memcpy(ti->class, parent->class, parent->class_size);
-        ti->class->interfaces = NULL;
+        memcpy(ti->klass, parent->klass, parent->class_size);
+        ti->klass->interfaces = NULL;
 
-        for (e = parent->class->interfaces; e; e = e->next) {
+        for (e = parent->klass->interfaces; e; e = e->next) {
             InterfaceClass *iface = e->data;
             ObjectClass *klass = OBJECT_CLASS(iface);
 
@@ -377,7 +377,7 @@  static void type_initialize(TypeImpl *ti)
                              ti->interfaces[i].typename, parent->name);
                 abort();
             }
-            for (e = ti->class->interfaces; e; e = e->next) {
+            for (e = ti->klass->interfaces; e; e = e->next) {
                 TypeImpl *target_type = OBJECT_CLASS(e->data)->type;
 
                 if (type_is_ancestor(target_type, t)) {
@@ -393,20 +393,20 @@  static void type_initialize(TypeImpl *ti)
         }
     }
 
-    ti->class->properties = g_hash_table_new_full(g_str_hash, g_str_equal, NULL,
+    ti->klass->properties = g_hash_table_new_full(g_str_hash, g_str_equal, NULL,
                                                   object_property_free);
 
-    ti->class->type = ti;
+    ti->klass->type = ti;
 
     while (parent) {
         if (parent->class_base_init) {
-            parent->class_base_init(ti->class, ti->class_data);
+            parent->class_base_init(ti->klass, ti->class_data);
         }
         parent = type_get_parent(parent);
     }
 
     if (ti->class_init) {
-        ti->class_init(ti->class, ti->class_data);
+        ti->class_init(ti->klass, ti->class_data);
     }
 }
 
@@ -554,7 +554,7 @@  static void object_initialize_with_type(Object *obj, size_t size, TypeImpl *type
     g_assert(size >= type->instance_size);
 
     memset(obj, 0, type->instance_size);
-    obj->class = type->class;
+    obj->klass = type->klass;
     object_ref(obj);
     object_class_property_init_all(obj);
     obj->properties = g_hash_table_new_full(g_str_hash, g_str_equal,
@@ -731,7 +731,7 @@  static void object_deinit(Object *obj, TypeImpl *type)
 static void object_finalize(void *data)
 {
     Object *obj = data;
-    TypeImpl *ti = obj->class->type;
+    TypeImpl *ti = obj->klass->type;
 
     object_property_del_all(obj);
     object_deinit(obj, ti);
@@ -912,7 +912,7 @@  Object *object_dynamic_cast(Object *obj, const char *typename)
 Object *object_dynamic_cast_assert(Object *obj, const char *typename,
                                    const char *file, int line, const char *func)
 {
-    trace_object_dynamic_cast_assert(obj ? obj->class->type->name : "(null)",
+    trace_object_dynamic_cast_assert(obj ? obj->klass->type->name : "(null)",
                                      typename, file, line, func);
 
 #ifdef CONFIG_QOM_CAST_DEBUG
@@ -920,7 +920,7 @@  Object *object_dynamic_cast_assert(Object *obj, const char *typename,
     Object *inst;
 
     for (i = 0; obj && i < OBJECT_CLASS_CAST_CACHE; i++) {
-        if (qatomic_read(&obj->class->object_cast_cache[i]) == typename) {
+        if (qatomic_read(&obj->klass->object_cast_cache[i]) == typename) {
             goto out;
         }
     }
@@ -937,10 +937,10 @@  Object *object_dynamic_cast_assert(Object *obj, const char *typename,
 
     if (obj && obj == inst) {
         for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) {
-            qatomic_set(&obj->class->object_cast_cache[i - 1],
-                       qatomic_read(&obj->class->object_cast_cache[i]));
+            qatomic_set(&obj->klass->object_cast_cache[i - 1],
+                       qatomic_read(&obj->klass->object_cast_cache[i]));
         }
-        qatomic_set(&obj->class->object_cast_cache[i - 1], typename);
+        qatomic_set(&obj->klass->object_cast_cache[i - 1], typename);
     }
 
 out:
@@ -971,7 +971,7 @@  ObjectClass *object_class_dynamic_cast(ObjectClass *class,
         return NULL;
     }
 
-    if (type->class->interfaces &&
+    if (type->klass->interfaces &&
             type_is_ancestor(target_type, type_interface)) {
         int found = 0;
         GSList *i;
@@ -996,45 +996,45 @@  ObjectClass *object_class_dynamic_cast(ObjectClass *class,
     return ret;
 }
 
-ObjectClass *object_class_dynamic_cast_assert(ObjectClass *class,
+ObjectClass *object_class_dynamic_cast_assert(ObjectClass *klass,
                                               const char *typename,
                                               const char *file, int line,
                                               const char *func)
 {
     ObjectClass *ret;
 
-    trace_object_class_dynamic_cast_assert(class ? class->type->name : "(null)",
+    trace_object_class_dynamic_cast_assert(klass ? klass->type->name : "(null)",
                                            typename, file, line, func);
 
 #ifdef CONFIG_QOM_CAST_DEBUG
     int i;
 
-    for (i = 0; class && i < OBJECT_CLASS_CAST_CACHE; i++) {
-        if (qatomic_read(&class->class_cast_cache[i]) == typename) {
-            ret = class;
+    for (i = 0; klass && i < OBJECT_CLASS_CAST_CACHE; i++) {
+        if (qatomic_read(&klass->class_cast_cache[i]) == typename) {
+            ret = klass;
             goto out;
         }
     }
 #else
-    if (!class || !class->interfaces) {
-        return class;
+    if (!klass || !klass->interfaces) {
+        return klass;
     }
 #endif
 
-    ret = object_class_dynamic_cast(class, typename);
-    if (!ret && class) {
+    ret = object_class_dynamic_cast(klass, typename);
+    if (!ret && klass) {
         fprintf(stderr, "%s:%d:%s: Object %p is not an instance of type %s\n",
-                file, line, func, class, typename);
+                file, line, func, klass, typename);
         abort();
     }
 
 #ifdef CONFIG_QOM_CAST_DEBUG
-    if (class && ret == class) {
+    if (klass && ret == klass) {
         for (i = 1; i < OBJECT_CLASS_CAST_CACHE; i++) {
-            qatomic_set(&class->class_cast_cache[i - 1],
-                       qatomic_read(&class->class_cast_cache[i]));
+            qatomic_set(&klass->class_cast_cache[i - 1],
+                       qatomic_read(&klass->class_cast_cache[i]));
         }
-        qatomic_set(&class->class_cast_cache[i - 1], typename);
+        qatomic_set(&klass->class_cast_cache[i - 1], typename);
     }
 out:
 #endif
@@ -1043,12 +1043,12 @@  out:
 
 const char *object_get_typename(const Object *obj)
 {
-    return obj->class->type->name;
+    return obj->klass->type->name;
 }
 
 ObjectClass *object_get_class(Object *obj)
 {
-    return obj->class;
+    return obj->klass;
 }
 
 bool object_class_is_abstract(ObjectClass *klass)
@@ -1071,7 +1071,7 @@  ObjectClass *object_class_by_name(const char *typename)
 
     type_initialize(type);
 
-    return type->class;
+    return type->klass;
 }
 
 ObjectClass *module_object_class_by_name(const char *typename)
@@ -1093,9 +1093,9 @@  ObjectClass *module_object_class_by_name(const char *typename)
     return oc;
 }
 
-ObjectClass *object_class_get_parent(ObjectClass *class)
+ObjectClass *object_class_get_parent(ObjectClass *klass)
 {
-    TypeImpl *type = type_get_parent(class->type);
+    TypeImpl *type = type_get_parent(klass->type);
 
     if (!type) {
         return NULL;
@@ -1103,7 +1103,7 @@  ObjectClass *object_class_get_parent(ObjectClass *class)
 
     type_initialize(type);
 
-    return type->class;
+    return type->klass;
 }
 
 typedef struct OCFData
@@ -1122,7 +1122,7 @@  static void object_class_foreach_tramp(gpointer key, gpointer value,
     ObjectClass *k;
 
     type_initialize(type);
-    k = type->class;
+    k = type->klass;
 
     if (!data->include_abstract && type->abstract) {
         return;
@@ -1792,8 +1792,8 @@  static void object_finalize_child_property(Object *obj, const char *name,
 {
     Object *child = opaque;
 
-    if (child->class->unparent) {
-        (child->class->unparent)(child);
+    if (child->klass->unparent) {
+        (child->klass->unparent)(child);
     }
     child->parent = NULL;
     object_unref(child);