Re: [Qemu-devel] [RFC PATCH] qapi: Rename simple union's generated tag member to type

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3628 bytes --]

On 10/08/2015 06:26 AM, Markus Armbruster wrote:
> Struct and union type members are generally named alike in QMP and C,
> except for a simple union's implicit tag member, which is "type" in
> QMP, and "kind" in C.  Can't change QMP, so rename it in C.
> 
> Since the implicit enumeration type is still called *Kind, this
> doesn't clean up the type vs. kind mess completely.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
...
>  scripts/qapi-types.py           | 12 ++++--------
>  scripts/qapi-visit.py           | 15 ++++-----------
>  tests/test-qmp-commands.c       |  2 +-
>  tests/test-qmp-input-visitor.c  | 30 +++++++++++++++---------------
>  tests/test-qmp-output-visitor.c |  8 ++++----
>  tpm.c                           |  2 +-
>  ui/input-keymap.c               | 10 +++++-----
>  ui/input-legacy.c               |  2 +-
>  ui/input.c                      | 22 +++++++++++-----------
>  util/qemu-sockets.c             | 12 ++++++------
>  33 files changed, 113 insertions(+), 123 deletions(-)

This touches a lot of files all in one commit (both your RFC and my v5
version), and then I get to touch the same files all over again when I
swap to a named rather than anonymous union in the C struct.  So here's
what I'm currently playing with:

first patch: hack _just_ scripts/qapi*py to turn:

{ 'union':'Foo', 'data':{'a':'int','b':'bool'}}

into:

struct Foo {
    union {
        FooKind type;
        FooKind kind; /* temporary hack */
    };
    union { /* union tag is @type */
        void *data; /* will go away much later in series */
        int64_t a;
        bool b;
        union {
            void *data;
            int64_t a;
            int b;
        } u;
    };
};

so that old code accessing foo->kind and foo->a just works, but also
leaving the door open to access foo->type and foo->u.a.  Then a series
of patches grouped by logical file sets (so no one patch is too huge to
review, and spreading the load among maintainers), and a final patch to
scripts/qapi*.py to get to the desired:

struct Foo {
    FooKind type;
    union { /* union tag is @type */
        void *data; /* will go away much later in series */
        int64_t a;
        bool b;
    } u;
};

at which point we've gotten rid of any collisions between tag value
(branch names) and QMP names, at the possible expense of a new collision
with 'u'.  I'm also beefing up the testsuite and check_name() function
(or maybe somewhere else, still haven't actually written that part of my
planned series) to reject 'u' and anything spelled 'has_*' as member
names, to proactively avoid the need to worry about collisions with 'u'
or the added members for optional fields.  I'll probably also reject
'*List' as a user-supplied type name, addressing the comment just barely
added in current qapi-next that we don't have a reserved namespace for
arrays.

The collision with 'data' is harder; I can't remove it until we delete
visit_start_union()/visit_end_union() which starts to get in the mess of
patches that work with qapi visitor interfaces, so that will be in a
later subset.

It may be a day or two before I can post the pending work.  Meanwhile, I
previously posted subset C which is somewhat orthogonal (not sure if it
needs any minor rebasing to apply against current qapi-next), if you
want to dive into reviewing that, instead of waiting:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg01980.html

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]