Hi ----- Original Message ----- > Marc-André Lureau <marcandre.lur...@redhat.com> writes: > > > Wrap generated code with #if/#endif using the ifcond_decorator. > > > > Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com> > > --- > > scripts/qapi-commands.py | 2 ++ > > tests/test-qmp-commands.c | 4 ++-- > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > > index 5eb96b2d95..db45415c41 100644 > > --- a/scripts/qapi-commands.py > > +++ b/scripts/qapi-commands.py > > @@ -228,6 +228,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor): > > self.defn = None > > self._regy = None > > self._visited_ret_types = None > > + self.if_members = ['decl', 'defn', '_regy'] > > > > def visit_begin(self, schema): > > self.decl = '' > > @@ -240,6 +241,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor): > > self._regy = None > > self._visited_ret_types = None > > > > + @ifcond_decorator > > def visit_command(self, name, info, arg_type, ret_type, > > gen, success_response, boxed, ifcond): > > if not gen: > > diff --git a/tests/test-qmp-commands.c b/tests/test-qmp-commands.c > > index 9b9a7ffee7..ad7b6e4e1d 100644 > > --- a/tests/test-qmp-commands.c > > +++ b/tests/test-qmp-commands.c > > @@ -10,11 +10,11 @@ > > > > static QmpCommandList qmp_commands; > > > > -/* #if defined(TEST_IF_CMD) */ > > +#if defined(TEST_IF_CMD) > > void qmp_TestIfCmd(TestIfStruct *foo, Error **errp) > > { > > } > > -/* #endif */ > > +#endif > > > > void qmp_user_def_cmd(Error **errp) > > { > > To find out whether the clever decorator is worth its keep, I replaced > it with the dumbest, most obvious code I could think of (appended). > Does this make visit_command() easier or harder to understand? > > > > diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py > index db45415c41..13d37d2ac2 100644 > --- a/scripts/qapi-commands.py > +++ b/scripts/qapi-commands.py > @@ -228,7 +228,6 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor): > self.defn = None > self._regy = None > self._visited_ret_types = None > - self.if_members = ['decl', 'defn', '_regy'] > > def visit_begin(self, schema): > self.decl = '' > @@ -241,18 +240,21 @@ class QAPISchemaGenCommandVisitor(QAPISchemaVisitor): > self._regy = None > self._visited_ret_types = None > > - @ifcond_decorator > def visit_command(self, name, info, arg_type, ret_type, > gen, success_response, boxed, ifcond): > if not gen: > return > - self.decl += gen_command_decl(name, arg_type, boxed, ret_type) > + decl = gen_command_decl(name, arg_type, boxed, ret_type) > + defn = '' > if ret_type and ret_type not in self._visited_ret_types: > self._visited_ret_types.add(ret_type) > - self.defn += gen_marshal_output(ret_type) > - self.decl += gen_marshal_decl(name) > - self.defn += gen_marshal(name, arg_type, boxed, ret_type) > - self._regy += gen_register_command(name, success_response) > + defn += gen_marshal_output(ret_type) > + decl += gen_marshal_decl(name) > + defn += gen_marshal(name, arg_type, boxed, ret_type) > + self.decl += gen_ifcond(ifcond, decl) > + self.defn += gen_ifcond(ifcond, defn) > + self._regy += gen_ifcond(ifcond, > + gen_register_command(name, > success_response))
This is one of the most simple case, and it is already very intrusive. If you have more branches / early return it becomes awful. Do it for all the places where we have @ifcond_decorator and you will probably prefer the concise decorated version. For me this case is one of the reason why decorators exist: to wrap enter/exit of a function. > > > (input_file, output_dir, do_c, do_h, prefix, opts) = parse_command_line() > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 52099332f1..3de5b8b8f0 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -1918,29 +1918,10 @@ def gen_endif(ifcond): > return ret > > > -# Wrap a method to add #if / #endif to generated code, only if some > -# code was generated. > -# self must have 'if_members' listing the attributes to wrap. > -# The last argument of the wrapped function must be the 'ifcond'. > -def ifcond_decorator(func): > - > - def func_wrapper(self, *args, **kwargs): > - ifcond = args[-1] > - save = {} > - for mem in self.if_members: > - save[mem] = getattr(self, mem) > - func(self, *args, **kwargs) > - for mem, val in save.items(): > - newval = getattr(self, mem) > - if newval != val: > - assert newval.startswith(val) > - newval = newval[len(val):] > - val += gen_if(ifcond) > - val += newval > - val += gen_endif(ifcond) > - setattr(self, mem, val) > - > - return func_wrapper > +def gen_ifcond(ifcond, ctext): > + if ifcond: > + return gen_if(ifcond) + ctext + gen_endif(ifcond) > + return ctext > > > def gen_enum_lookup(name, values, prefix=None): >