[v2,02/19] qapi-visit: Expose visit_type_FOO_members()

Commit Message

Eric Blake Feb. 25, 2016, 11:38 p.m. UTC

Dan Berrange reported a case where he needs to work with a
QCryptoBlockOptions union type using the OptsVisitor, but only
visit one of the branches of that type (the discriminator is not
visited directly, but learned externally).  When things were
boxed, it was easy: just visit the variant directly, which took
care of both allocating the variant and visiting its members, then
store that pointer in the union type.  But now that things are
unboxed, we need a way to visit the members without allocation,
done by exposing visit_type_FOO_members() to the user.

Before the patch, we had quite a bit of code associated with
object_members_seen to make sure that a declaration of the helper
was in scope before any use of the function.  But now that the
helper is public and declared in the header, the .c file no
longer needs to worry about topological sorting (the helper is
always in scope), which leads to some nice cleanups.

Signed-off-by: Eric Blake <[e-mail]>

---
v2: Rebase atop s/fields/members/rename, don't provide useless
declaration on alternates
---
 scripts/qapi-visit.py | 33 +++++++--------------------------
 1 file changed, 7 insertions(+), 26 deletions(-)

Comments

Markus Armbruster March 2, 2016, 5:24 p.m. UTC | #1

Eric Blake <[e-mail]> writes:

> Dan Berrange reported a case where he needs to work with a
> QCryptoBlockOptions union type using the OptsVisitor, but only
> visit one of the branches of that type (the discriminator is not
> visited directly, but learned externally).  When things were
> boxed, it was easy: just visit the variant directly, which took
> care of both allocating the variant and visiting its members, then
> store that pointer in the union type.  But now that things are
> unboxed, we need a way to visit the members without allocation,
> done by exposing visit_type_FOO_members() to the user.
>
> Before the patch, we had quite a bit of code associated with
> object_members_seen to make sure that a declaration of the helper
> was in scope before any use of the function.  But now that the
> helper is public and declared in the header, the .c file no
> longer needs to worry about topological sorting (the helper is
> always in scope), which leads to some nice cleanups.
>
> Signed-off-by: Eric Blake <[e-mail]>
>
> ---
> v2: Rebase atop s/fields/members/rename, don't provide useless
> declaration on alternates

I like this much better than v1.  Thanks!

Patch

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 1e52f76..a712e9a 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -15,10 +15,6 @@ 
 from qapi import *
 import re

-# visit_type_FOO_members() is always emitted; track if a forward declaration
-# or implementation has already been output.
-object_members_seen = set()
-

 def gen_visit_decl(name, scalar=False):
     c_type = c_name(name) + ' *'
@@ -30,37 +26,23 @@  void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_type)sobj, Error **
                  c_name=c_name(name), c_type=c_type)


-def gen_visit_members_decl(typ):
-    if typ.name in object_members_seen:
-        return ''
-    object_members_seen.add(typ.name)
+def gen_visit_members_decl(name):
     return mcgen('''

-static void visit_type_%(c_type)s_members(Visitor *v, %(c_type)s *obj, Error **errp);
+void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp);
 ''',
-                 c_type=typ.c_name())
+                 c_name=c_name(name))


 def gen_visit_object_members(name, base, members, variants):
-    ret = ''
+    ret = mcgen('''

-    if base:
-        ret += gen_visit_members_decl(base)
-    if variants:
-        for var in variants.variants:
-            # Ugly special case for simple union TODO get rid of it
-            if not var.simple_union_type():
-                ret += gen_visit_members_decl(var.type)
-
-    object_members_seen.add(name)
-    ret += mcgen('''
-
-static void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
+void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
 {
     Error *err = NULL;

 ''',
-                 c_name=c_name(name))
+                c_name=c_name(name))

     if base:
         ret += mcgen('''
@@ -173,8 +155,6 @@  def gen_visit_alternate(name, variants):
     for var in variants.variants:
         if var.type.alternate_qtype() == 'QTYPE_QINT':
             promote_int = 'false'
-        if isinstance(var.type, QAPISchemaObjectType):
-            ret += gen_visit_members_decl(var.type)

     ret += mcgen('''

@@ -316,6 +296,7 @@  class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
             self.defn += defn

     def visit_object_type(self, name, info, base, members, variants):
+        self.decl += gen_visit_members_decl(name)
         self.decl += gen_visit_decl(name)
         self.defn += gen_visit_object(name, base, members, variants)