Re: [Qemu-devel] [PATCH v2 15/19] qapi-visit: Move error check into gen_visit_members_call()

[Eric Blake <eblake@redhat.com>]

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

On 03/03/2016 04:56 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> 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 <eblake@redhat.com>
>>

>> +++ 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


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