diff mbox

[08/26] qapi: Generate a nicer struct for flat unions

Message ID 1438679896-5077-9-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster Aug. 4, 2015, 9:17 a.m. UTC
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 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 enum1;
        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
    {
        /* Members inherited from UserDefUnionBase: */
        char *string;
        EnumOne enum1;
        /* Own members: */
        union { /* union tag is @enum1 */
            void *data;
            UserDefA *value1;
            UserDefB *value2;
            UserDefB *value3;
        };
    };

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi-types.py | 38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

Comments

Eric Blake Aug. 4, 2015, 5 p.m. UTC | #1
On 08/04/2015 03:17 AM, Markus Armbruster wrote:
> 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 at the
> beginning.
> 

> 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.

And I already have a followup patch to your RFCv3 where it IS useful :)

> 
> We now generate:
> 
>     struct UserDefFlatUnion
>     {
>         /* Members inherited from UserDefUnionBase: */
>         char *string;
>         EnumOne enum1;
>         /* Own members: */
>         union { /* union tag is @enum1 */
>             void *data;
>             UserDefA *value1;
>             UserDefB *value2;
>             UserDefB *value3;
>         };
>     };
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  scripts/qapi-types.py | 38 ++++++++++++++++++++++----------------
>  1 file changed, 22 insertions(+), 16 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
diff mbox

Patch

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index ac8dad3..82141cd 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -200,13 +200,30 @@  def generate_union(expr, meta):
     ret = mcgen('''
 struct %(name)s
 {
-    %(discriminator_type_name)s %(discriminator)s;
-    union {
+''',
+                name=name)
+    if base:
+        ret += mcgen('''
+    /* Members inherited from %(c_name)s: */
+''',
+                     c_name=c_name(base))
+        base_fields = find_struct(base)['data']
+        ret += generate_struct_fields(base_fields)
+        ret += mcgen('''
+    /* Own members: */
+''')
+    else:
+        assert not discriminator
+        ret += mcgen('''
+    %(discriminator_type_name)s kind;
+''',
+                     discriminator_type_name=c_name(discriminator_type_name))
+
+    ret += mcgen('''
+    union { /* union tag is @%(c_name)s */
         void *data;
 ''',
-                name=name,
-                discriminator=c_name(discriminator or 'kind'),
-                discriminator_type_name=c_name(discriminator_type_name))
+                 c_name=c_name(discriminator or 'kind'))
 
     for key in typeinfo:
         ret += mcgen('''
@@ -217,17 +234,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':