Re: [Qemu-devel] [PATCH V2 0/8] qapi script: support enum as discriminator and better enum name

[Wenchao Xia <xiawenc@linux.vnet.ibm.com>]

于 2013/11/28 22:24, Luiz Capitulino 写道:
> On Thu, 28 Nov 2013 14:19:48 +0800
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>
>> 于 2013/11/26 0:47, Luiz Capitulino 写道:
>>> On Wed, 13 Nov 2013 06:25:00 +0800
>>> Wenchao Xia <xiawenc@linux.vnet.ibm.com> wrote:
>>>
>>>> This series is respined from RFC series at:
>>>> http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html
>>>>
>>>> Patch 1-6 add support for enum as discriminator.
>>>> Patch 7 improve enum name generation, now AIOContext->AIO_CONTEXT, X86CPU->
>>>> X86_CPU.
>>>> Patch 8 are the test cases.
>>>
>>> Can you please clarify what is the problem this series is trying to
>>> solve, how it does it and provide before/after type of examples?
>>>
>>> That's what I'd expect from an intro email, but this one has only a
>>> reference to an RFC series that has no better info, and some crypt
>>> changelog with magic numbers :(
>>>
>>> Besides, this doesn't apply anymore...
>>>
>>     Let me have an introduction:
>>
>> 1. support using enum as discriminator in union.
>> For example, if we have following define in qapi schema:
>> { 'enum': 'EnumOne',
>>     'data': [ 'value1', 'value2', 'value3' ] }
>>
>> { 'type': 'UserDefBase0',
>>     'data': { 'base-string0': 'str', 'base-enum0': 'EnumOne' } }
>>
>> Before this series, discriminator in union must be a string, and a
>> hidden enum type as discriminator is generated. After this series,
>> qapi schema can directly use predefined enum type:
>> { 'union': 'UserDefEnumDiscriminatorUnion',
>>     'base': 'UserDefBase0',
>>     'discriminator' : 'base-enum0',
>>     'data': { 'value1' : 'UserDefA',
>>               'value2' : 'UserDefInherit',
>>               'value3' : 'UserDefB' } }
>
> OK, now tell me who will benefit from this. You don't have to answer
> right now, you can resend your series with this information. Note that
> I'm not asking to be convinced, I'm asking to have enough info so that
> I can understand what this is about when I get to review this (and this
> is just what an intro email is about).
>

   OK, I will send v3 with better doc.

>>
>> The implement is done by:
>> 1.1 remember the enum defines by qapi scripts.(patch 1)
>> 1.2 use the remembered enum define to check correctness at compile
>> time.(patch 3)
>> 1.3 use the same enum name generation rule to avoid C code mismatch,
>> esp for "case [ENUM_VALUE]" in qapi-visit.c.(patch 4,5)
>> 1.4 when pre-defined enum type is used as discriminator, don't generate
>> a hidden enum type, switch the code path to support enum as
>> discriminator.
>> 1.5 test case shows how it looks like.(Patch 8)
>>
>> 2. Better enum name generation
>> Before this patch, AIOContext->A_I_O_Context, after this patch,
>> AIOContet->AIO_Context. Since previous patch has foldered enum
>> name generation codes into one function, it is done easily by modifying
>> it.(Patch 7)
>>
>>>>
>>>> Changes from RFC:
>>>>     Mainly address Eric's comments: fix typo, add patch 2 to allow partly mapping
>>>> enum value in union, add related test case, remove direct inherit support "_base"
>>>> and related test case.
>>>>
>>>> v2:
>>>>     General:
>>>>     3: use Raise exception instead of sys.error.write in qapi.py.
>>>>     Address Eric's comments:
>>>>     2,3: more check for enum value at compile time.
>>>>     8: correspond test case change.
>>>>
>>>> Wenchao Xia (8):
>>>>     1 qapi script: remember enum values
>>>>     2 qapi script: add check for duplicated key
>>>>     3 qapi script: check correctness of discriminator values in union
>>>>     4 qapi script: code move for generate_enum_name()
>>>>     5 qapi script: use same function to generate enum string
>>>>     6 qapi script: not generate hidden enum type for pre-defined enum discriminator
>>>>     7 qapi script: do not add "_" for every capitalized char in enum
>>>>     8 tests: add cases for inherited struct and union with discriminator
>>>>
>>>>    include/qapi/qmp/qerror.h               |    2 +-
>>>>    scripts/qapi-types.py                   |   34 ++++----
>>>>    scripts/qapi-visit.py                   |   55 +++++++++--
>>>>    scripts/qapi.py                         |   84 ++++++++++++++++-
>>>>    target-i386/cpu.c                       |    2 +-
>>>>    tests/qapi-schema/comments.out          |    2 +-
>>>>    tests/qapi-schema/qapi-schema-test.json |   27 ++++++
>>>>    tests/qapi-schema/qapi-schema-test.out  |   15 +++-
>>>>    tests/test-qmp-input-visitor.c          |  120 +++++++++++++++++++++++++
>>>>    tests/test-qmp-output-visitor.c         |  149 +++++++++++++++++++++++++++++++
>>>>    10 files changed, 454 insertions(+), 36 deletions(-)
>>>>
>>>
>>
>
>