Message ID | 20180321115211.17937-4-marcandre.lureau@redhat.com |
---|---|
State | New |
Headers | show |
Series | qapi: add #if pre-processor conditions to generated code | expand |
Marc-André Lureau <marcandre.lureau@redhat.com> writes: > Accept 'if' key in top-level elements, accepted as string or list of > string type. The following patches will modify the test visitor to > check the value is correctly saved, and generate #if/#endif code (as a > single #if/endif line or a series for a list). > > Example of 'if' key: > { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' }, > 'if': 'defined(TEST_IF_STRUCT)' } > > The generated code is for now *unconditional*. Later patches generate > the conditionals. > > A following patch for qapi-code-gen.txt will provide more complete > documentation for 'if' usage. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Reviewed-by: Markus Armbruster <armbru@redhat.com> [...] > diff --git a/tests/Makefile.include b/tests/Makefile.include > index 0b277036df..f77ad2ba3b 100644 > --- a/tests/Makefile.include > +++ b/tests/Makefile.include > @@ -436,6 +436,10 @@ qapi-schema += args-unknown.json > qapi-schema += bad-base.json > qapi-schema += bad-data.json > qapi-schema += bad-ident.json > +qapi-schema += bad-if.json > +qapi-schema += bad-if-empty.json > +qapi-schema += bad-if-empty-list.json > +qapi-schema += bad-if-list.json > qapi-schema += bad-type-bool.json > qapi-schema += bad-type-dict.json > qapi-schema += bad-type-int.json > @@ -933,6 +937,8 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json > "TEST","$*.out") > @# Sanitize error messages (make them independent of build directory) > @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -u $(SRC_PATH)/$*.err - > + @if test "$(QAPI_REGENERATE)" == 1 ; then cp $*.test.out $(SRC_PATH)/$*.out ; fi > + @if test "$(QAPI_REGENERATE)" == 1 ; then cp $*.test.err $(SRC_PATH)/$*.err ; fi > @diff -u $(SRC_PATH)/$*.out $*.test.out > @diff -u $(SRC_PATH)/$*.exit $*.test.exit > Huh? [...]
On Tue, Jun 19, 2018 at 9:57 AM, Markus Armbruster <armbru@redhat.com> wrote: > Marc-André Lureau <marcandre.lureau@redhat.com> writes: > >> Accept 'if' key in top-level elements, accepted as string or list of >> string type. The following patches will modify the test visitor to >> check the value is correctly saved, and generate #if/#endif code (as a >> single #if/endif line or a series for a list). >> >> Example of 'if' key: >> { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' }, >> 'if': 'defined(TEST_IF_STRUCT)' } >> >> The generated code is for now *unconditional*. Later patches generate >> the conditionals. >> >> A following patch for qapi-code-gen.txt will provide more complete >> documentation for 'if' usage. >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> Reviewed-by: Markus Armbruster <armbru@redhat.com> > [...] >> diff --git a/tests/Makefile.include b/tests/Makefile.include >> index 0b277036df..f77ad2ba3b 100644 >> --- a/tests/Makefile.include >> +++ b/tests/Makefile.include >> @@ -436,6 +436,10 @@ qapi-schema += args-unknown.json >> qapi-schema += bad-base.json >> qapi-schema += bad-data.json >> qapi-schema += bad-ident.json >> +qapi-schema += bad-if.json >> +qapi-schema += bad-if-empty.json >> +qapi-schema += bad-if-empty-list.json >> +qapi-schema += bad-if-list.json >> qapi-schema += bad-type-bool.json >> qapi-schema += bad-type-dict.json >> qapi-schema += bad-type-int.json >> @@ -933,6 +937,8 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json >> "TEST","$*.out") >> @# Sanitize error messages (make them independent of build directory) >> @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -u $(SRC_PATH)/$*.err - >> + @if test "$(QAPI_REGENERATE)" == 1 ; then cp $*.test.out $(SRC_PATH)/$*.out ; fi >> + @if test "$(QAPI_REGENERATE)" == 1 ; then cp $*.test.err $(SRC_PATH)/$*.err ; fi >> @diff -u $(SRC_PATH)/$*.out $*.test.out >> @diff -u $(SRC_PATH)/$*.exit $*.test.exit >> > > Huh? Oops, leftover. Do you have an alternative? I could propose it as a seperate patch if it's generally useful.
Marc-André Lureau <marcandre.lureau@gmail.com> writes: > On Tue, Jun 19, 2018 at 9:57 AM, Markus Armbruster <armbru@redhat.com> wrote: >> Marc-André Lureau <marcandre.lureau@redhat.com> writes: >> >>> Accept 'if' key in top-level elements, accepted as string or list of >>> string type. The following patches will modify the test visitor to >>> check the value is correctly saved, and generate #if/#endif code (as a >>> single #if/endif line or a series for a list). >>> >>> Example of 'if' key: >>> { 'struct': 'TestIfStruct', 'data': { 'foo': 'int' }, >>> 'if': 'defined(TEST_IF_STRUCT)' } >>> >>> The generated code is for now *unconditional*. Later patches generate >>> the conditionals. >>> >>> A following patch for qapi-code-gen.txt will provide more complete >>> documentation for 'if' usage. >>> >>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >>> Reviewed-by: Markus Armbruster <armbru@redhat.com> >> [...] >>> diff --git a/tests/Makefile.include b/tests/Makefile.include >>> index 0b277036df..f77ad2ba3b 100644 >>> --- a/tests/Makefile.include >>> +++ b/tests/Makefile.include >>> @@ -436,6 +436,10 @@ qapi-schema += args-unknown.json >>> qapi-schema += bad-base.json >>> qapi-schema += bad-data.json >>> qapi-schema += bad-ident.json >>> +qapi-schema += bad-if.json >>> +qapi-schema += bad-if-empty.json >>> +qapi-schema += bad-if-empty-list.json >>> +qapi-schema += bad-if-list.json >>> qapi-schema += bad-type-bool.json >>> qapi-schema += bad-type-dict.json >>> qapi-schema += bad-type-int.json >>> @@ -933,6 +937,8 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json >>> "TEST","$*.out") >>> @# Sanitize error messages (make them independent of build directory) >>> @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -u $(SRC_PATH)/$*.err - >>> + @if test "$(QAPI_REGENERATE)" == 1 ; then cp $*.test.out $(SRC_PATH)/$*.out ; fi >>> + @if test "$(QAPI_REGENERATE)" == 1 ; then cp $*.test.err $(SRC_PATH)/$*.err ; fi >>> @diff -u $(SRC_PATH)/$*.out $*.test.out >>> @diff -u $(SRC_PATH)/$*.exit $*.test.exit >>> >> >> Huh? > > Oops, leftover. Do you have an alternative? I could propose it as a > seperate patch if it's generally useful. I guess what you're trying to do here is updating expected test output. So far, everybody does that by hand. A make target to do that (check-accept?) would be nice. Trivial to do if we had a single test harness, but of course we have many. My R-by stands with this hunk dropped.
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index 2c05e3c284..4f9e02b0b4 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -637,6 +637,27 @@ def add_name(name, info, meta, implicit=False): all_names[name] = meta +def check_if(expr, info): + + def check_if_str(ifcond, info): + if not isinstance(ifcond, str): + raise QAPISemError( + info, "'if' condition must be a string or a list of strings") + if ifcond == '': + raise QAPISemError(info, "'if' condition '' makes no sense") + + ifcond = expr.get('if') + if ifcond is None: + return + if isinstance(ifcond, list): + if ifcond == []: + raise QAPISemError(info, "'if' condition [] is useless") + for elt in ifcond: + check_if_str(elt, info) + else: + check_if_str(ifcond, info) + + def check_type(info, source, value, allow_array=False, allow_dict=False, allow_optional=False, allow_metas=[]): @@ -876,6 +897,8 @@ def check_keys(expr_elem, meta, required, optional=[]): raise QAPISemError(info, "'%s' of %s '%s' should only use true value" % (key, meta, name)) + if key == 'if': + check_if(expr, info) for key in required: if key not in expr: raise QAPISemError(info, "Key '%s' is missing from %s '%s'" @@ -904,28 +927,28 @@ def check_exprs(exprs): if 'enum' in expr: meta = 'enum' - check_keys(expr_elem, 'enum', ['data'], ['prefix']) + check_keys(expr_elem, 'enum', ['data'], ['if', 'prefix']) enum_types[expr[meta]] = expr elif 'union' in expr: meta = 'union' check_keys(expr_elem, 'union', ['data'], - ['base', 'discriminator']) + ['base', 'discriminator', 'if']) union_types[expr[meta]] = expr elif 'alternate' in expr: meta = 'alternate' - check_keys(expr_elem, 'alternate', ['data']) + check_keys(expr_elem, 'alternate', ['data'], ['if']) elif 'struct' in expr: meta = 'struct' - check_keys(expr_elem, 'struct', ['data'], ['base']) + check_keys(expr_elem, 'struct', ['data'], ['base', 'if']) struct_types[expr[meta]] = expr elif 'command' in expr: meta = 'command' check_keys(expr_elem, 'command', [], ['data', 'returns', 'gen', 'success-response', - 'boxed', 'allow-oob']) + 'boxed', 'allow-oob', 'if']) elif 'event' in expr: meta = 'event' - check_keys(expr_elem, 'event', [], ['data', 'boxed']) + check_keys(expr_elem, 'event', [], ['data', 'boxed', 'if']) else: raise QAPISemError(expr_elem['info'], "Expression is missing metatype") diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c index 93fbbb1b73..c25fc2100a 100644 --- a/tests/test-qmp-cmds.c +++ b/tests/test-qmp-cmds.c @@ -12,6 +12,12 @@ static QmpCommandList qmp_commands; +/* #if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD) */ +void qmp_TestIfCmd(TestIfStruct *foo, Error **errp) +{ +} +/* #endif */ + void qmp_user_def_cmd(Error **errp) { } diff --git a/tests/Makefile.include b/tests/Makefile.include index 0b277036df..f77ad2ba3b 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -436,6 +436,10 @@ qapi-schema += args-unknown.json qapi-schema += bad-base.json qapi-schema += bad-data.json qapi-schema += bad-ident.json +qapi-schema += bad-if.json +qapi-schema += bad-if-empty.json +qapi-schema += bad-if-empty-list.json +qapi-schema += bad-if-list.json qapi-schema += bad-type-bool.json qapi-schema += bad-type-dict.json qapi-schema += bad-type-int.json @@ -933,6 +937,8 @@ $(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json "TEST","$*.out") @# Sanitize error messages (make them independent of build directory) @perl -p -e 's|\Q$(SRC_PATH)\E/||g' $*.test.err | diff -u $(SRC_PATH)/$*.err - + @if test "$(QAPI_REGENERATE)" == 1 ; then cp $*.test.out $(SRC_PATH)/$*.out ; fi + @if test "$(QAPI_REGENERATE)" == 1 ; then cp $*.test.err $(SRC_PATH)/$*.err ; fi @diff -u $(SRC_PATH)/$*.out $*.test.out @diff -u $(SRC_PATH)/$*.exit $*.test.exit diff --git a/tests/qapi-schema/bad-if-empty-list.err b/tests/qapi-schema/bad-if-empty-list.err new file mode 100644 index 0000000000..75fe6497bc --- /dev/null +++ b/tests/qapi-schema/bad-if-empty-list.err @@ -0,0 +1 @@ +tests/qapi-schema/bad-if-empty-list.json:2: 'if' condition [] is useless diff --git a/tests/qapi-schema/bad-if-empty-list.exit b/tests/qapi-schema/bad-if-empty-list.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/bad-if-empty-list.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/bad-if-empty-list.json b/tests/qapi-schema/bad-if-empty-list.json new file mode 100644 index 0000000000..94f2eb8670 --- /dev/null +++ b/tests/qapi-schema/bad-if-empty-list.json @@ -0,0 +1,3 @@ +# check empty 'if' list +{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' }, + 'if': [] } diff --git a/tests/qapi-schema/bad-if-empty-list.out b/tests/qapi-schema/bad-if-empty-list.out new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/bad-if-empty.err b/tests/qapi-schema/bad-if-empty.err new file mode 100644 index 0000000000..358bdc3e51 --- /dev/null +++ b/tests/qapi-schema/bad-if-empty.err @@ -0,0 +1 @@ +tests/qapi-schema/bad-if-empty.json:2: 'if' condition '' makes no sense diff --git a/tests/qapi-schema/bad-if-empty.exit b/tests/qapi-schema/bad-if-empty.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/bad-if-empty.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/bad-if-empty.json b/tests/qapi-schema/bad-if-empty.json new file mode 100644 index 0000000000..fe1dd4eca6 --- /dev/null +++ b/tests/qapi-schema/bad-if-empty.json @@ -0,0 +1,3 @@ +# check empty 'if' +{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' }, + 'if': '' } diff --git a/tests/qapi-schema/bad-if-empty.out b/tests/qapi-schema/bad-if-empty.out new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/bad-if-list.err b/tests/qapi-schema/bad-if-list.err new file mode 100644 index 0000000000..0af6316f78 --- /dev/null +++ b/tests/qapi-schema/bad-if-list.err @@ -0,0 +1 @@ +tests/qapi-schema/bad-if-list.json:2: 'if' condition '' makes no sense diff --git a/tests/qapi-schema/bad-if-list.exit b/tests/qapi-schema/bad-if-list.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/bad-if-list.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/bad-if-list.json b/tests/qapi-schema/bad-if-list.json new file mode 100644 index 0000000000..49ced9b9ca --- /dev/null +++ b/tests/qapi-schema/bad-if-list.json @@ -0,0 +1,3 @@ +# check invalid 'if' content +{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' }, + 'if': ['foo', ''] } diff --git a/tests/qapi-schema/bad-if-list.out b/tests/qapi-schema/bad-if-list.out new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/bad-if.err b/tests/qapi-schema/bad-if.err new file mode 100644 index 0000000000..c2e3f5f44c --- /dev/null +++ b/tests/qapi-schema/bad-if.err @@ -0,0 +1 @@ +tests/qapi-schema/bad-if.json:2: 'if' condition must be a string or a list of strings diff --git a/tests/qapi-schema/bad-if.exit b/tests/qapi-schema/bad-if.exit new file mode 100644 index 0000000000..d00491fd7e --- /dev/null +++ b/tests/qapi-schema/bad-if.exit @@ -0,0 +1 @@ +1 diff --git a/tests/qapi-schema/bad-if.json b/tests/qapi-schema/bad-if.json new file mode 100644 index 0000000000..3edd1a0bf2 --- /dev/null +++ b/tests/qapi-schema/bad-if.json @@ -0,0 +1,3 @@ +# check invalid 'if' type +{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' }, + 'if': { 'value': 'defined(TEST_IF_STRUCT)' } } diff --git a/tests/qapi-schema/bad-if.out b/tests/qapi-schema/bad-if.out new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index c72dbd8050..b997b2d43d 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -188,3 +188,23 @@ 'data': { 'a': ['__org.qemu_x-Enum'], 'b': ['__org.qemu_x-Struct'], 'c': '__org.qemu_x-Union2', 'd': '__org.qemu_x-Alt' }, 'returns': '__org.qemu_x-Union1' } + +# test 'if' condition handling + +{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' }, + 'if': 'defined(TEST_IF_STRUCT)' } + +{ 'enum': 'TestIfEnum', 'data': [ 'foo', 'bar' ], + 'if': 'defined(TEST_IF_ENUM)' } + +{ 'union': 'TestIfUnion', 'data': { 'foo': 'TestStruct' }, + 'if': 'defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)' } + +{ 'alternate': 'TestIfAlternate', 'data': { 'foo': 'int', 'bar': 'TestStruct' }, + 'if': 'defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)' } + +{ 'command': 'TestIfCmd', 'data': { 'foo': 'TestIfStruct' }, + 'if': ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] } + +{ 'event': 'TestIfEvent', 'data': { 'foo': 'TestIfStruct' }, + 'if': 'defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)' } diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 012e7fc06a..b11682314c 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -230,3 +230,25 @@ object q_obj___org.qemu_x-command-arg member d: __org.qemu_x-Alt optional=False command __org.qemu_x-command q_obj___org.qemu_x-command-arg -> __org.qemu_x-Union1 gen=True success_response=True boxed=False +object TestIfStruct + member foo: int optional=False +enum TestIfEnum ['foo', 'bar'] +object q_obj_TestStruct-wrapper + member data: TestStruct optional=False +enum TestIfUnionKind ['foo'] +object TestIfUnion + member type: TestIfUnionKind optional=False + tag type + case foo: q_obj_TestStruct-wrapper +alternate TestIfAlternate + tag type + case foo: int + case bar: TestStruct +object q_obj_TestIfCmd-arg + member foo: TestIfStruct optional=False +command TestIfCmd q_obj_TestIfCmd-arg -> None + gen=True success_response=True boxed=False +object q_obj_TestIfEvent-arg + member foo: TestIfStruct optional=False +event TestIfEvent q_obj_TestIfEvent-arg + boxed=False