diff mbox series

[v3,10/49] qapi-introspect: add preprocessor conditions to generated QLit

Message ID 20180321115211.17937-11-marcandre.lureau@redhat.com
State New
Headers show
Series qapi: add #if pre-processor conditions to generated code | expand

Commit Message

Marc-André Lureau March 21, 2018, 11:51 a.m. UTC
The generator will take (obj, condition) tuples to wrap generated QLit
objects for 'obj' with #if/#endif conditions.

This commit adds 'ifcond' condition to top-level QLit objects.

See generated tests/test-qmp-introspect.c. Example diff after this patch:

    --- before	2018-01-08 11:55:24.757083654 +0100
    +++ tests/test-qmp-introspect.c	2018-01-08 13:08:44.477641629 +0100
    @@ -51,6 +51,8 @@
             { "name", QLIT_QSTR("EVENT_F"), },
             {}
         })),
    +#if defined(TEST_IF_CMD)
    +#if defined(TEST_IF_STRUCT)
         QLIT_QDICT(((QLitDictEntry[]) {
             { "arg-type", QLIT_QSTR("5"), },
             { "meta-type", QLIT_QSTR("command"), },
    @@ -58,12 +60,16 @@
             { "ret-type", QLIT_QSTR("0"), },
             {}
         })),
    +#endif /* defined(TEST_IF_STRUCT) */
    +#endif /* defined(TEST_IF_CMD) */

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/introspect.py | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

Comments

Markus Armbruster June 21, 2018, 1:05 p.m. UTC | #1
Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> The generator will take (obj, condition) tuples to wrap generated QLit
> objects for 'obj' with #if/#endif conditions.
>
> This commit adds 'ifcond' condition to top-level QLit objects.
>
> See generated tests/test-qmp-introspect.c. Example diff after this patch:
>
>     --- before	2018-01-08 11:55:24.757083654 +0100
>     +++ tests/test-qmp-introspect.c	2018-01-08 13:08:44.477641629 +0100
>     @@ -51,6 +51,8 @@
>              { "name", QLIT_QSTR("EVENT_F"), },
>              {}
>          })),
>     +#if defined(TEST_IF_CMD)
>     +#if defined(TEST_IF_STRUCT)
>          QLIT_QDICT(((QLitDictEntry[]) {
>              { "arg-type", QLIT_QSTR("5"), },
>              { "meta-type", QLIT_QSTR("command"), },
>     @@ -58,12 +60,16 @@
>              { "ret-type", QLIT_QSTR("0"), },
>              {}
>          })),
>     +#endif /* defined(TEST_IF_STRUCT) */
>     +#endif /* defined(TEST_IF_CMD) */
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  scripts/qapi/introspect.py | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index 1a8eb9e33e..cb4825f134 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -18,6 +18,15 @@ def to_qlit(obj, level=0, suppress_first_indent=False):
>      def indent(level):
>          return level * 4 * ' '
>  
> +    if isinstance(obj, tuple):
> +        ifobj, ifcond = obj
> +        ret = gen_if(ifcond)
> +        ret += to_qlit(ifobj, level)
> +        endif = gen_endif(ifcond)
> +        if endif:
> +            ret += '\n' + endif
> +        return ret
> +
>      ret = ''
>      if not suppress_first_indent:
>          ret += indent(level)

@obj represents a JSON object.  It consists of dictionaries, lists,
strings, booleans and None.  Numbers aren't implemented.  Your patch
splices in tuples to represent conditionals at any level.

So far, tuples occur only as elements of the outermost list (see
._gen_qlit() below), but to_qlit() supports them anywhere.  I figure
that will be needed later.  If yes, the whole thing is a bit of a hack,
but at least it's a simple hack.  If no, I'd look for a solution that
feels less hackish.

By the way, this hunk makes my pylint die with "RecursionError: maximum
recursion depth exceeded".  If I comment out the line with the
recursion, it survives.  Latest git survives, too.  Oh well, python3
setup.py install.

> @@ -26,7 +35,7 @@ def to_qlit(obj, level=0, suppress_first_indent=False):
>      elif isinstance(obj, str):
>          ret += 'QLIT_QSTR(' + to_c_string(obj) + ')'
>      elif isinstance(obj, list):
> -        elts = [to_qlit(elt, level + 1)
> +        elts = [to_qlit(elt, level + 1).strip('\n')
>                  for elt in obj]
>          elts.append(indent(level + 1) + "{}")
>          ret += 'QLIT_QLIST(((QLitObject[]) {\n'

The .strip('\n') looks odd.  Reminds me of the asymmetry I noted in
PATCH 08: you take care to adjust vertical space around #if there, but
not around #endif.  Let's not worry about it now.

> @@ -131,12 +140,12 @@ const QLitObject %(c_name)s = %(c_string)s;
>              return '[' + self._use_type(typ.element_type) + ']'
>          return self._name(typ.name)
>  
> -    def _gen_qlit(self, name, mtype, obj):
> +    def _gen_qlit(self, name, mtype, obj, ifcond):
>          if mtype not in ('command', 'event', 'builtin', 'array'):
>              name = self._name(name)
>          obj['name'] = name
>          obj['meta-type'] = mtype
> -        self._qlits.append(obj)
> +        self._qlits.append((obj, ifcond))

This is where we produce ._qlits.  It's also the only place where we
create tuples.  Thus, all elements of ._qlits are tuples, and no tuples
occur at deeper levels.

>  
>      def _gen_member(self, member):
>          ret = {'name': member.name, 'type': self._use_type(member.type)}
> @@ -152,26 +161,27 @@ const QLitObject %(c_name)s = %(c_string)s;
>          return {'case': variant.name, 'type': self._use_type(variant.type)}
>  
>      def visit_builtin_type(self, name, info, json_type):
> -        self._gen_qlit(name, 'builtin', {'json-type': json_type})
> +        self._gen_qlit(name, 'builtin', {'json-type': json_type}, [])
>  
>      def visit_enum_type(self, name, info, ifcond, values, prefix):
> -        self._gen_qlit(name, 'enum', {'values': values})
> +        self._gen_qlit(name, 'enum', {'values': values}, ifcond)
>  
>      def visit_array_type(self, name, info, ifcond, element_type):
>          element = self._use_type(element_type)
> -        self._gen_qlit('[' + element + ']', 'array', {'element-type': element})
> +        self._gen_qlit('[' + element + ']', 'array', {'element-type': element},
> +                       ifcond)
>  
>      def visit_object_type_flat(self, name, info, ifcond, members, variants):
>          obj = {'members': [self._gen_member(m) for m in members]}
>          if variants:
>              obj.update(self._gen_variants(variants.tag_member.name,
>                                            variants.variants))
> -        self._gen_qlit(name, 'object', obj)
> +        self._gen_qlit(name, 'object', obj, ifcond)
>  
>      def visit_alternate_type(self, name, info, ifcond, variants):
>          self._gen_qlit(name, 'alternate',
>                         {'members': [{'type': self._use_type(m.type)}
> -                                    for m in variants.variants]})
> +                                    for m in variants.variants]}, ifcond)
>  
>      def visit_command(self, name, info, ifcond, arg_type, ret_type,
>                        gen, success_response, boxed, allow_oob):
> @@ -180,11 +190,12 @@ const QLitObject %(c_name)s = %(c_string)s;
>          self._gen_qlit(name, 'command',
>                         {'arg-type': self._use_type(arg_type),
>                          'ret-type': self._use_type(ret_type),
> -                        'allow-oob': allow_oob})
> +                        'allow-oob': allow_oob}, ifcond)
>  
>      def visit_event(self, name, info, ifcond, arg_type, boxed):
>          arg_type = arg_type or self._schema.the_empty_object_type
> -        self._gen_qlit(name, 'event', {'arg-type': self._use_type(arg_type)})
> +        self._gen_qlit(name, 'event', {'arg-type': self._use_type(arg_type)},
> +                       ifcond)
>  
>  
>  def gen_introspect(schema, output_dir, prefix, opt_unmask):

I still prefer the approach I explored in

    [PATCH RFC 09/14] qapi: Pass ifcond to visitors
    [PATCH RFC 13/14] qapi-introspect: Add #if conditions to introspection value

because it separates the concerns.  I'd love to complete my exploration
so we can compare apples to apples.  However, with the soft freeze
closing in, I'll probably have to take the solution that's already
complete, regardless of my preference.
diff mbox series

Patch

diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 1a8eb9e33e..cb4825f134 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -18,6 +18,15 @@  def to_qlit(obj, level=0, suppress_first_indent=False):
     def indent(level):
         return level * 4 * ' '
 
+    if isinstance(obj, tuple):
+        ifobj, ifcond = obj
+        ret = gen_if(ifcond)
+        ret += to_qlit(ifobj, level)
+        endif = gen_endif(ifcond)
+        if endif:
+            ret += '\n' + endif
+        return ret
+
     ret = ''
     if not suppress_first_indent:
         ret += indent(level)
@@ -26,7 +35,7 @@  def to_qlit(obj, level=0, suppress_first_indent=False):
     elif isinstance(obj, str):
         ret += 'QLIT_QSTR(' + to_c_string(obj) + ')'
     elif isinstance(obj, list):
-        elts = [to_qlit(elt, level + 1)
+        elts = [to_qlit(elt, level + 1).strip('\n')
                 for elt in obj]
         elts.append(indent(level + 1) + "{}")
         ret += 'QLIT_QLIST(((QLitObject[]) {\n'
@@ -131,12 +140,12 @@  const QLitObject %(c_name)s = %(c_string)s;
             return '[' + self._use_type(typ.element_type) + ']'
         return self._name(typ.name)
 
-    def _gen_qlit(self, name, mtype, obj):
+    def _gen_qlit(self, name, mtype, obj, ifcond):
         if mtype not in ('command', 'event', 'builtin', 'array'):
             name = self._name(name)
         obj['name'] = name
         obj['meta-type'] = mtype
-        self._qlits.append(obj)
+        self._qlits.append((obj, ifcond))
 
     def _gen_member(self, member):
         ret = {'name': member.name, 'type': self._use_type(member.type)}
@@ -152,26 +161,27 @@  const QLitObject %(c_name)s = %(c_string)s;
         return {'case': variant.name, 'type': self._use_type(variant.type)}
 
     def visit_builtin_type(self, name, info, json_type):
-        self._gen_qlit(name, 'builtin', {'json-type': json_type})
+        self._gen_qlit(name, 'builtin', {'json-type': json_type}, [])
 
     def visit_enum_type(self, name, info, ifcond, values, prefix):
-        self._gen_qlit(name, 'enum', {'values': values})
+        self._gen_qlit(name, 'enum', {'values': values}, ifcond)
 
     def visit_array_type(self, name, info, ifcond, element_type):
         element = self._use_type(element_type)
-        self._gen_qlit('[' + element + ']', 'array', {'element-type': element})
+        self._gen_qlit('[' + element + ']', 'array', {'element-type': element},
+                       ifcond)
 
     def visit_object_type_flat(self, name, info, ifcond, members, variants):
         obj = {'members': [self._gen_member(m) for m in members]}
         if variants:
             obj.update(self._gen_variants(variants.tag_member.name,
                                           variants.variants))
-        self._gen_qlit(name, 'object', obj)
+        self._gen_qlit(name, 'object', obj, ifcond)
 
     def visit_alternate_type(self, name, info, ifcond, variants):
         self._gen_qlit(name, 'alternate',
                        {'members': [{'type': self._use_type(m.type)}
-                                    for m in variants.variants]})
+                                    for m in variants.variants]}, ifcond)
 
     def visit_command(self, name, info, ifcond, arg_type, ret_type,
                       gen, success_response, boxed, allow_oob):
@@ -180,11 +190,12 @@  const QLitObject %(c_name)s = %(c_string)s;
         self._gen_qlit(name, 'command',
                        {'arg-type': self._use_type(arg_type),
                         'ret-type': self._use_type(ret_type),
-                        'allow-oob': allow_oob})
+                        'allow-oob': allow_oob}, ifcond)
 
     def visit_event(self, name, info, ifcond, arg_type, boxed):
         arg_type = arg_type or self._schema.the_empty_object_type
-        self._gen_qlit(name, 'event', {'arg-type': self._use_type(arg_type)})
+        self._gen_qlit(name, 'event', {'arg-type': self._use_type(arg_type)},
+                       ifcond)
 
 
 def gen_introspect(schema, output_dir, prefix, opt_unmask):