diff mbox

[v9,08/17] tests: Convert to new qapi union layout

Message ID 1444968943-11254-9-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake Oct. 16, 2015, 4:15 a.m. UTC 
We have two issues with our qapi union layout:
1) Even though the QMP wire format spells the tag 'type', the
C code spells it 'kind', requiring some hacks in the generator.
2) The C struct uses an anonymous union, which places all tag
values in the same namespace as all non-variant members. This
leads to spurious collisions if a tag value matches a QMP name.

Make the conversion to the new layout for testsuite code.

Note that this includes updating an error message regarding a
collision.  After the conversion to the new union qapi layout
is complete, there will still be further patches for cleaning
up collision detection, since the use of a named union can
completely eliminate the collision wording changed here.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v9: new patch, but incorporates parts of v5 31/46 and Markus' RFC:
http://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02236.html
---
 scripts/qapi.py                         |  2 +-
 tests/qapi-schema/union-clash-type.err  |  2 +-
 tests/qapi-schema/union-clash-type.json |  6 +--
 tests/test-qmp-commands.c               |  4 +-
 tests/test-qmp-input-visitor.c          | 78 ++++++++++++++++-----------------
 tests/test-qmp-output-visitor.c         | 42 +++++++++---------
 6 files changed, 66 insertions(+), 68 deletions(-)

Comments

Markus Armbruster Oct. 22, 2015, 2:01 p.m. UTC | #1 
Eric Blake <eblake@redhat.com> writes:

> We have two issues with our qapi union layout:
> 1) Even though the QMP wire format spells the tag 'type', the
> C code spells it 'kind', requiring some hacks in the generator.
> 2) The C struct uses an anonymous union, which places all tag
> values in the same namespace as all non-variant members. This
> leads to spurious collisions if a tag value matches a QMP name.
>
> Make the conversion to the new layout for testsuite code.
>
> Note that this includes updating an error message regarding a
> collision.  After the conversion to the new union qapi layout
> is complete, there will still be further patches for cleaning
> up collision detection, since the use of a named union can
> completely eliminate the collision wording changed here.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v9: new patch, but incorporates parts of v5 31/46 and Markus' RFC:
> http://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02236.html
> ---
>  scripts/qapi.py                         |  2 +-
>  tests/qapi-schema/union-clash-type.err  |  2 +-
>  tests/qapi-schema/union-clash-type.json |  6 +--
>  tests/test-qmp-commands.c               |  4 +-
>  tests/test-qmp-input-visitor.c          | 78 ++++++++++++++++-----------------
>  tests/test-qmp-output-visitor.c         | 42 +++++++++---------
>  6 files changed, 66 insertions(+), 68 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 1e7e08b..aab2b46 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -546,7 +546,7 @@ def check_union(expr, expr_info):
>      base = expr.get('base')
>      discriminator = expr.get('discriminator')
>      members = expr['data']
> -    values = {'MAX': '(automatic)', 'KIND': '(automatic)'}
> +    values = {'MAX': '(automatic)', 'TYPE': '(automatic tag)'}
>
>      # Two types of unions, determined by discriminator.
>

Umm, does this really belong to this patch?

> diff --git a/tests/qapi-schema/union-clash-type.err b/tests/qapi-schema/union-clash-type.err
> index a5dead1..c14bbdd 100644
> --- a/tests/qapi-schema/union-clash-type.err
> +++ b/tests/qapi-schema/union-clash-type.err
> @@ -1 +1 @@
> -tests/qapi-schema/union-clash-type.json:8: Union 'TestUnion' member 'kind' clashes with '(automatic)'
> +tests/qapi-schema/union-clash-type.json:6: Union 'TestUnion' member 'type' clashes with '(automatic tag)'
> diff --git a/tests/qapi-schema/union-clash-type.json b/tests/qapi-schema/union-clash-type.json
> index cfc256b..641b2d5 100644
> --- a/tests/qapi-schema/union-clash-type.json
> +++ b/tests/qapi-schema/union-clash-type.json
> @@ -1,9 +1,7 @@
>  # Union branch 'type'
>  # Reject this, because we would have a clash in generated C, between the
> -# simple union's implicit tag member 'kind' and the branch name 'kind'
> +# simple union's implicit tag member 'type' and the branch name 'type'
>  # within the union.
> -# TODO: Even when the generated C is switched to use 'type' rather than
> -# 'kind', to match the QMP spelling, the collision should still be detected.
> -# Or, we could munge the branch name to allow compilation.
> +# TODO: If desired, we could munge the branch name to allow compilation.

Let's mark it TODO only if we intend to revisit the idea of munging
branch names.

>  { 'union': 'TestUnion',
>    'data': { 'kind': 'int', 'type': 'str' } }
[Mind-numbing mechanical switch to u. and from kind to type...]
Eric Blake Oct. 22, 2015, 2:22 p.m. UTC | #2 
On 10/22/2015 08:01 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> We have two issues with our qapi union layout:
>> 1) Even though the QMP wire format spells the tag 'type', the
>> C code spells it 'kind', requiring some hacks in the generator.
>> 2) The C struct uses an anonymous union, which places all tag
>> values in the same namespace as all non-variant members. This
>> leads to spurious collisions if a tag value matches a QMP name.
>>
>> Make the conversion to the new layout for testsuite code.
>>
>> Note that this includes updating an error message regarding a
>> collision.  After the conversion to the new union qapi layout
>> is complete, there will still be further patches for cleaning
>> up collision detection, since the use of a named union can
>> completely eliminate the collision wording changed here.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>
>> ---
>> v9: new patch, but incorporates parts of v5 31/46 and Markus' RFC:
>> http://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02236.html
>> ---
>>  scripts/qapi.py                         |  2 +-
>>  tests/qapi-schema/union-clash-type.err  |  2 +-
>>  tests/qapi-schema/union-clash-type.json |  6 +--
>>  tests/test-qmp-commands.c               |  4 +-
>>  tests/test-qmp-input-visitor.c          | 78 ++++++++++++++++-----------------
>>  tests/test-qmp-output-visitor.c         | 42 +++++++++---------
>>  6 files changed, 66 insertions(+), 68 deletions(-)
>>
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index 1e7e08b..aab2b46 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -546,7 +546,7 @@ def check_union(expr, expr_info):
>>      base = expr.get('base')
>>      discriminator = expr.get('discriminator')
>>      members = expr['data']
>> -    values = {'MAX': '(automatic)', 'KIND': '(automatic)'}
>> +    values = {'MAX': '(automatic)', 'TYPE': '(automatic tag)'}
>>
>>      # Two types of unions, determined by discriminator.
>>
> 
> Umm, does this really belong to this patch?

Yes, because it cleans up the error message in union-clash-type.err, as
mentioned in the commit message. But since I'm already splitting out the
qapi-visit parts of 7/17, maybe it belongs better in that patch (all
changes to the rest of qapi to deal with the qapi-type parts)?

> 
>> diff --git a/tests/qapi-schema/union-clash-type.err b/tests/qapi-schema/union-clash-type.err
>> index a5dead1..c14bbdd 100644
>> --- a/tests/qapi-schema/union-clash-type.err
>> +++ b/tests/qapi-schema/union-clash-type.err
>> @@ -1 +1 @@
>> -tests/qapi-schema/union-clash-type.json:8: Union 'TestUnion' member 'kind' clashes with '(automatic)'
>> +tests/qapi-schema/union-clash-type.json:6: Union 'TestUnion' member 'type' clashes with '(automatic tag)'

At any rate, this is what improved by adjusting that line of qapi.py.

>> diff --git a/tests/qapi-schema/union-clash-type.json b/tests/qapi-schema/union-clash-type.json
>> index cfc256b..641b2d5 100644
>> --- a/tests/qapi-schema/union-clash-type.json
>> +++ b/tests/qapi-schema/union-clash-type.json
>> @@ -1,9 +1,7 @@
>>  # Union branch 'type'
>>  # Reject this, because we would have a clash in generated C, between the
>> -# simple union's implicit tag member 'kind' and the branch name 'kind'
>> +# simple union's implicit tag member 'type' and the branch name 'type'
>>  # within the union.
>> -# TODO: Even when the generated C is switched to use 'type' rather than
>> -# 'kind', to match the QMP spelling, the collision should still be detected.
>> -# Or, we could munge the branch name to allow compilation.
>> +# TODO: If desired, we could munge the branch name to allow compilation.
> 
> Let's mark it TODO only if we intend to revisit the idea of munging
> branch names.

I have a later patch queued that deletes this test altogether, so for
v10 I'll probably just eliminate changes here for less churn.
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04111.html

> 
>>  { 'union': 'TestUnion',
>>    'data': { 'kind': 'int', 'type': 'str' } }
> [Mind-numbing mechanical switch to u. and from kind to type...]
> 

Yep.  I wish I knew coccinelle well enough to see if it could do the
conversion for me, but I ended up doing it by hand (basically by
applying 16/17 early, then seeing what failed to compile, fixing it up,
then rebasing it into position).
Markus Armbruster Oct. 22, 2015, 2:57 p.m. UTC | #3 
Eric Blake <eblake@redhat.com> writes:

> On 10/22/2015 08:01 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>> 
>>> We have two issues with our qapi union layout:
>>> 1) Even though the QMP wire format spells the tag 'type', the
>>> C code spells it 'kind', requiring some hacks in the generator.
>>> 2) The C struct uses an anonymous union, which places all tag
>>> values in the same namespace as all non-variant members. This
>>> leads to spurious collisions if a tag value matches a QMP name.
>>>
>>> Make the conversion to the new layout for testsuite code.
>>>
>>> Note that this includes updating an error message regarding a
>>> collision.  After the conversion to the new union qapi layout
>>> is complete, there will still be further patches for cleaning
>>> up collision detection, since the use of a named union can
>>> completely eliminate the collision wording changed here.
>>>
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>>
>>> ---
>>> v9: new patch, but incorporates parts of v5 31/46 and Markus' RFC:
>>> http://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02236.html
>>> ---
>>>  scripts/qapi.py                         |  2 +-
>>>  tests/qapi-schema/union-clash-type.err  |  2 +-
>>>  tests/qapi-schema/union-clash-type.json |  6 +--
>>>  tests/test-qmp-commands.c               |  4 +-
>>>  tests/test-qmp-input-visitor.c          | 78 ++++++++++++++++-----------------
>>>  tests/test-qmp-output-visitor.c         | 42 +++++++++---------
>>>  6 files changed, 66 insertions(+), 68 deletions(-)
>>>
>>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>>> index 1e7e08b..aab2b46 100644
>>> --- a/scripts/qapi.py
>>> +++ b/scripts/qapi.py
>>> @@ -546,7 +546,7 @@ def check_union(expr, expr_info):
>>>      base = expr.get('base')
>>>      discriminator = expr.get('discriminator')
>>>      members = expr['data']
>>> -    values = {'MAX': '(automatic)', 'KIND': '(automatic)'}
>>> +    values = {'MAX': '(automatic)', 'TYPE': '(automatic tag)'}
>>>
>>>      # Two types of unions, determined by discriminator.
>>>
>> 
>> Umm, does this really belong to this patch?
>
> Yes, because it cleans up the error message in union-clash-type.err, as
> mentioned in the commit message. But since I'm already splitting out the
> qapi-visit parts of 7/17, maybe it belongs better in that patch (all
> changes to the rest of qapi to deal with the qapi-type parts)?

Makes sense, I think.

>>> diff --git a/tests/qapi-schema/union-clash-type.err
>>> b/tests/qapi-schema/union-clash-type.err
>>> index a5dead1..c14bbdd 100644
>>> --- a/tests/qapi-schema/union-clash-type.err
>>> +++ b/tests/qapi-schema/union-clash-type.err
>>> @@ -1 +1 @@
>>> -tests/qapi-schema/union-clash-type.json:8: Union 'TestUnion'
>>> member 'kind' clashes with '(automatic)'
>>> +tests/qapi-schema/union-clash-type.json:6: Union 'TestUnion'
>>> member 'type' clashes with '(automatic tag)'
>
> At any rate, this is what improved by adjusting that line of qapi.py.
>
>>> diff --git a/tests/qapi-schema/union-clash-type.json
>>> b/tests/qapi-schema/union-clash-type.json
>>> index cfc256b..641b2d5 100644
>>> --- a/tests/qapi-schema/union-clash-type.json
>>> +++ b/tests/qapi-schema/union-clash-type.json
>>> @@ -1,9 +1,7 @@
>>>  # Union branch 'type'
>>>  # Reject this, because we would have a clash in generated C, between the
>>> -# simple union's implicit tag member 'kind' and the branch name 'kind'
>>> +# simple union's implicit tag member 'type' and the branch name 'type'
>>>  # within the union.
>>> -# TODO: Even when the generated C is switched to use 'type' rather than
>>> -# 'kind', to match the QMP spelling, the collision should still be detected.
>>> -# Or, we could munge the branch name to allow compilation.
>>> +# TODO: If desired, we could munge the branch name to allow compilation.
>> 
>> Let's mark it TODO only if we intend to revisit the idea of munging
>> branch names.
>
> I have a later patch queued that deletes this test altogether, so for
> v10 I'll probably just eliminate changes here for less churn.
> https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg04111.html
>
>> 
>>>  { 'union': 'TestUnion',
>>>    'data': { 'kind': 'int', 'type': 'str' } }
>> [Mind-numbing mechanical switch to u. and from kind to type...]
>> 
>
> Yep.  I wish I knew coccinelle well enough to see if it could do the
> conversion for me, but I ended up doing it by hand (basically by
> applying 16/17 early, then seeing what failed to compile, fixing it up,
> then rebasing it into position).

I might be able to sledgehammer it into service here, but since you've
done the job already...
diff mbox

Patch

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1e7e08b..aab2b46 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -546,7 +546,7 @@  def check_union(expr, expr_info):
     base = expr.get('base')
     discriminator = expr.get('discriminator')
     members = expr['data']
-    values = {'MAX': '(automatic)', 'KIND': '(automatic)'}
+    values = {'MAX': '(automatic)', 'TYPE': '(automatic tag)'}

     # Two types of unions, determined by discriminator.

diff --git a/tests/qapi-schema/union-clash-type.err b/tests/qapi-schema/union-clash-type.err
index a5dead1..c14bbdd 100644
--- a/tests/qapi-schema/union-clash-type.err
+++ b/tests/qapi-schema/union-clash-type.err
@@ -1 +1 @@ 
-tests/qapi-schema/union-clash-type.json:8: Union 'TestUnion' member 'kind' clashes with '(automatic)'
+tests/qapi-schema/union-clash-type.json:6: Union 'TestUnion' member 'type' clashes with '(automatic tag)'
diff --git a/tests/qapi-schema/union-clash-type.json b/tests/qapi-schema/union-clash-type.json
index cfc256b..641b2d5 100644
--- a/tests/qapi-schema/union-clash-type.json
+++ b/tests/qapi-schema/union-clash-type.json
@@ -1,9 +1,7 @@ 
 # Union branch 'type'
 # Reject this, because we would have a clash in generated C, between the
-# simple union's implicit tag member 'kind' and the branch name 'kind'
+# simple union's implicit tag member 'type' and the branch name 'type'
 # within the union.
-# TODO: Even when the generated C is switched to use 'type' rather than
-# 'kind', to match the QMP spelling, the collision should still be detected.
-# Or, we could munge the branch name to allow compilation.
+# TODO: If desired, we could munge the branch name to allow compilation.
 { 'union': 'TestUnion',
   'data': { 'kind': 'int', 'type': 'str' } }
diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c
index ea700d8..f23d8ea 100644
--- a/tests/test-qmp-commands.c
+++ b/tests/test-qmp-commands.c
@@ -62,8 +62,8 @@  __org_qemu_x_Union1 *qmp___org_qemu_x_command(__org_qemu_x_EnumList *a,
 {
     __org_qemu_x_Union1 *ret = g_new0(__org_qemu_x_Union1, 1);

-    ret->kind = ORG_QEMU_X_UNION1_KIND___ORG_QEMU_X_BRANCH;
-    ret->__org_qemu_x_branch = strdup("blah1");
+    ret->type = ORG_QEMU_X_UNION1_KIND___ORG_QEMU_X_BRANCH;
+    ret->u.__org_qemu_x_branch = strdup("blah1");

     return ret;
 }
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 514cfd0..617eada 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -359,7 +359,7 @@  static void test_visitor_in_union_flat(TestInputVisitorData *data,
     g_assert_cmpint(tmp->enum1, ==, ENUM_ONE_VALUE1);
     g_assert_cmpstr(tmp->string, ==, "str");
     g_assert_cmpint(tmp->integer, ==, 41);
-    g_assert_cmpint(tmp->value1->boolean, ==, true);
+    g_assert_cmpint(tmp->u.value1->boolean, ==, true);
     qapi_free_UserDefFlatUnion(tmp);
 }

@@ -372,15 +372,15 @@  static void test_visitor_in_alternate(TestInputVisitorData *data,

     v = visitor_input_test_init(data, "42");
     visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
-    g_assert_cmpint(tmp->kind, ==, USER_DEF_ALTERNATE_KIND_I);
-    g_assert_cmpint(tmp->i, ==, 42);
+    g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_I);
+    g_assert_cmpint(tmp->u.i, ==, 42);
     qapi_free_UserDefAlternate(tmp);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "'string'");
     visit_type_UserDefAlternate(v, &tmp, NULL, &error_abort);
-    g_assert_cmpint(tmp->kind, ==, USER_DEF_ALTERNATE_KIND_S);
-    g_assert_cmpstr(tmp->s, ==, "string");
+    g_assert_cmpint(tmp->type, ==, USER_DEF_ALTERNATE_KIND_S);
+    g_assert_cmpstr(tmp->u.s, ==, "string");
     qapi_free_UserDefAlternate(tmp);
     visitor_input_teardown(data, NULL);

@@ -419,8 +419,8 @@  static void test_visitor_in_alternate_number(TestInputVisitorData *data,
      * parse the same as ans */
     v = visitor_input_test_init(data, "42");
     visit_type_AltStrNum(v, &asn, NULL, &err);
-    /* FIXME g_assert_cmpint(asn->kind, == ALT_STR_NUM_KIND_N); */
-    /* FIXME g_assert_cmpfloat(asn->n, ==, 42); */
+    /* FIXME g_assert_cmpint(asn->type, == ALT_STR_NUM_KIND_N); */
+    /* FIXME g_assert_cmpfloat(asn->u.n, ==, 42); */
     g_assert(err);
     error_free(err);
     err = NULL;
@@ -429,29 +429,29 @@  static void test_visitor_in_alternate_number(TestInputVisitorData *data,

     v = visitor_input_test_init(data, "42");
     visit_type_AltNumStr(v, &ans, NULL, &error_abort);
-    g_assert_cmpint(ans->kind, ==, ALT_NUM_STR_KIND_N);
-    g_assert_cmpfloat(ans->n, ==, 42);
+    g_assert_cmpint(ans->type, ==, ALT_NUM_STR_KIND_N);
+    g_assert_cmpfloat(ans->u.n, ==, 42);
     qapi_free_AltNumStr(ans);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42");
     visit_type_AltStrInt(v, &asi, NULL, &error_abort);
-    g_assert_cmpint(asi->kind, ==, ALT_STR_INT_KIND_I);
-    g_assert_cmpint(asi->i, ==, 42);
+    g_assert_cmpint(asi->type, ==, ALT_STR_INT_KIND_I);
+    g_assert_cmpint(asi->u.i, ==, 42);
     qapi_free_AltStrInt(asi);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42");
     visit_type_AltIntNum(v, &ain, NULL, &error_abort);
-    g_assert_cmpint(ain->kind, ==, ALT_INT_NUM_KIND_I);
-    g_assert_cmpint(ain->i, ==, 42);
+    g_assert_cmpint(ain->type, ==, ALT_INT_NUM_KIND_I);
+    g_assert_cmpint(ain->u.i, ==, 42);
     qapi_free_AltIntNum(ain);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42");
     visit_type_AltNumInt(v, &ani, NULL, &error_abort);
-    g_assert_cmpint(ani->kind, ==, ALT_NUM_INT_KIND_I);
-    g_assert_cmpint(ani->i, ==, 42);
+    g_assert_cmpint(ani->type, ==, ALT_NUM_INT_KIND_I);
+    g_assert_cmpint(ani->u.i, ==, 42);
     qapi_free_AltNumInt(ani);
     visitor_input_teardown(data, NULL);

@@ -467,15 +467,15 @@  static void test_visitor_in_alternate_number(TestInputVisitorData *data,

     v = visitor_input_test_init(data, "42.5");
     visit_type_AltStrNum(v, &asn, NULL, &error_abort);
-    g_assert_cmpint(asn->kind, ==, ALT_STR_NUM_KIND_N);
-    g_assert_cmpfloat(asn->n, ==, 42.5);
+    g_assert_cmpint(asn->type, ==, ALT_STR_NUM_KIND_N);
+    g_assert_cmpfloat(asn->u.n, ==, 42.5);
     qapi_free_AltStrNum(asn);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42.5");
     visit_type_AltNumStr(v, &ans, NULL, &error_abort);
-    g_assert_cmpint(ans->kind, ==, ALT_NUM_STR_KIND_N);
-    g_assert_cmpfloat(ans->n, ==, 42.5);
+    g_assert_cmpint(ans->type, ==, ALT_NUM_STR_KIND_N);
+    g_assert_cmpfloat(ans->u.n, ==, 42.5);
     qapi_free_AltNumStr(ans);
     visitor_input_teardown(data, NULL);

@@ -489,15 +489,15 @@  static void test_visitor_in_alternate_number(TestInputVisitorData *data,

     v = visitor_input_test_init(data, "42.5");
     visit_type_AltIntNum(v, &ain, NULL, &error_abort);
-    g_assert_cmpint(ain->kind, ==, ALT_INT_NUM_KIND_N);
-    g_assert_cmpfloat(ain->n, ==, 42.5);
+    g_assert_cmpint(ain->type, ==, ALT_INT_NUM_KIND_N);
+    g_assert_cmpfloat(ain->u.n, ==, 42.5);
     qapi_free_AltIntNum(ain);
     visitor_input_teardown(data, NULL);

     v = visitor_input_test_init(data, "42.5");
     visit_type_AltNumInt(v, &ani, NULL, &error_abort);
-    g_assert_cmpint(ani->kind, ==, ALT_NUM_INT_KIND_N);
-    g_assert_cmpfloat(ani->n, ==, 42.5);
+    g_assert_cmpint(ani->type, ==, ALT_NUM_INT_KIND_N);
+    g_assert_cmpfloat(ani->u.n, ==, 42.5);
     qapi_free_AltNumInt(ani);
     visitor_input_teardown(data, NULL);
 }
@@ -527,68 +527,68 @@  static void test_native_list_integer_helper(TestInputVisitorData *data,
     visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
     g_assert(err == NULL);
     g_assert(cvalue != NULL);
-    g_assert_cmpint(cvalue->kind, ==, kind);
+    g_assert_cmpint(cvalue->type, ==, kind);

     switch (kind) {
     case USER_DEF_NATIVE_LIST_UNION_KIND_INTEGER: {
         intList *elem = NULL;
-        for (i = 0, elem = cvalue->integer; elem; elem = elem->next, i++) {
+        for (i = 0, elem = cvalue->u.integer; elem; elem = elem->next, i++) {
             g_assert_cmpint(elem->value, ==, i);
         }
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_S8: {
         int8List *elem = NULL;
-        for (i = 0, elem = cvalue->s8; elem; elem = elem->next, i++) {
+        for (i = 0, elem = cvalue->u.s8; elem; elem = elem->next, i++) {
             g_assert_cmpint(elem->value, ==, i);
         }
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_S16: {
         int16List *elem = NULL;
-        for (i = 0, elem = cvalue->s16; elem; elem = elem->next, i++) {
+        for (i = 0, elem = cvalue->u.s16; elem; elem = elem->next, i++) {
             g_assert_cmpint(elem->value, ==, i);
         }
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_S32: {
         int32List *elem = NULL;
-        for (i = 0, elem = cvalue->s32; elem; elem = elem->next, i++) {
+        for (i = 0, elem = cvalue->u.s32; elem; elem = elem->next, i++) {
             g_assert_cmpint(elem->value, ==, i);
         }
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_S64: {
         int64List *elem = NULL;
-        for (i = 0, elem = cvalue->s64; elem; elem = elem->next, i++) {
+        for (i = 0, elem = cvalue->u.s64; elem; elem = elem->next, i++) {
             g_assert_cmpint(elem->value, ==, i);
         }
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_U8: {
         uint8List *elem = NULL;
-        for (i = 0, elem = cvalue->u8; elem; elem = elem->next, i++) {
+        for (i = 0, elem = cvalue->u.u8; elem; elem = elem->next, i++) {
             g_assert_cmpint(elem->value, ==, i);
         }
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_U16: {
         uint16List *elem = NULL;
-        for (i = 0, elem = cvalue->u16; elem; elem = elem->next, i++) {
+        for (i = 0, elem = cvalue->u.u16; elem; elem = elem->next, i++) {
             g_assert_cmpint(elem->value, ==, i);
         }
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_U32: {
         uint32List *elem = NULL;
-        for (i = 0, elem = cvalue->u32; elem; elem = elem->next, i++) {
+        for (i = 0, elem = cvalue->u.u32; elem; elem = elem->next, i++) {
             g_assert_cmpint(elem->value, ==, i);
         }
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_U64: {
         uint64List *elem = NULL;
-        for (i = 0, elem = cvalue->u64; elem; elem = elem->next, i++) {
+        for (i = 0, elem = cvalue->u.u64; elem; elem = elem->next, i++) {
             g_assert_cmpint(elem->value, ==, i);
         }
         break;
@@ -690,9 +690,9 @@  static void test_visitor_in_native_list_bool(TestInputVisitorData *data,
     visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
     g_assert(err == NULL);
     g_assert(cvalue != NULL);
-    g_assert_cmpint(cvalue->kind, ==, USER_DEF_NATIVE_LIST_UNION_KIND_BOOLEAN);
+    g_assert_cmpint(cvalue->type, ==, USER_DEF_NATIVE_LIST_UNION_KIND_BOOLEAN);

-    for (i = 0, elem = cvalue->boolean; elem; elem = elem->next, i++) {
+    for (i = 0, elem = cvalue->u.boolean; elem; elem = elem->next, i++) {
         g_assert_cmpint(elem->value, ==, (i % 3 == 0) ? 1 : 0);
     }

@@ -725,9 +725,9 @@  static void test_visitor_in_native_list_string(TestInputVisitorData *data,
     visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
     g_assert(err == NULL);
     g_assert(cvalue != NULL);
-    g_assert_cmpint(cvalue->kind, ==, USER_DEF_NATIVE_LIST_UNION_KIND_STRING);
+    g_assert_cmpint(cvalue->type, ==, USER_DEF_NATIVE_LIST_UNION_KIND_STRING);

-    for (i = 0, elem = cvalue->string; elem; elem = elem->next, i++) {
+    for (i = 0, elem = cvalue->u.string; elem; elem = elem->next, i++) {
         gchar str[8];
         sprintf(str, "%d", i);
         g_assert_cmpstr(elem->value, ==, str);
@@ -764,9 +764,9 @@  static void test_visitor_in_native_list_number(TestInputVisitorData *data,
     visit_type_UserDefNativeListUnion(v, &cvalue, NULL, &err);
     g_assert(err == NULL);
     g_assert(cvalue != NULL);
-    g_assert_cmpint(cvalue->kind, ==, USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER);
+    g_assert_cmpint(cvalue->type, ==, USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER);

-    for (i = 0, elem = cvalue->number; elem; elem = elem->next, i++) {
+    for (i = 0, elem = cvalue->u.number; elem; elem = elem->next, i++) {
         GString *double_expected = g_string_new("");
         GString *double_actual = g_string_new("");

diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index 88e01ea..25c5e25 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -486,9 +486,9 @@  static void test_visitor_out_union_flat(TestOutputVisitorData *data,
     UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion));
     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; */
-    tmp->value1->boolean = true;
+    tmp->u.value1 = g_malloc0(sizeof(UserDefA));
+    tmp->integer = 41;
+    tmp->u.value1->boolean = true;

     visit_type_UserDefFlatUnion(data->ov, &tmp, NULL, &err);
     g_assert(err == NULL);
@@ -499,7 +499,7 @@  static void test_visitor_out_union_flat(TestOutputVisitorData *data,

     g_assert_cmpstr(qdict_get_str(qdict, "enum1"), ==, "value1");
     g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "str");
-    /* TODO g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 41); */
+    g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 41);
     g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, true);

     qapi_free_UserDefFlatUnion(tmp);
@@ -513,8 +513,8 @@  static void test_visitor_out_alternate(TestOutputVisitorData *data,
     Error *err = NULL;

     UserDefAlternate *tmp = g_malloc0(sizeof(UserDefAlternate));
-    tmp->kind = USER_DEF_ALTERNATE_KIND_I;
-    tmp->i = 42;
+    tmp->type = USER_DEF_ALTERNATE_KIND_I;
+    tmp->u.i = 42;

     visit_type_UserDefAlternate(data->ov, &tmp, NULL, &err);
     g_assert(err == NULL);
@@ -539,9 +539,9 @@  static void test_visitor_out_empty(TestOutputVisitorData *data,
 static void init_native_list(UserDefNativeListUnion *cvalue)
 {
     int i;
-    switch (cvalue->kind) {
+    switch (cvalue->type) {
     case USER_DEF_NATIVE_LIST_UNION_KIND_INTEGER: {
-        intList **list = &cvalue->integer;
+        intList **list = &cvalue->u.integer;
         for (i = 0; i < 32; i++) {
             *list = g_new0(intList, 1);
             (*list)->value = i;
@@ -551,7 +551,7 @@  static void init_native_list(UserDefNativeListUnion *cvalue)
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_S8: {
-        int8List **list = &cvalue->s8;
+        int8List **list = &cvalue->u.s8;
         for (i = 0; i < 32; i++) {
             *list = g_new0(int8List, 1);
             (*list)->value = i;
@@ -561,7 +561,7 @@  static void init_native_list(UserDefNativeListUnion *cvalue)
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_S16: {
-        int16List **list = &cvalue->s16;
+        int16List **list = &cvalue->u.s16;
         for (i = 0; i < 32; i++) {
             *list = g_new0(int16List, 1);
             (*list)->value = i;
@@ -571,7 +571,7 @@  static void init_native_list(UserDefNativeListUnion *cvalue)
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_S32: {
-        int32List **list = &cvalue->s32;
+        int32List **list = &cvalue->u.s32;
         for (i = 0; i < 32; i++) {
             *list = g_new0(int32List, 1);
             (*list)->value = i;
@@ -581,7 +581,7 @@  static void init_native_list(UserDefNativeListUnion *cvalue)
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_S64: {
-        int64List **list = &cvalue->s64;
+        int64List **list = &cvalue->u.s64;
         for (i = 0; i < 32; i++) {
             *list = g_new0(int64List, 1);
             (*list)->value = i;
@@ -591,7 +591,7 @@  static void init_native_list(UserDefNativeListUnion *cvalue)
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_U8: {
-        uint8List **list = &cvalue->u8;
+        uint8List **list = &cvalue->u.u8;
         for (i = 0; i < 32; i++) {
             *list = g_new0(uint8List, 1);
             (*list)->value = i;
@@ -601,7 +601,7 @@  static void init_native_list(UserDefNativeListUnion *cvalue)
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_U16: {
-        uint16List **list = &cvalue->u16;
+        uint16List **list = &cvalue->u.u16;
         for (i = 0; i < 32; i++) {
             *list = g_new0(uint16List, 1);
             (*list)->value = i;
@@ -611,7 +611,7 @@  static void init_native_list(UserDefNativeListUnion *cvalue)
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_U32: {
-        uint32List **list = &cvalue->u32;
+        uint32List **list = &cvalue->u.u32;
         for (i = 0; i < 32; i++) {
             *list = g_new0(uint32List, 1);
             (*list)->value = i;
@@ -621,7 +621,7 @@  static void init_native_list(UserDefNativeListUnion *cvalue)
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_U64: {
-        uint64List **list = &cvalue->u64;
+        uint64List **list = &cvalue->u.u64;
         for (i = 0; i < 32; i++) {
             *list = g_new0(uint64List, 1);
             (*list)->value = i;
@@ -631,7 +631,7 @@  static void init_native_list(UserDefNativeListUnion *cvalue)
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_BOOLEAN: {
-        boolList **list = &cvalue->boolean;
+        boolList **list = &cvalue->u.boolean;
         for (i = 0; i < 32; i++) {
             *list = g_new0(boolList, 1);
             (*list)->value = (i % 3 == 0);
@@ -641,7 +641,7 @@  static void init_native_list(UserDefNativeListUnion *cvalue)
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_STRING: {
-        strList **list = &cvalue->string;
+        strList **list = &cvalue->u.string;
         for (i = 0; i < 32; i++) {
             *list = g_new0(strList, 1);
             (*list)->value = g_strdup_printf("%d", i);
@@ -651,7 +651,7 @@  static void init_native_list(UserDefNativeListUnion *cvalue)
         break;
     }
     case USER_DEF_NATIVE_LIST_UNION_KIND_NUMBER: {
-        numberList **list = &cvalue->number;
+        numberList **list = &cvalue->u.number;
         for (i = 0; i < 32; i++) {
             *list = g_new0(numberList, 1);
             (*list)->value = (double)i / 3;
@@ -760,14 +760,14 @@  static void test_native_list(TestOutputVisitorData *data,
     Error *err = NULL;
     QObject *obj;

-    cvalue->kind = kind;
+    cvalue->type = kind;
     init_native_list(cvalue);

     visit_type_UserDefNativeListUnion(data->ov, &cvalue, NULL, &err);
     g_assert(err == NULL);

     obj = qmp_output_get_qobject(data->qov);
-    check_native_list(obj, cvalue->kind);
+    check_native_list(obj, cvalue->type);
     qapi_free_UserDefNativeListUnion(cvalue);
     qobject_decref(obj);
 }