[Qemu-devel] [RFC PATCH 3/2] qapi-visit: Remove redundant functions for flat union base

[Eric Blake]

The code for visiting the base class of a child struct created
visit_type_Base_fields(); the code for visiting the base class
of a flat union created visit_type_Union_fields(). If the same
type is shared between a struct and a union, the two functions
differed only by whether they visited the discriminator used by
the union. But if the base class always visits all its fields,
then we don't need to revisit the discriminator specially for
a union.  By consistently visiting the base class fields under
the name of the base class, we can eliminate some redundant
code.

The function signature of gen_visit_struct_fields() is changed
to operate on the actual type object, which in turn requires
some tweaks to the visitor to supply the type object directly;
maybe this implies that the visitor interface should be changed
instead.

Signed-off-by: Eric Blake <address@hidden>
---

Solves the concern mentioned here:
https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg05596.html

 scripts/qapi-visit.py | 61 +++++++++++++++++++++++++++------------------------
 1 file changed, 32 insertions(+), 29 deletions(-)

diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 30bd471..728abae 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -58,16 +58,15 @@ static void visit_type_implicit_%(c_type)s(Visitor *m, 
%(c_type)s **obj, Error *
                  c_type=typ.c_name())
     return ret

-def gen_visit_struct_fields(name, base, members):
-    if name in struct_fields_seen:
+def gen_visit_struct_fields(typ):
+    if typ.name in struct_fields_seen:
         return ''
-    struct_fields_seen.add(name)
+    struct_fields_seen.add(typ.name)

     ret = ''

-    if base:
-        ret += gen_visit_struct_fields(base.name, base.base,
-                                       base.local_members)
+    if typ.base:
+        ret += gen_visit_struct_fields(typ.base)

     ret += mcgen('''

@@ -76,19 +75,19 @@ static void visit_type_%(c_name)s_fields(Visitor *m, 
%(c_name)s **obj, Error **e
     Error *err = NULL;

 ''',
-                 c_name=c_name(name))
+                 c_name=c_name(typ.name))
     push_indent()

-    if base:
+    if typ.base:
         ret += mcgen('''
 visit_type_%(c_type)s_fields(m, (%(c_type)s **)obj, &err);
 if (err) {
     goto out;
 }
 ''',
-                     c_type=base.c_name())
+                     c_type=typ.base.c_name())

-    for memb in members:
+    for memb in typ.local_members:
         if memb.optional:
             ret += mcgen('''
 visit_optional(m, &(*obj)->has_%(c_name)s, "%(name)s", &err);
@@ -115,7 +114,7 @@ if (err) {
 ''')

     pop_indent()
-    if re.search('^ *goto out;', ret, re.MULTILINE):
+    if typ.base or typ.local_members:
         ret += mcgen('''

 out:
@@ -126,8 +125,8 @@ out:
 ''')
     return ret

-def gen_visit_struct(name, base, members):
-    ret = gen_visit_struct_fields(name, base, members)
+def gen_visit_struct(name, base, members, schema):
+    ret = gen_visit_struct_fields(schema.lookup_type(name))
     ret += mcgen('''

 void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, const char *name, 
Error **errp)
@@ -236,8 +235,7 @@ def gen_visit_union(name, base, variants):
     ret = ''

     if base:
-        members = [m for m in base.members if m != variants.tag_member]
-        ret += gen_visit_struct_fields(name, None, members)
+        ret += gen_visit_struct_fields(base)

     for var in variants.variants:
         if var.flat:
@@ -257,29 +255,31 @@ void visit_type_%(c_name)s(Visitor *m, %(c_name)s **obj, 
const char *name, Error
 ''',
                  c_name=c_name(name), name=name)

-    if base:
-        ret += mcgen('''
-        visit_type_%(c_name)s_fields(m, obj, &err);
-        if (err) {
-            goto out_obj;
-        }
-''',
-                     c_name=c_name(name))
-
     tag_key = variants.tag_member.name
-    ret += mcgen('''
+    if base:
+        ret += mcgen('''
+        visit_type_%(c_name)s_fields(m, (%(c_name)s **)obj, &err);
+        if (err) {
+            goto out_obj;
+        }
+''',
+                     c_name=c_name(base.name))
+    else:
+        ret += mcgen('''
         visit_type_%(c_type)s(m, &(*obj)->%(c_name)s, "%(name)s", &err);
         if (err) {
             goto out_obj;
         }
+''',
+                     c_type=variants.tag_member.type.c_name(),
+                     c_name=c_name(tag_key), name=tag_key)
+    ret += mcgen('''
         if (!visit_start_union(m, !!(*obj)->data, &err) || err) {
             goto out_obj;
         }
         switch ((*obj)->%(c_name)s) {
 ''',
-                 c_type=variants.tag_member.type.c_name(),
-                 c_name=c_name(variants.tag_member.name),
-                 name=tag_key)
+                 c_name=c_name(tag_key))

     for var in variants.variants:
         ret += mcgen('''
@@ -326,10 +326,12 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
         self.decl = None
         self.defn = None
         self.btin = None
+        self.schema = None
     def visit_begin(self, schema):
         self.decl = ''
         self.defn = ''
         self.btin = guardstart('QAPI_VISIT_BUILTIN_VISITOR_DECL')
+        self.schema = schema
     def visit_end(self):
         # to avoid header dependency hell, we always generate
         # declarations for built-in types in our header files and
@@ -340,6 +342,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
         # ...this doesn't work for cases where we link in multiple
         # objects that have the functions defined, so we use
         # do_builtins (option -b) to provide control
+        self.schema = None
     def visit_enum_type(self, name, info, values):
         self.decl += gen_visit_decl(name, scalar=True)
         self.defn += gen_visit_enum(name)
@@ -359,7 +362,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
             if variants:
                 self.defn += gen_visit_union(name, base, variants)
             else:
-                self.defn += gen_visit_struct(name, base, members)
+                self.defn += gen_visit_struct(name, base, members, self.schema)
     def visit_alternate_type(self, name, info, variants):
         self.decl += gen_visit_decl(name)
         self.defn += gen_visit_alternate(name, variants)
-- 
2.4.3