| Message ID | 20180321115211.17937-12-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: > Wrap generated code with #if/#endif using an 'ifcontext' on > QAPIGenCSnippet objects. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > scripts/qapi/commands.py | 19 ++++++++++--------- > tests/test-qmp-cmds.c | 4 ++-- > 2 files changed, 12 insertions(+), 11 deletions(-) > > diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py > index e2366b4801..40bb680b7c 100644 > --- a/scripts/qapi/commands.py > +++ b/scripts/qapi/commands.py > @@ -237,7 +237,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor): > QAPISchemaModularCVisitor.__init__( > self, prefix, 'qapi-commands', > ' * Schema-defined QAPI/QMP commands', __doc__) > - self._regy = '' > + self._regy = QAPIGenCSnippet() Aha, here's the answer to my question on PATCH 08: you need QAPIGenCSnippet... > self._visited_ret_types = {} > > def _begin_module(self, name): > @@ -273,19 +273,20 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor): > void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds); > ''', > c_prefix=c_name(self._prefix, protect=False))) > - genc.add(gen_registry(self._regy, self._prefix)) > + genc.add(gen_registry(self._regy.get_content(), self._prefix)) > > def visit_command(self, name, info, ifcond, arg_type, ret_type, > gen, success_response, boxed, allow_oob): > if not gen: > return > - self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type)) > - if ret_type and ret_type not in self._visited_ret_types[self._genc]: > - self._visited_ret_types[self._genc].add(ret_type) > - self._genc.add(gen_marshal_output(ret_type)) > - self._genh.add(gen_marshal_decl(name)) > - self._genc.add(gen_marshal(name, arg_type, boxed, ret_type)) > - self._regy += gen_register_command(name, success_response, allow_oob) > + with ifcontext(ifcond, self._genh, self._genc, self._regy): ... so you can pass ._regy to with ifcontext. The name QAPIGenCSnippet makes sense for this role. Less so for its role as base class of QAPIGenC. Dunno... QAPIGenCCode? > + self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type)) > + if ret_type and ret_type not in self._visited_ret_types[self._genc]: pycodestyle-3 reports commands.py:284:80: E501 line too long (80 > 79 characters) > + self._visited_ret_types[self._genc].add(ret_type) > + self._genc.add(gen_marshal_output(ret_type)) > + self._genh.add(gen_marshal_decl(name)) > + self._genc.add(gen_marshal(name, arg_type, boxed, ret_type)) > + self._regy.add(gen_register_command(name, success_response, allow_oob)) pycodestyle-3 reports commands.py:289:80: E501 line too long (83 > 79 characters) > > > def gen_commands(schema, output_dir, prefix): > diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c > index c25fc2100a..e675722593 100644 > --- a/tests/test-qmp-cmds.c > +++ b/tests/test-qmp-cmds.c > @@ -12,11 +12,11 @@ > > static QmpCommandList qmp_commands; > > -/* #if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD) */ > +#if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD) > void qmp_TestIfCmd(TestIfStruct *foo, Error **errp) > { > } > -/* #endif */ > +#endif > > void qmp_user_def_cmd(Error **errp) > {
Marc-André Lureau <marcandre.lureau@redhat.com> writes: > Wrap generated code with #if/#endif using an 'ifcontext' on > QAPIGenCSnippet objects. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > scripts/qapi/commands.py | 19 ++++++++++--------- > tests/test-qmp-cmds.c | 4 ++-- > 2 files changed, 12 insertions(+), 11 deletions(-) > > diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py > index e2366b4801..40bb680b7c 100644 > --- a/scripts/qapi/commands.py > +++ b/scripts/qapi/commands.py > @@ -237,7 +237,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor): > QAPISchemaModularCVisitor.__init__( > self, prefix, 'qapi-commands', > ' * Schema-defined QAPI/QMP commands', __doc__) > - self._regy = '' > + self._regy = QAPIGenCSnippet() > self._visited_ret_types = {} > > def _begin_module(self, name): > @@ -273,19 +273,20 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor): > void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds); > ''', > c_prefix=c_name(self._prefix, protect=False))) > - genc.add(gen_registry(self._regy, self._prefix)) > + genc.add(gen_registry(self._regy.get_content(), self._prefix)) > > def visit_command(self, name, info, ifcond, arg_type, ret_type, > gen, success_response, boxed, allow_oob): > if not gen: > return > - self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type)) > - if ret_type and ret_type not in self._visited_ret_types[self._genc]: > - self._visited_ret_types[self._genc].add(ret_type) > - self._genc.add(gen_marshal_output(ret_type)) > - self._genh.add(gen_marshal_decl(name)) > - self._genc.add(gen_marshal(name, arg_type, boxed, ret_type)) > - self._regy += gen_register_command(name, success_response, allow_oob) > + with ifcontext(ifcond, self._genh, self._genc, self._regy): > + self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type)) > + if ret_type and ret_type not in self._visited_ret_types[self._genc]: > + self._visited_ret_types[self._genc].add(ret_type) > + self._genc.add(gen_marshal_output(ret_type)) I'm afraid this falls apart when multiple commands with different conditions share a return type. That case needs test coverage. Aside: the qmp_marshal_FOO() should be static. I can see just two uses preventing that: monitor.c:1146: qmp_marshal_qmp_capabilities, QCO_NO_OPTIONS); monitor.c:4312: qmp_marshal_query_version(NULL, &ver, NULL); Would be nice to get rid of those. Not necessarily in this series, of course. > + self._genh.add(gen_marshal_decl(name)) > + self._genc.add(gen_marshal(name, arg_type, boxed, ret_type)) > + self._regy.add(gen_register_command(name, success_response, allow_oob)) > > > def gen_commands(schema, output_dir, prefix): > diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c > index c25fc2100a..e675722593 100644 > --- a/tests/test-qmp-cmds.c > +++ b/tests/test-qmp-cmds.c > @@ -12,11 +12,11 @@ > > static QmpCommandList qmp_commands; > > -/* #if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD) */ > +#if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD) > void qmp_TestIfCmd(TestIfStruct *foo, Error **errp) > { > } > -/* #endif */ > +#endif > > void qmp_user_def_cmd(Error **errp) > {
Markus Armbruster <armbru@redhat.com> writes: > Marc-André Lureau <marcandre.lureau@redhat.com> writes: > >> Wrap generated code with #if/#endif using an 'ifcontext' on >> QAPIGenCSnippet objects. >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> >> --- >> scripts/qapi/commands.py | 19 ++++++++++--------- >> tests/test-qmp-cmds.c | 4 ++-- >> 2 files changed, 12 insertions(+), 11 deletions(-) >> >> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py >> index e2366b4801..40bb680b7c 100644 >> --- a/scripts/qapi/commands.py >> +++ b/scripts/qapi/commands.py >> @@ -237,7 +237,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor): >> QAPISchemaModularCVisitor.__init__( >> self, prefix, 'qapi-commands', >> ' * Schema-defined QAPI/QMP commands', __doc__) >> - self._regy = '' >> + self._regy = QAPIGenCSnippet() >> self._visited_ret_types = {} >> >> def _begin_module(self, name): >> @@ -273,19 +273,20 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor): >> void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds); >> ''', >> c_prefix=c_name(self._prefix, protect=False))) >> - genc.add(gen_registry(self._regy, self._prefix)) >> + genc.add(gen_registry(self._regy.get_content(), self._prefix)) >> >> def visit_command(self, name, info, ifcond, arg_type, ret_type, >> gen, success_response, boxed, allow_oob): >> if not gen: >> return >> - self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type)) >> - if ret_type and ret_type not in self._visited_ret_types[self._genc]: >> - self._visited_ret_types[self._genc].add(ret_type) >> - self._genc.add(gen_marshal_output(ret_type)) >> - self._genh.add(gen_marshal_decl(name)) >> - self._genc.add(gen_marshal(name, arg_type, boxed, ret_type)) >> - self._regy += gen_register_command(name, success_response, allow_oob) >> + with ifcontext(ifcond, self._genh, self._genc, self._regy): >> + self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type)) >> + if ret_type and ret_type not in self._visited_ret_types[self._genc]: >> + self._visited_ret_types[self._genc].add(ret_type) >> + self._genc.add(gen_marshal_output(ret_type)) > > I'm afraid this falls apart when multiple commands with different > conditions share a return type. If you'd rather fix this later, we can consider just documenting the bug for now. > That case needs test coverage. Needed regardless. [...]
diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py index e2366b4801..40bb680b7c 100644 --- a/scripts/qapi/commands.py +++ b/scripts/qapi/commands.py @@ -237,7 +237,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor): QAPISchemaModularCVisitor.__init__( self, prefix, 'qapi-commands', ' * Schema-defined QAPI/QMP commands', __doc__) - self._regy = '' + self._regy = QAPIGenCSnippet() self._visited_ret_types = {} def _begin_module(self, name): @@ -273,19 +273,20 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor): void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds); ''', c_prefix=c_name(self._prefix, protect=False))) - genc.add(gen_registry(self._regy, self._prefix)) + genc.add(gen_registry(self._regy.get_content(), self._prefix)) def visit_command(self, name, info, ifcond, arg_type, ret_type, gen, success_response, boxed, allow_oob): if not gen: return - self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type)) - if ret_type and ret_type not in self._visited_ret_types[self._genc]: - self._visited_ret_types[self._genc].add(ret_type) - self._genc.add(gen_marshal_output(ret_type)) - self._genh.add(gen_marshal_decl(name)) - self._genc.add(gen_marshal(name, arg_type, boxed, ret_type)) - self._regy += gen_register_command(name, success_response, allow_oob) + with ifcontext(ifcond, self._genh, self._genc, self._regy): + self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type)) + if ret_type and ret_type not in self._visited_ret_types[self._genc]: + self._visited_ret_types[self._genc].add(ret_type) + self._genc.add(gen_marshal_output(ret_type)) + self._genh.add(gen_marshal_decl(name)) + self._genc.add(gen_marshal(name, arg_type, boxed, ret_type)) + self._regy.add(gen_register_command(name, success_response, allow_oob)) def gen_commands(schema, output_dir, prefix): diff --git a/tests/test-qmp-cmds.c b/tests/test-qmp-cmds.c index c25fc2100a..e675722593 100644 --- a/tests/test-qmp-cmds.c +++ b/tests/test-qmp-cmds.c @@ -12,11 +12,11 @@ static QmpCommandList qmp_commands; -/* #if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD) */ +#if defined(TEST_IF_STRUCT) && defined(TEST_IF_CMD) void qmp_TestIfCmd(TestIfStruct *foo, Error **errp) { } -/* #endif */ +#endif void qmp_user_def_cmd(Error **errp) {
Wrap generated code with #if/#endif using an 'ifcontext' on QAPIGenCSnippet objects. Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- scripts/qapi/commands.py | 19 ++++++++++--------- tests/test-qmp-cmds.c | 4 ++-- 2 files changed, 12 insertions(+), 11 deletions(-)