On Tue, 4 Mar 2014 18:44:30 -0800 Wenchao Xia <wenchaoq...@gmail.com> 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 >