[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discrimin
From: |
Luiz Capitulino |
Subject: |
Re: [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name |
Date: |
Fri, 7 Mar 2014 08:55:02 -0500 |
On Tue, 4 Mar 2014 18:44:30 -0800
Wenchao Xia <address@hidden> wrote:
> This series address two issues:
>
> 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' } }
>
> The benefit is that every thing is defined explicitly in schema file,
> the discriminator enum type can be used in other API define in schema,
> and a compile time check will be put to verify the correctness according
> to enum define. Currently BlockdevOptions used discriminator which can
> be converted, in the future other union can also use enum discriminator.
I've applied this series to the qmp tree, with the new 07/10 you sent.
>
> 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), more strict check(patch 2)
> 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 switch the code path, when pre-defined enum type is used as discriminator,
> don't generate a hidden enum type, use the enum type instead, add
> docs/qapi-code-gen.txt.(Patch 6)
> 1.5 test case shows how it looks like.(Patch 7)
> 1.6 convert BlockdevOptions. (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 9)
>
>
> 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. RFC series at:
> http://lists.nongnu.org/archive/html/qemu-devel/2013-11/msg00363.html
>
> v2:
> General:
> 3/8: use Raise exception instead of sys.error.write in qapi.py.
> Address Eric's comments:
> 2/8,3/8: more check for enum value at compile time, not allow partly
> mapping.
> 8/8: correspond test case change.
>
> v3:
> General:
> move enum name generation patch to last in the series, add convert patch
> 8/9.
> Address Luiz and Kevin's comments:
> Better introduction.
> 6/9: renamed this patch, add docs/qapi-code-gen.txt part.
>
> v4:
> Address Eric's comments:
> 5/9: better commit message.
> 6/9: typo fix in doc.
> 9/9: typo fix, fix indentation, better incode comment.
>
> v5:
> Address Eric's comments:
> 6/10: doc typo fix.
> 8/10: new patch to remove string discriminator.
> 9/10: removed the string discriminator test case.
>
> v6:
> rebased on upstream by adding "blgdebug" and "blkverify" in qapi-schema.json
> in patch 7/10.
>
> v7:
> The series is rebased on Markus's tree:
> git://repo.or.cz/qemu/armbru.git branch qapi-scripts
>
> Address Markus's comments:
> 2/11: typo fix in error message.
> 3/11: new patch to recording addtional line info in schema parsing.
> 4/11: move the check into qapi.py, report better with fp/line info.
> 7/11: move the UnionKind adding in qapi.py after 1st parsing and
> with better error info.
> 9/11: move the check into qapi.py with fp/line info, test case
> corresond change.
> 11/11: new patch for errpr path test case.
>
> v8:
> The series is ontop of Markus's tree and rebased on upstream:
> Address Markus's comments:
> 1/10: better commit title, simplify is_enum().
> 2/10: test case squashed into this patch.
> 3/10: no change, column computation is not touched.
> 4/10: simplify commit title and message, refine the semantic check
> logic as comments in v7, check in discriminator_find_enum_define() is moved
> out so that the function can be used without error info, squash related test
> into this patch, re-orgnize 'expr_elem' with separate 'info' member,
> QAPIExprError now takes only info to work, better error message, remove
> check of whether all enum values are covered by branch, use expr['key']
> instead of expr.get('key') when possible.
> 6/10: remove useless comments in generate_enum_full_value(), make line
> shorter by change variable name.
> 7/10: building 'expr_elem' for discriminator_find_enum_define() is removed,
> make line shorter in qapi-visit.py, add a test case that enum is used before
> define.
> 8/10: rebased on uptream by adding 'quorum' driver type.
> 9/10: better doc and error message, add a test case for string
> discriminator.
> use string concatenation rather than line continuation for string in C code.
>
> v9:
> Address Eric's comments:
> Solve double SoB in commit messages.
> 2/10: typo fix in commit message.
> 4/10: add expr change in commit message, typo fix in comments for
> discriminator_find_enum_define() and move it to patch 7/10.
> 9/10: better commit message.
>
> Address Markus's comments:
> 4/10: add back commit message, discriminator_find_enum_define() is moved to
> patch 7/10, since no caller in this patch now, simplify check for flat union.
> Improve comments for TODO in check for every branch, remove unused define in
> test schema.
>
> Wenchao Xia (10):
> 1 qapi script: remember explicitly defined enum values
> 2 qapi script: add check for duplicated key
> 3 qapi script: remember line number in schema parsing
> 4 qapi script: check correctness of union
> 5 qapi script: code move for generate_enum_name()
> 6 qapi script: use same function to generate enum string
> 7 qapi script: support enum type as discriminator in union
> 8 qapi: convert BlockdevOptions to use enum discriminator
> 9 qapi script: do not allow string discriminator
> 10 qapi script: do not add "_" for every capitalized char in enum
>
> docs/qapi-code-gen.txt | 5 +-
> include/qapi/qmp/qerror.h | 2 +-
> qapi-schema.json | 14 ++-
> scripts/qapi-types.py | 34 ++--
> scripts/qapi-visit.py | 42 ++++--
> scripts/qapi.py | 184
> ++++++++++++++++++--
> target-i386/cpu.c | 2 +-
> tests/Makefile | 6 +-
> tests/qapi-schema/comments.out | 2 +-
> tests/qapi-schema/duplicate-key.err | 1 +
> tests/qapi-schema/duplicate-key.exit | 1 +
> tests/qapi-schema/duplicate-key.json | 2 +
> .../qapi-schema/flat-union-invalid-branch-key.err | 1 +
> .../qapi-schema/flat-union-invalid-branch-key.exit | 1 +
> .../qapi-schema/flat-union-invalid-branch-key.json | 17 ++
> .../flat-union-invalid-discriminator.err | 1 +
> .../flat-union-invalid-discriminator.exit | 1 +
> .../flat-union-invalid-discriminator.json | 17 ++
> tests/qapi-schema/flat-union-no-base.err | 1 +
> tests/qapi-schema/flat-union-no-base.exit | 1 +
> tests/qapi-schema/flat-union-no-base.json | 10 +
> tests/qapi-schema/flat-union-reverse-define.exit | 1 +
> tests/qapi-schema/flat-union-reverse-define.json | 17 ++
> tests/qapi-schema/flat-union-reverse-define.out | 9 +
> .../flat-union-string-discriminator.err | 1 +
> .../flat-union-string-discriminator.exit | 1 +
> .../flat-union-string-discriminator.json | 17 ++
> tests/qapi-schema/qapi-schema-test.json | 9 +-
> tests/qapi-schema/qapi-schema-test.out | 13 +-
> tests/qapi-schema/union-invalid-base.err | 1 +
> tests/qapi-schema/union-invalid-base.exit | 1 +
> tests/qapi-schema/union-invalid-base.json | 10 +
> tests/test-qmp-input-strict.c | 5 +-
> tests/test-qmp-input-visitor.c | 10 +-
> tests/test-qmp-output-visitor.c | 10 +-
> 35 files changed, 383 insertions(+), 67 deletions(-)
> create mode 100644 tests/qapi-schema/duplicate-key.err
> create mode 100644 tests/qapi-schema/duplicate-key.exit
> create mode 100644 tests/qapi-schema/duplicate-key.json
> create mode 100644 tests/qapi-schema/duplicate-key.out
> create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.err
> create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.exit
> create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.json
> create mode 100644 tests/qapi-schema/flat-union-invalid-branch-key.out
> create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.err
> create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.exit
> create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.json
> create mode 100644 tests/qapi-schema/flat-union-invalid-discriminator.out
> create mode 100644 tests/qapi-schema/flat-union-no-base.err
> create mode 100644 tests/qapi-schema/flat-union-no-base.exit
> create mode 100644 tests/qapi-schema/flat-union-no-base.json
> create mode 100644 tests/qapi-schema/flat-union-no-base.out
> create mode 100644 tests/qapi-schema/flat-union-reverse-define.err
> create mode 100644 tests/qapi-schema/flat-union-reverse-define.exit
> create mode 100644 tests/qapi-schema/flat-union-reverse-define.json
> create mode 100644 tests/qapi-schema/flat-union-reverse-define.out
> create mode 100644 tests/qapi-schema/flat-union-string-discriminator.err
> create mode 100644 tests/qapi-schema/flat-union-string-discriminator.exit
> create mode 100644 tests/qapi-schema/flat-union-string-discriminator.json
> create mode 100644 tests/qapi-schema/flat-union-string-discriminator.out
> create mode 100644 tests/qapi-schema/union-invalid-base.err
> create mode 100644 tests/qapi-schema/union-invalid-base.exit
> create mode 100644 tests/qapi-schema/union-invalid-base.json
> create mode 100644 tests/qapi-schema/union-invalid-base.out
>
- Re: [Qemu-devel] [PATCH V9 07/10] qapi script: support enum type as discriminator in union, (continued)
[Qemu-devel] [PATCH V9 08/10] qapi: convert BlockdevOptions to use enum discriminator, Wenchao Xia, 2014/03/04
[Qemu-devel] [PATCH V9 09/10] qapi script: do not allow string discriminator, Wenchao Xia, 2014/03/04
[Qemu-devel] [PATCH V9 10/10] qapi script: do not add "_" for every capitalized char in enum, Wenchao Xia, 2014/03/04
Re: [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name, Markus Armbruster, 2014/03/06
Re: [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name, Markus Armbruster, 2014/03/06
Re: [Qemu-devel] [PATCH V9 00/10] qapi script: support enum as discriminator and better enum name,
Luiz Capitulino <=