[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 15/19] qapi-visit: Move error check into gen_
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 15/19] qapi-visit: Move error check into gen_visit_members_call() |
Date: |
Fri, 4 Mar 2016 07:27:20 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
On 03/03/2016 04:56 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> When first introduced, neither branch of gen_visit_members_call()
>> would output a goto. But now that the implicit struct visit
>> always ends with a goto, we should do the same for regular
>> struct visits, so that callers don't have to worry about whether
>> they are creating two identical goto's in a row.
>>
>> Generated code gets slightly larger; if desired, we could patch
>> qapi.py:gen_visit_members() to have a mode where it skips the
>> final goto and leave it up to the callers when to use that mode,
>> but that adds more maintenance burden when the compiler should
>> be smart enough to not bloat the .o file just because the .c
>> file got larger.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>
>> +++ b/scripts/qapi-visit.py
>> @@ -48,6 +48,7 @@ def gen_visit_members_call(typ, direct_name,
>> implicit_name=None):
> assert isinstance(typ, QAPISchemaObjectType)
> if typ.is_empty():
> pass
> elif typ.is_implicit():
> assert implicit_name
> assert not typ.variants
> ret += gen_visit_members(typ.members, prefix=implicit_name)
>
> This is the goto-generating part mentioned in the commit message.
>
>> @@ -95,9 +95,9 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s
>> *obj, Error **errp)
>> }
>> ''')
>>
>> - # 'goto out' produced for base, by gen_visit_members() for each member,
>> - # and if variants were present
>> - if base or members or variants:
>> + # 'goto out' produced for non-empty base, by gen_visit_members() for
>> + # each member, and if variants were present
>> + if (base and not base.is_empty()) or members or variants:
>> ret += mcgen('''
>>
>> out:
>
> Uh, sure this hunk belongs to this patch?
Unfortunately, yeah - because the empty base case doesn't generate a
goto. I'm leaning more and more towards not bothering to special case
empty types on the next round.
I've already started playing with making type.c_name() work on implicit
types - it generates names like '_obj_Foo_wrapper', which won't collide
(because we reserved leading underscore for our own use), but also
doesn't quite match conventional naming conventions - but as long as it
is used only in generated code, it isn't that bad. And with an implicit
type directly laid out, I can then just blindly call
visit_type_FOO_members(), even for implicit base or variant. The v4
spin of the second half of this series is looking promising...
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature