On 10/23/2015 09:06 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> 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 <ebl...@redhat.com> >> >> ---
>> 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
