[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 03/12] qapi: Lazy creation of array types
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v6 03/12] qapi: Lazy creation of array types |
Date: |
Tue, 06 Oct 2015 10:35:51 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 10/02/2015 02:06 AM, Markus Armbruster wrote:
>> Woot!
>>
>> Eric Blake <address@hidden> writes:
>>
>>> Commit ac88219a had several TODO markers about whether we needed
>>> to automatically create the corresponding array type alongside
>>> any other type. It turns out that most of the time, we don't!
>>>
>>> As part of lazy creation of array types, this patch now assigns
>>> an 'info' to array types at their point of first instantiation,
>>> rather than leaving it None.
>>
>> I guess our general for info should be:
>
> s/general/general description/ ?
General idea.
Looks like my typing engine doesn't run on all cylinders on Fridays...
>> For explicitly defined entities, info points to the (explicit)
>> definition. For implicitly defined entities, it points to a place that
>> triggers implicit definition. For some kinds of entities, multiple
>> places may exist, and info points to one of them.
>>
>
> Right now, we don't document any of the fields, but I can certainly add
> that comment.
>
>
>>
>> Here, you're talking about the effect on generated code. I'd put the
>> specific case of introspection last, in a paragraph of its own.
>>
>
> Improving the commit message is easier than redoing bad code :)
>
>>> +++ b/qapi-schema.json
>>> @@ -3396,6 +3396,16 @@
>>> 'features': 'int' } }
>>>
>>> ##
>>> +# @Dummy
>>> +#
>>> +# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally
>>> +#
>>> +# Since 2.5
>>> +##
>>> +{ 'struct': 'Dummy', 'data': { 'unused': ['X86CPUFeatureWordInfo'] } }
>>
>> DummyToForceArrays?
>
> Sure.
>
>>
>>> +
>>> +
>>> +##
>>> # @RxState:
>>> #
>>> # Packets receiving state
>>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>>> index 8123ab3..15640b6 100644
>>> --- a/scripts/qapi.py
>>> +++ b/scripts/qapi.py
>>> @@ -1143,7 +1143,7 @@ class QAPISchema(object):
>>> def _def_builtin_type(self, name, json_type, c_type, c_null):
>>> self._def_entity(QAPISchemaBuiltinType(name, json_type,
>>> c_type, c_null))
>>> - self._make_array_type(name) # TODO really needed?
>>> + self._make_array_type(name, None)
>>
>> Should we keep a TODO here?
>
> Sure, with better text explaining the guard issue on types shared in
> multiple qapi-types.h files.
>
>
>>> typ = self._make_implicit_object_type(typ, 'wrapper',
>>> - [self._make_member('data', typ)])
>>> + [self._make_member('data', typ,
>>> + info)])
>>
>> Consider a hanging indent here:
>>
>> typ = self._make_implicit_object_type(
>> typ, 'wrapper', [self._make_member('data', typ, info)])
>
>
> Okay. pep8 and pylint like it (I'm not used to doing it, but it does
> look better, and emacs didn't fight me too hard).
I'm still getting used to hanging indents myself. When in Rome...
- [Qemu-devel] [PATCH v6 12/12] RFC: qapi: Hide _info member, (continued)
- [Qemu-devel] [PATCH v6 12/12] RFC: qapi: Hide _info member, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v6 10/12] qapi: Correct error for union branch 'kind' clash, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v6 09/12] qapi: Defer duplicate enum value checks to schema check(), Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v6 01/12] qapi: Use predicate callback to determine visit filtering, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v6 03/12] qapi: Lazy creation of array types, Eric Blake, 2015/10/08