[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC v2 33/47] qapi: Clean up after recent conver
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH RFC v2 33/47] qapi: Clean up after recent conversions to QAPISchemaVisitor |
Date: |
Tue, 28 Jul 2015 15:13:22 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.1.0 |
On 07/28/2015 03:18 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> 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
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, (continued)
[Qemu-devel] [PATCH RFC v2 19/47] qapi: Generated code cleanup, Markus Armbruster, 2015/07/01
[Qemu-devel] [PATCH RFC v2 45/47] qapi: New QMP command query-schema for QMP schema introspection, Markus Armbruster, 2015/07/01