Re: [Qemu-devel] [PATCH v2 23/54] qapi-commands: add #if conditions to commands

[Markus Armbruster]

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))
 
 
 (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):