On 07/28/2015 03:18 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 07/01/2015 02:22 PM, Markus Armbruster wrote:
>>> Drop helper functions that are now unused.
>>>
>>> Make pylint reasonably happy.
>>>
>>> Rename generate_FOO() functions to gen_FOO() for consistency.
>>>
>>> Use more consistent and sensible variable names.
>>>
>>> Consistently use c_ for mapping keys when their value is a C
>>> identifier or type.
>>>
>>> Simplify gen_enum() and gen_visit_union()
>>>
>>> Consistently use single quotes for C text string literals.
>>
>> Not sure if it is worth splitting this into pieces.  Fortunately, there
>> are no changes to generated output.
> 
> I'm afraid splitting would increase churn without much gain.  If you
> want me to split, I can try.

I can live without the split; generated output being unchanged is
already a good sign.

>>> -def generate_command_decl(name, args, ret_type):
>>> -    arglist=""
>>> +def gen_command_decl(name, args, rets):
>>
>> I can see how 'args' is plural (even if it is a single string for the
>> name of a type containing the args), but should it be 'ret' instead of
>> 'rets'?
> 
> I now realize readers may find this odd.
> 
> The QMP specification talks about command arguments and command
> responses.
> 
> The QMP wire format uses 'arguments' and 'return'.
> 
> The schema language uses 'data' and 'returns'.  Near-perfect score on
> terminology inconsistency, as usual.  Anyway, 'data' is plural (and a
> rather unhelpful choice of syntax).  'returns' could either be the
> plural of the noun "return", or the third person singular of the verb
> "to return".
> 
> Permit me a philosophical digression.  The common way to do functions in
> programming is to have multiple arguments and a single return value.  I
> believe this is mostly common machines' calling conventions leaking into
> languages.  From a more abstract point of view, there's no structural
> difference between function arguments and values: both are simply an
> element of an arbitrary set (domain and codomain, respectively).  In
> particular, both can be tuples.
> 
> It's perfectly sane to have functions take exactly one argument and
> yield exactly one value.  Some functional languages work that way.
> 
> But when both argument and value are generally tuples anyway, as they
> are in QAPI/QMP, it's more natural to talk about arguments and return
> values.  I abbreviated to args and rets.  There's method to my madness
> ;)
> 
> I'm open to better ideas on terminology.

Not sure I'm thinking of anything better; so while I found it unusual,
the explanation helps and I certainly won't reject it as wrong.

>>> -def generate_visit_struct_fields(name, members, base = None):
>>> +def gen_visit_struct_fields(name, base, members):
>>>      struct_fields_seen.add(name)
>>
>>> -                     type=base.c_name(), c_name=c_name('base'))
>>> +                     c_type=base.c_name(), c_name=c_name('base'))
>>
>> Possible churn here based on my earlier comments about c_name(constant)
>> being constant.
> 
> I'm leaning towards leaving it as is just to keep the code similar to
> other places generating visit_type_FOO() calls.

And I already easily got rid of it in my followup RFC patches, so no
problem if you leave it as is for the sake of getting this series in.

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