[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 11/25] qapi-visit: Remove redundant function
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v10 11/25] qapi-visit: Remove redundant functions for flat union base |
Date: |
Fri, 23 Oct 2015 21:35:15 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> The code for visiting the base class of a child struct created
> visit_type_Base_fields() which covers all fields of Base; while
> the code for visiting the base class of a flat union created
> visit_type_Union_fields() covering all fields of the base
> except the discriminator. But since the base class includes
> the discriminator of a flat union, we can just visit the entire
> base, without needing a separate visit of the discriminator.
> Not only is consistently visiting all fields easier to
> understand, it lets us share code.
>
> The generated code in qapi-visit.c loses several now-unused
> visit_type_UNION_fields(), along with changes like:
>
> |@@ -1654,11 +1557,7 @@ void visit_type_BlockdevOptions(Visitor
> | if (!*obj) {
> | goto out_obj;
> | }
> |- visit_type_BlockdevOptions_fields(v, obj, &err);
> |- if (err) {
> |- goto out_obj;
> |- }
> |- visit_type_BlockdevDriver(v, &(*obj)->driver, "driver", &err);
> |+ visit_type_BlockdevOptionsBase_fields(v, (BlockdevOptionsBase **)obj,
> &err);
> | if (err) {
> | goto out_obj;
> | }
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v10: split out regex to earlier commit, rebase to earlier changes,
> improve commit message
> v9: (no v6-8): hoist from v5 35/46; rebase to master; fix indentation
> botch in gen_visit_union(); polish commit message
> ---
> scripts/qapi-visit.py | 32 +++++++++++++++-----------------
> 1 file changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index e30062b..3c23c53 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -226,8 +226,7 @@ def gen_visit_union(name, base, variants):
> ret = ''
>
> if base:
> - members = [m for m in base.members if m != variants.tag_member]
> - ret += gen_visit_struct_fields(name, None, members)
> + ret += gen_visit_fields_decl(base)
>
> for var in variants.variants:
> # Ugly special case for simple union TODO get rid of it
Instead of generating a _fields() for the union, we'll reuse the base's.
May have to generate a forward declaration for it.
> @@ -252,31 +251,30 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s
> **obj, const char *name, Error
>
Visit the non-variant members:
> if base:
A flat union's non-variant members are exactly the base's members, so
call the base's _fields():
> ret += mcgen('''
> - visit_type_%(c_name)s_fields(v, obj, &err);
> + visit_type_%(c_name)s_fields(v, (%(c_name)s **)obj, &err);
> ''',
> - c_name=c_name(name))
+ c_name=base.c_name())
I stared at the new cast here some until I realized: to call the base's
_fields(), we need to cast obj to base.
> - ret += gen_err_check(label='out_obj')
> -
> - tag_key = variants.tag_member.name
> - if not variants.tag_name:
> - # we pointlessly use a different key for simple unions
> - tag_key = 'type'
> - ret += mcgen('''
> + c_name=base.c_name())
> + else:
A simple union's non-variant member is the implicit tag, and no more, so
visit it:
> + ret += mcgen('''
> visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
> - if (err) {
> - goto out_obj;
> - }
> +''',
> + c_type=variants.tag_member.type.c_name(),
> + # TODO ugly special case for simple union
> + # Use same tag name in C as on the wire to get rid of
> + # it, then: c_name=c_name(variants.tag_member.name)
> + c_name='kind',
> + name=variants.tag_member.name)
The 'type' vs. 'kind' stupidity never fails to confuse me, and at this
time of day I give up, and diff the generated output. It looks okay.
Common error check for both branches of the if:
> + ret += gen_err_check(label='out_obj')
> + ret += mcgen('''
> if (!visit_start_union(v, !!(*obj)->data, &err) || err) {
> goto out_obj;
> }
Visit variant members:
> switch ((*obj)->%(c_name)s) {
> ''',
> - c_type=variants.tag_member.type.c_name(),
> # TODO ugly special case for simple union
> # Use same tag name in C as on the wire to get rid of
> # it, then: c_name=c_name(variants.tag_member.name)
> - c_name=c_name(variants.tag_name or 'kind'),
> - name=tag_key)
> + c_name=c_name(variants.tag_name or 'kind'))
>
> for var in variants.variants:
> # TODO ugly special case for simple union
- Re: [Qemu-devel] [PATCH v10 09/25] qapi: Prefer typesafe upcasts to qapi base classes, (continued)
[Qemu-devel] [PATCH v10 20/25] memory: Convert to new qapi union layout, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 22/25] qapi: Finish converting to new qapi union layout, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 18/25] char: Convert to new qapi union layout, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 23/25] qapi: Reserve 'u' member name, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 11/25] qapi-visit: Remove redundant functions for flat union base, Eric Blake, 2015/10/23
- Re: [Qemu-devel] [PATCH v10 11/25] qapi-visit: Remove redundant functions for flat union base,
Markus Armbruster <=
[Qemu-devel] [PATCH v10 16/25] sockets: Convert to new qapi union layout, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 19/25] input: Convert to new qapi union layout, Eric Blake, 2015/10/23
[Qemu-devel] [PATCH v10 24/25] qapi: Remove outdated tests related to QMP/branch collisions, Eric Blake, 2015/10/23
Re: [Qemu-devel] [PATCH v10 00/25] qapi collision reduction (post-introspection subset B'), Markus Armbruster, 2015/10/26