diff options
author | Eric Blake <eblake@redhat.com> | 2015-10-26 16:35:01 -0600 |
---|---|---|
committer | Markus Armbruster <armbru@redhat.com> | 2015-11-02 08:30:28 +0100 |
commit | e4ba22b31943ab02373359555bd7bcd66442632f (patch) | |
tree | 07a8cc91f27f3529dddf35319c3d4f2214bbd149 /scripts | |
parent | ce21131a0b9e556bb73bf65eacdc07ccb21f78a9 (diff) |
qapi: Finish converting to new qapi union layout
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 non-variant
member's name.
This patch is the back end for a series that converts to a
saner qapi union layout. Now that all clients have been
converted to use 'type' and 'obj->u.value', we can drop the
temporary parallel support for 'kind' and 'obj->value'.
Given a simple union qapi type:
{ 'union':'Foo', 'data': { 'a':'int', 'b':'bool' } }
this is the overall effect, when compared to the state before
this series of patches:
| struct Foo {
|- FooKind kind;
|- union { /* union tag is @kind */
|+ FooKind type;
|+ union { /* union tag is @type */
| void *data;
| int64_t a;
| bool b;
|- };
|+ } u;
| };
The testsuite still contains some examples of artificial restrictions
(see flat-union-clash-type.json, for example) that are no longer
technically necessary, now that there is no longer a collision between
enum tag values and non-variant member names; but fixing this will be
done in later patches, in part because some further changes are required
to keep QAPISchema*.check() from asserting. Also, a later patch will
add a reservation for the member name 'u' to avoid a collision between a
user's non-variant names and our internal choice of C union name.
Note, however, that we do not rename the generated enum, which
is still 'FooKind'. A further patch could generate implicit
enums as 'FooType', but while the generator already reserved
the '*Kind' namespace (commit 4dc2e69), there are already QMP
constructs with '*Type' naming, which means changing our
reservation namespace would have lots of churn to C code to
deal with a forced name change.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <1445898903-12082-23-git-send-email-eblake@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Diffstat (limited to 'scripts')
-rw-r--r-- | scripts/qapi-types.py | 28 |
1 files changed, 6 insertions, 22 deletions
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 1420e00ffb..7e35bb6ac7 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -149,23 +149,10 @@ struct %(c_name)s { if base: ret += gen_struct_fields([], base) else: - # TODO As a hack, we emit both 'kind' and 'type'. Ultimately, we - # want to use only 'type', but the conversion is large enough to - # require staging over several commits. - ret += mcgen(''' - union { - %(c_type)s kind; - %(c_type)s type; - }; -''', - c_type=c_name(variants.tag_member.type.name)) - - # TODO As a hack, we emit the union twice, once as an anonymous union - # and once as a named union. Ultimately, we want to use only the - # named union version (as it avoids conflicts between tag values as - # branch names competing with non-variant QMP names), but the conversion - # is large enough to require staging over several commits. - tmp = '' + ret += gen_struct_field(variants.tag_member.name, + variants.tag_member.type, + False) + # FIXME: What purpose does data serve, besides preventing a union that # has a branch named 'data'? We use it in qapi-visit.py to decide # whether to bypass the switch statement if visiting the discriminator @@ -174,7 +161,7 @@ struct %(c_name)s { # should not be any data leaks even without a data pointer. Or, if # 'data' is merely added to guarantee we don't have an empty union, # shouldn't we enforce that at .json parse time? - tmp += mcgen(''' + ret += mcgen(''' union { /* union tag is @%(c_name)s */ void *data; ''', @@ -183,17 +170,14 @@ struct %(c_name)s { for var in variants.variants: # Ugly special case for simple union TODO get rid of it typ = var.simple_union_type() or var.type - tmp += mcgen(''' + ret += mcgen(''' %(c_type)s %(c_name)s; ''', c_type=typ.c_type(), c_name=c_name(var.name)) - ret += tmp - ret += ' ' + '\n '.join(tmp.split('\n')) ret += mcgen(''' } u; - }; }; ''') |