[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
Re: [Qemu-devel] [PATCH RFC v2 33/47] qapi: Clean up after recent conversions to QAPISchemaVisitor
Tue, 28 Jul 2015 15:13:22 -0600
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
> I now realize readers may find this odd.
> The QMP specification talks about command arguments and command
> 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):
>>> - 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
Description: OpenPGP digital signature
[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
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, (continued)