[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 13/19] qapi-visit: Simplify visit of empty br
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 13/19] qapi-visit: Simplify visit of empty branch in union |
Date: |
Thu, 03 Mar 2016 10:14:44 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> Now that we have is_empty() and gen_visit_fields_call(), it's
s/fields/members/. Better grep all the commit messages for "fields".
> fairly easy to skip the visit of a variant type that has no
> members.
I figure that alone would've been just as easy before
gen_visit_members_call(), but see below.
> Only one such instance exists at the moment
> (CpuInfoOther),
My testing shows qapi-schema-test.json's Empty2 is also affected:
@@ -198,7 +198,6 @@ void visit_type_Empty2_members(Visitor *
{
Error *err = NULL;
- visit_type_Empty1_members(v, (Empty1 *)obj, &err);
if (err) {
goto out;
}
This isn't "the visit of a variant type that has no members", it's a
visit of an empty base type.
If you go back to PATCH 10, you can see this patch's stated purpose is
tied to the last hunk there, which uses gen_visit_members_call() for a
variant. Its other use is for the base.
Suggest to rephrase this patch's commit message to capture both.
> but the idea of a union where some branches
> add no fields beyond the base type is common enough that we
> may add syntax for other cases, as in
> { 'union':'U', 'base':'B', 'discriminator':'D',
> 'data': { 'default': {}, 'extra':'Type' } }
I find 'default' confusing, because a variant record's default case is
the case that applies to all the tag values not explicitly listed. What
about simply 'case1' and 'case2'?
> Note that the Abort type is another example of an empty type,
> but it is only used by a simple union rather than a flat
> union; and with simple unions, the wrapper type providing
> the 'data' QMP key is not empty.
As so often, simple unions are anything but.
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v2: no change
> ---
> scripts/qapi-visit.py | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index dbae00c..a9de393 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -37,7 +37,9 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s
> *obj, Error **errp);
> def gen_visit_members_call(typ, c_name):
> ret = ''
> assert isinstance(typ, QAPISchemaObjectType)
> - if typ.is_implicit():
> + if typ.is_empty():
> + pass
> + elif typ.is_implicit():
> # TODO ugly special case for simple union
> assert len(typ.members) == 1
> assert not typ.variants
Okay because the patch is trivial. If it wasn't, I'd say avoiding the
call isn't worthwhile; the compiler might even optimize it to nothing.
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PATCH v2 13/19] qapi-visit: Simplify visit of empty branch in union,
Markus Armbruster <=