Re: [Qemu-devel] [PATCH v10 08/25] qapi-types: Refactor base fields output

[Eric Blake]

On 10/23/2015 09:06 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> Move code from gen_union() into gen_struct_fields() in order for
>> a later patch to share code when enumerating inherited fields
>> for struct types.
>>
>> No change to generated code.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>
>> ---

>>      if base:
>> -        ret += mcgen('''
>> -    /* Members inherited from %(c_name)s: */
>> -''',
>> -                     c_name=c_name(base.name))
>> -        ret += gen_struct_fields(base.members)
>> -        ret += mcgen('''
>> -    /* Own members: */
>> -''')
>> +        ret += gen_struct_fields([], base)
>>      else:
>>          ret += mcgen('''
>>      %(c_type)s kind;
> 
> One baby step towards
> 
>     gen_struct_fields(members, base);
> 
> which is where we want both gen_struct() and gen_union() to go.

Yep, it's almost like you're peering ahead to my unpublished patch queue :)

> 
> Slightly confusing: members are *local* members, while base.members are
> *all* members.  But the same exists elsewhere already, so let's not
> worry about it.

I am seriously debating about renaming things to
gen_visit_struct(...local_members), just to make it obvious that we have
only local members in that variable.  Separate patch, I can add it in
v11 if we need a respin, otherwise a later subset would be fine.

> 
> I kind of dislike gen_struct_fields() taking two parameters and calling
> itself for one, but I guess we can reconsider after gen_struct() and
> gen_union() are unified.  gen_struct_fields() might have just one caller
> then.

Yes, after my patch that unifies gen_struct/union into a single
gen_object(), there is just a single caller, so I could inline it at
that point.

I also debated whether to open-code the for loop over members, instead
of using a 1-deep recursive call with base=None, just so things are a
bit less magic.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature