[Qemu-devel] [PATCH RFC v2 07/47] qapi: Generate a nicer struct for flat unions

[Markus Armbruster]

The struct generated for a flat union is weird: the members of its
base are at the end, except for the union tag, which is renamed to
'kind' and put at the beginning.

Example: qapi-schema-test.json has

    { 'struct': 'UserDefUnionBase',
      'data': { 'string': 'str', 'enum1': 'EnumOne' } }

    { 'union': 'UserDefFlatUnion',
      'base': 'UserDefUnionBase',
      'discriminator': 'enum1',
      'data': { 'value1' : 'UserDefA',
                'value2' : 'UserDefB',
                'value3' : 'UserDefB' } }

We generate:

    struct UserDefFlatUnion
    {
        EnumOne kind;
        union {
            void *data;
            UserDefA *value1;
            UserDefB *value2;
            UserDefB *value3;
        };
        char *string;
    };

Change to put all base members at the beginning, unadulterated.  Not
only is this easier to understand, it also permits casting the flat
union to its base, if that should become useful.

We now generate:

    struct UserDefFlatUnion
    {
        char *string;
        EnumOne enum1;
        /* union tag is EnumOne enum1 */
        union {
            void *data;
            UserDefA *value1;
            UserDefB *value2;
            UserDefB *value3;
        };
    };

Signed-off-by: Markus Armbruster <address@hidden>
---
 scripts/qapi-types.py           | 32 ++++++++++++++++++--------------
 scripts/qapi-visit.py           |  7 +++++--
 tests/test-qmp-input-visitor.c  |  2 +-
 tests/test-qmp-output-visitor.c |  2 +-
 4 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 4902440..c6c2786 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -200,12 +200,27 @@ def generate_union(expr, meta):
     ret = mcgen('''
 struct %(name)s
 {
+''',
+                name=name)
+    if base:
+        base_fields = find_struct(base)['data']
+        ret += generate_struct_fields(base_fields)
+        ret += mcgen('''
+    /* union tag is %(discriminator_type_name)s %(c_name)s */
+''',
+                     discriminator_type_name=c_name(discriminator_type_name),
+                     c_name=c_name(discriminator))
+    else:
+        assert not discriminator
+        ret += mcgen('''
     %(discriminator_type_name)s kind;
+''',
+                     discriminator_type_name=c_name(discriminator_type_name))
+
+    ret += mcgen('''
     union {
         void *data;
-''',
-                name=name,
-                discriminator_type_name=c_name(discriminator_type_name))
+''')
 
     for key in typeinfo:
         ret += mcgen('''
@@ -216,17 +231,6 @@ struct %(name)s
 
     ret += mcgen('''
     };
-''')
-
-    if base:
-        assert discriminator
-        base_fields = find_struct(base)['data'].copy()
-        del base_fields[discriminator]
-        ret += generate_struct_fields(base_fields)
-    else:
-        assert not discriminator
-
-    ret += mcgen('''
 };
 ''')
     if meta == 'alternate':
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index e8ee268..4ec79a6 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -288,20 +288,23 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, 
const char *name, Error **e
                      name=c_name(name))
 
     if not discriminator:
+        tag = 'kind'
         disc_key = "type"
     else:
+        tag = discriminator
         disc_key = discriminator
     ret += mcgen('''
-        visit_type_%(disc_type)s(m, &(*obj)->kind, "%(disc_key)s", &err);
+        visit_type_%(disc_type)s(m, &(*obj)->%(c_tag)s, "%(disc_key)s", &err);
         if (err) {
             goto out_obj;
         }
         if (!visit_start_union(m, !!(*obj)->data, &err) || err) {
             goto out_obj;
         }
-        switch ((*obj)->kind) {
+        switch ((*obj)->%(c_tag)s) {
 ''',
                  disc_type = disc_type,
+                 c_tag=c_name(tag),
                  disc_key = disc_key)
 
     for key in members:
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index b961953..b7a87ee 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -313,7 +313,7 @@ static void test_visitor_in_union_flat(TestInputVisitorData 
*data,
 
     visit_type_UserDefFlatUnion(v, &tmp, NULL, &err);
     g_assert(err == NULL);
-    g_assert_cmpint(tmp->kind, ==, ENUM_ONE_VALUE1);
+    g_assert_cmpint(tmp->enum1, ==, ENUM_ONE_VALUE1);
     g_assert_cmpstr(tmp->string, ==, "str");
     /* TODO g_assert_cmpint(tmp->integer, ==, 41); */
     g_assert_cmpint(tmp->value1->boolean, ==, true);
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 87ba350..338ada0 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -437,7 +437,7 @@ static void 
test_visitor_out_union_flat(TestOutputVisitorData *data,
     Error *err = NULL;
 
     UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion));
-    tmp->kind = ENUM_ONE_VALUE1;
+    tmp->enum1 = ENUM_ONE_VALUE1;
     tmp->string = g_strdup("str");
     tmp->value1 = g_malloc0(sizeof(UserDefA));
     /* TODO when generator bug is fixed: tmp->integer = 41; */
-- 
1.9.3