[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 23/54] qapi-commands: add #if conditions to c
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH v2 23/54] qapi-commands: add #if conditions to commands |
Date: |
Tue, 5 Sep 2017 11:05:22 -0400 (EDT) |
Hi
----- Original Message -----
> Marc-André Lureau <address@hidden> writes:
>
> > Wrap generated code with #if/#endif using the ifcond_decorator.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> > 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):
>