[Qemu-devel] [PATCH v8 10/18] qapi: Move union tag quirks into subclass

[Eric Blake <eblake@redhat.com>]

Right now, simple unions have a quirk of using 'kind' in the C
struct to match the QMP wire name 'type'.  This has resulted in
messy clients each doing special cases.  While we plan to
eventually rename things to match, it is better in the meantime
to consolidate the quirks into a special subclass, by adding a
new member.c_name() function.  This will also make it easier
for reworking how alternate types are laid out in a future
patch.  Use the new c_name() function where possible.

No change to generated code.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v7: new patch, but borrows idea of subclass from v6 10/12, as
well as c_name() from 7/12
---
 scripts/qapi-commands.py |  8 ++++----
 scripts/qapi-types.py    | 12 +++++-------
 scripts/qapi-visit.py    | 17 +++++------------
 scripts/qapi.py          | 26 +++++++++++++++++++-------
 4 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 43a893b..53a79ab 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -32,8 +32,8 @@ def gen_call(name, arg_type, ret_type):
     if arg_type:
         for memb in arg_type.members:
             if memb.optional:
-                argstr += 'has_%s, ' % c_name(memb.name)
-            argstr += '%s, ' % c_name(memb.name)
+                argstr += 'has_' + memb.c_name() + ', '
+            argstr += memb.c_name() + ', '

     lhs = ''
     if ret_type:
@@ -77,11 +77,11 @@ def gen_marshal_vars(arg_type, ret_type):
                 ret += mcgen('''
     bool has_%(c_name)s = false;
 ''',
-                             c_name=c_name(memb.name))
+                             c_name=memb.c_name())
             ret += mcgen('''
     %(c_type)s %(c_name)s = %(c_null)s;
 ''',
-                         c_name=c_name(memb.name),
+                         c_name=memb.c_name(),
                          c_type=memb.type.c_type(),
                          c_null=memb.type.c_null())
         ret += '\n'
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 4fe618e..e37d529 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -136,9 +136,10 @@ struct %(c_name)s {
 ''')
     else:
         ret += mcgen('''
-    %(c_type)s kind;
+    %(c_type)s %(c_name)s;
 ''',
-                     c_type=c_name(variants.tag_member.type.name))
+                     c_type=variants.tag_member.type.c_name(),
+                     c_name=variants.tag_member.c_name())

     # FIXME: What purpose does data serve, besides preventing a union that
     # has a branch named 'data'? We use it in qapi-visit.py to decide
@@ -152,10 +153,7 @@ struct %(c_name)s {
     union { /* union tag is @%(c_name)s */
         void *data;
 ''',
-                 # TODO ugly special case for simple union
-                 # Use same tag name in C as on the wire to get rid of
-                 # it, then: c_name=c_name(variants.tag_member.name)
-                 c_name=c_name(variants.tag_name or 'kind'))
+                 c_name=variants.tag_member.c_name())

     for var in variants.variants:
         # Ugly special case for simple union TODO get rid of it
@@ -164,7 +162,7 @@ struct %(c_name)s {
         %(c_type)s %(c_name)s;
 ''',
                      c_type=typ.c_type(),
-                     c_name=c_name(var.name))
+                     c_name=var.c_name())

     ret += mcgen('''
     };
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 2a9fab8..cb251b2 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -196,7 +196,7 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
                      case=c_enum_const(variants.tag_member.type.name,
                                        var.name),
                      c_type=var.type.c_name(),
-                     c_name=c_name(var.name))
+                     c_name=var.c_name())

     ret += mcgen('''
     default:
@@ -249,10 +249,6 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
                      c_name=c_name(name))
         ret += gen_err_check(label='out_obj')

-    tag_key = variants.tag_member.name
-    if not variants.tag_name:
-        # we pointlessly use a different key for simple unions
-        tag_key = 'type'
     ret += mcgen('''
     visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
     if (err) {
@@ -264,11 +260,8 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
     switch ((*obj)->%(c_name)s) {
 ''',
                  c_type=variants.tag_member.type.c_name(),
-                 # TODO ugly special case for simple union
-                 # Use same tag name in C as on the wire to get rid of
-                 # it, then: c_name=c_name(variants.tag_member.name)
-                 c_name=c_name(variants.tag_name or 'kind'),
-                 name=tag_key)
+                 c_name=variants.tag_member.c_name(),
+                 name=variants.tag_member.name)

     for var in variants.variants:
         # TODO ugly special case for simple union
@@ -283,13 +276,13 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj, const char *name, Error
         visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "data", &err);
 ''',
                          c_type=simple_union_type.c_name(),
-                         c_name=c_name(var.name))
+                         c_name=var.c_name())
         else:
             ret += mcgen('''
         visit_type_implicit_%(c_type)s(v, &(*obj)->%(c_name)s, &err);
 ''',
                          c_type=var.type.c_name(),
-                         c_name=c_name(var.name))
+                         c_name=var.c_name())
         ret += mcgen('''
         break;
 ''')
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 5b66264..80c026b 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -975,7 +975,7 @@ class QAPISchemaObjectType(QAPISchemaType):
             members = []
         seen = {}
         for m in members:
-            assert c_name(m.name) not in seen
+            assert m.c_name() not in seen
             seen[m.name] = m
         for m in self.local_members:
             m.check(schema, members, seen)
@@ -1022,6 +1022,18 @@ class QAPISchemaObjectTypeMember(object):
         all_members.append(self)
         seen[self.name] = self

+    def c_name(self):
+        return c_name(self.name)
+
+
+# TODO Drop this class once we no longer have the 'type'/'kind' mismatch
+class QAPISchemaObjectTypeUnionTag(QAPISchemaObjectTypeMember):
+    def c_name(self):
+        assert self.name == 'type'
+        assert isinstance(self.type, QAPISchemaEnumType)
+        assert self.type.is_implicit()
+        return 'kind'
+

 class QAPISchemaObjectTypeVariants(object):
     def __init__(self, tag_name, tag_member, variants):
@@ -1248,7 +1260,7 @@ class QAPISchema(object):
     def _make_implicit_tag(self, type_name, variants):
         typ = self._make_implicit_enum_type(type_name,
                                             [v.name for v in variants])
-        return QAPISchemaObjectTypeMember('type', typ, False)
+        return QAPISchemaObjectTypeUnionTag('type', typ, False)

     def _def_union_type(self, expr, info):
         name = expr['union']
@@ -1555,8 +1567,8 @@ def gen_params(arg_type, extra):
         ret += sep
         sep = ', '
         if memb.optional:
-            ret += 'bool has_%s, ' % c_name(memb.name)
-        ret += '%s %s' % (memb.type.c_type(is_param=True), c_name(memb.name))
+            ret += 'bool has_' + memb.c_name() + sep
+        ret += '%s %s' % (memb.type.c_type(is_param=True), memb.c_name())
     if extra:
         ret += sep + extra
     return ret
@@ -1585,13 +1597,13 @@ def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False):
             ret += mcgen('''
     visit_optional(v, &%(prefix)shas_%(c_name)s, "%(name)s", %(errp)s);
 ''',
-                         prefix=prefix, c_name=c_name(memb.name),
+                         prefix=prefix, c_name=memb.c_name(),
                          name=memb.name, errp=errparg)
             ret += gen_err_check(skiperr=skiperr)
             ret += mcgen('''
     if (%(prefix)shas_%(c_name)s) {
 ''',
-                         prefix=prefix, c_name=c_name(memb.name))
+                         prefix=prefix, c_name=memb.c_name())
             push_indent()

         # Ugly: sometimes we need to cast away const
@@ -1604,7 +1616,7 @@ def gen_visit_fields(members, prefix='', need_cast=False, skiperr=False):
     visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s, "%(name)s", %(errp)s);
 ''',
                      c_type=memb.type.c_name(), prefix=prefix, cast=cast,
-                     c_name=c_name(memb.name), name=memb.name,
+                     c_name=memb.c_name(), name=memb.name,
                      errp=errparg)
         ret += gen_err_check(skiperr=skiperr)

-- 
2.4.3