[v10,08/25] qapi-types: Refactor base fields output

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 <eblake@redhat.com>

---
v10: new patch, split off of 5/17
---
 scripts/qapi-types.py | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Eric Blake <eblake@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 <eblake@redhat.com>
>
> ---
> v10: new patch, split off of 5/17
> ---
>  scripts/qapi-types.py | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index 4fe618e..a97cc10 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -51,9 +51,19 @@ def gen_struct_field(name, typ, optional):
>      return ret
>
>
> -def gen_struct_fields(members):
> +def gen_struct_fields(members, base=None):
>      ret = ''
>
> +    if base:
> +        ret += mcgen('''
> +    /* Members inherited from %(c_name)s: */
> +''',
> +                     c_name=base.c_name())
> +        ret += gen_struct_fields(base.members)
> +        ret += mcgen('''
> +    /* Own members: */
> +''')
> +
>      for memb in members:
>          ret += gen_struct_field(memb.name, memb.type, memb.optional)
>      return ret
> @@ -126,14 +136,7 @@ struct %(c_name)s {
>  ''',
>                  c_name=c_name(name))
>      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.

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

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