Re: [Qemu-devel] [PATCH v8 06/17] qapi-types: Consolidate gen_struct() and gen_union()

[Eric Blake <eblake@redhat.com>]

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

On 10/30/2015 07:01 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> These two methods are now close enough that we can finally merge
>> them, relying on the fact that simple unions now provide a
>> reasonable local_members.  Change gen_struct() to gen_object()
>> that handles all forms of QAPISchemaObjectType, and rename and
>> shrink gen_union() to gen_variants() to handle the portion of
>> gen_object() needed when variants are present.
>>
>> gen_struct_fields() now has a single caller, so it no longer
>> needs an optional parameter; however, I did not choose to inline
>> it into the caller.
>>
>> No difference to generated code.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>

>>      def visit_object_type(self, name, info, base, members, variants):
>>          self._fwdecl += gen_fwd_object_or_array(name)
>> -        if variants:
>> -            if members:
>> -                assert len(members) == 1
>> -                assert members[0] == variants.tag_member
>> -            self.decl += gen_union(name, base, variants)
>> -        else:
>> -            self.decl += gen_struct(name, base, members)
>> +        self.decl += gen_object(name, base, members, variants)
>>          if base:
>>              self.decl += gen_upcast(name, base)
>>          self._gen_type_cleanup(name)
>> @@ -282,7 +268,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>>      def visit_alternate_type(self, name, info, variants):
>>          self._fwdecl += gen_fwd_object_or_array(name)
>>          self._fwdefn += gen_alternate_qtypes(name, variants)
>> -        self.decl += gen_union(name, None, variants)
>> +        self.decl += gen_object(name, None, [variants.tag_member], variants)
>>          self.decl += gen_alternate_qtypes_decl(name)
>>          self._gen_type_cleanup(name)
> 
> Turned out nicely.

Yes, I was pretty pleased with it.  It gives more credence to our choice
of introspection representation.

> 
> We could morph gen_struct_field() back into its original shape in a
> separate patch:
> 
>     def gen_struct_fields(members):
>         ret = ''
> 
>         for memb in members:
>             ret += gen_struct_field(memb.name, memb.type, memb.optional)
>         return ret
> 
> with calling code
> 
>     ret += mcgen('''
>     /* Members inherited from %(c_name)s: */
> ''',
>                  c_name=base.c_name())
>     ret += gen_struct_fields(base.members)
>     ret += mcgen('''
>     /* Own members: */
> ''')
>     ret += gen_struct_fields(local_members)
> 
> Matter of taste.

Reasonable idea; I'll add it in for the v9 spin.

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