[Qemu-devel] [PATCH v7 14/15] qapi: Allow anonymous branch types in flat union

[Eric Blake <eblake@redhat.com>]

Recent commits added support for an anonymous type as the base
of a flat union; with a bit more work, we can also allow an
anonymous struct as a branch of a flat union.  This probably
most useful when a branch adds no additional members beyond the
common elements of the base (that is, the branch struct is '{}'),
but can be used for any struct in the same way we allow for an
anonymous struct for a command.

The generator has to do a bit of special-casing for the fact that
we do not emit a '_empty' struct nor a 'visit_type__empty_members()'
corresponding to the special ':empty' type; but when the branch
is truly empty, there's nothing to do.

The testsuite gets an update to use the new feature, and to ensure
that we can still detect invalid collisions of QMP names.

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

---
v7: new patch
---
 scripts/qapi.py                          | 21 +++++++++++++++------
 scripts/qapi-types.py                    | 10 +++++++---
 scripts/qapi-visit.py                    | 14 ++++++++++----
 tests/Makefile                           |  1 +
 tests/qapi-schema/flat-union-inline.err  |  2 +-
 tests/qapi-schema/flat-union-inline.json |  5 ++---
 tests/qapi-schema/qapi-schema-test.json  |  5 +++--
 tests/qapi-schema/qapi-schema-test.out   |  8 ++++++--
 tests/qapi-schema/union-inline.err       |  1 +
 tests/qapi-schema/union-inline.exit      |  1 +
 tests/qapi-schema/union-inline.json      |  4 ++++
 tests/qapi-schema/union-inline.out       |  0
 12 files changed, 51 insertions(+), 21 deletions(-)
 create mode 100644 tests/qapi-schema/union-inline.err
 create mode 100644 tests/qapi-schema/union-inline.exit
 create mode 100644 tests/qapi-schema/union-inline.json
 create mode 100644 tests/qapi-schema/union-inline.out

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 7d568d9..4c531e7 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -607,9 +607,11 @@ def check_union(expr, expr_info):
     for (key, value) in members.items():
         check_name(expr_info, "Member of union '%s'" % name, key)

-        # Each value must name a known type
+        # Each value must name a type; although the type may be anonymous
+        # for a flat union.
         check_type(expr_info, "Member '%s' of union '%s'" % (key, name),
-                   value, allow_array=not base, allow_metas=allow_metas)
+                   value, allow_array=not base, allow_dict=base is not None,
+                   allow_optional=True, allow_metas=allow_metas)

         # If the discriminator names an enum type, then all members
         # of 'data' must also be members of the enum type.
@@ -1061,6 +1063,9 @@ class QAPISchemaMember(object):
                 return '(parameter of %s)' % owner[:-4]
             elif owner.endswith('-base'):
                 return '(base of %s)' % owner[:-5]
+            elif owner.endswith('-branch'):
+                return ('(member of %s branch %s)'
+                        % tuple(owner[:-7].split(':')))
             else:
                 assert owner.endswith('-wrapper')
                 # Unreachable and not implemented
@@ -1335,7 +1340,11 @@ class QAPISchema(object):
                                               self._make_members(data, info),
                                               None))

-    def _make_variant(self, case, typ):
+    def _make_variant(self, case, typ, info, owner):
+        if isinstance(typ, dict):
+            typ = self._make_implicit_object_type(
+                "%s:%s" % (owner, case), info, 'branch',
+                self._make_members(typ, info)) or 'q_empty'
         return QAPISchemaObjectTypeVariant(case, typ)

     def _make_simple_variant(self, case, typ, info):
@@ -1356,7 +1365,7 @@ class QAPISchema(object):
             base = (self._make_implicit_object_type(
                     name, info, 'base', self._make_members(base, info)))
         if tag_name:
-            variants = [self._make_variant(key, value)
+            variants = [self._make_variant(key, value, info, name)
                         for (key, value) in data.iteritems()]
             members = []
         else:
@@ -1375,7 +1384,7 @@ class QAPISchema(object):
     def _def_alternate_type(self, expr, info):
         name = expr['alternate']
         data = expr['data']
-        variants = [self._make_variant(key, value)
+        variants = [self._make_variant(key, value, info, name)
                     for (key, value) in data.iteritems()]
         tag_member = QAPISchemaObjectTypeMember('type', 'QType', False)
         self._def_entity(
@@ -1485,7 +1494,7 @@ def c_enum_const(type_name, const_name, prefix=None):
         type_name = prefix
     return camel_to_upper(type_name) + '_' + c_name(const_name, False).upper()

-c_name_trans = string.maketrans('.-', '__')
+c_name_trans = string.maketrans('.-:', '___')


 # Map @name to a valid C identifier.
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index 5a9e2da..f1edab2 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -61,7 +61,8 @@ def gen_object(name, base, members, variants):
     ret = ''
     if variants:
         for v in variants.variants:
-            if isinstance(v.type, QAPISchemaObjectType):
+            if (isinstance(v.type, QAPISchemaObjectType)
+                    and not (v.type.is_implicit() and v.type.is_empty())):
                 ret += gen_object(v.type.name, v.type.base,
                                   v.type.local_members, v.type.variants)

@@ -123,11 +124,14 @@ def gen_variants(variants):
                 c_name=c_name(variants.tag_member.name))

     for var in variants.variants:
+        typ = var.type.c_unboxed_type()
+        if (isinstance(var.type, QAPISchemaObjectType) and
+                var.type.is_empty() and var.type.is_implicit()):
+            typ = 'char'
         ret += mcgen('''
         %(c_type)s %(c_name)s;
 ''',
-                     c_type=var.type.c_unboxed_type(),
-                     c_name=c_name(var.name))
+                     c_type=typ, c_name=c_name(var.name))

     ret += mcgen('''
     } u;
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 07ae6d1..46f8b39 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -79,13 +79,19 @@ void visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
         for var in variants.variants:
             ret += mcgen('''
     case %(case)s:
-        visit_type_%(c_type)s_members(v, &obj->u.%(c_name)s, &err);
-        break;
 ''',
                          case=c_enum_const(variants.tag_member.type.name,
                                            var.name,
-                                           variants.tag_member.type.prefix),
-                         c_type=var.type.c_name(), c_name=c_name(var.name))
+                                           variants.tag_member.type.prefix))
+            if (not isinstance(var.type, QAPISchemaObjectType) or
+                    not var.type.is_empty()):
+                ret += mcgen('''
+        visit_type_%(c_type)s_members(v, &obj->u.%(c_name)s, &err);
+''',
+                             c_type=var.type.c_name(), c_name=c_name(var.name))
+            ret += mcgen('''
+        break;
+''')

         ret += mcgen('''
     default:
diff --git a/tests/Makefile b/tests/Makefile
index 5cd6177..d7d9597 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -379,6 +379,7 @@ qapi-schema += union-base-no-discriminator.json
 qapi-schema += union-branch-case.json
 qapi-schema += union-clash-branches.json
 qapi-schema += union-empty.json
+qapi-schema += union-inline.json
 qapi-schema += union-invalid-base.json
 qapi-schema += union-optional-branch.json
 qapi-schema += union-unknown.json
diff --git a/tests/qapi-schema/flat-union-inline.err b/tests/qapi-schema/flat-union-inline.err
index 2333358..efcafec 100644
--- a/tests/qapi-schema/flat-union-inline.err
+++ b/tests/qapi-schema/flat-union-inline.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-inline.json:7: Member 'value1' of union 'TestUnion' should be a type name
+tests/qapi-schema/flat-union-inline.json:6: 'kind' (member of TestUnion branch value2) collides with 'kind' (member of Base)
diff --git a/tests/qapi-schema/flat-union-inline.json b/tests/qapi-schema/flat-union-inline.json
index 62c7cda..a049ec8 100644
--- a/tests/qapi-schema/flat-union-inline.json
+++ b/tests/qapi-schema/flat-union-inline.json
@@ -1,5 +1,4 @@
-# we require branches to be a struct name
-# TODO: should we allow anonymous inline branch types?
+# we allow anonymous union branches, but they must not have clashing names
 { 'enum': 'TestEnum',
   'data': [ 'value1', 'value2' ] }
 { 'struct': 'Base',
@@ -8,4 +7,4 @@
   'base': 'Base',
   'discriminator': 'enum1',
   'data': { 'value1': { 'string': 'str' },
-            'value2': { 'integer': 'int' } } }
+            'value2': { 'kind': 'int' } } }
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index df91f3d..5128b49 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -23,7 +23,7 @@
 # for testing override of default naming heuristic
 { 'enum': 'QEnumTwo',
   'prefix': 'QENUM_TWO',
-  'data': [ 'value1', 'value2' ] }
+  'data': [ 'value1', 'value2', 'value3' ] }

 # for testing nested structs
 { 'struct': 'UserDefOne',
@@ -81,7 +81,8 @@
   'base': { '*integer': 'int', 'string': 'str', 'enum1': 'QEnumTwo' },
   'discriminator': 'enum1',
   'data': { 'value1' : 'UserDefC', # intentional forward reference
-            'value2' : 'UserDefB' } }
+            'value2' : { },
+            'value3' : { 'boolean': 'bool', '*number': 'number' } } }

 { 'struct': 'WrapAlternate',
   'data': { 'alt': 'UserDefAlternate' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 8a00c6b..7fac2da 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -50,7 +50,7 @@ object NestedEnumsOne
     member enum2: EnumOne optional=True
     member enum3: EnumOne optional=False
     member enum4: EnumOne optional=True
-enum QEnumTwo ['value1', 'value2']
+enum QEnumTwo ['value1', 'value2', 'value3']
     prefix QENUM_TWO
 enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool']
     prefix QTYPE
@@ -82,7 +82,8 @@ object UserDefFlatUnion2
     base q_obj_UserDefFlatUnion2-base
     tag enum1
     case value1: UserDefC
-    case value2: UserDefB
+    case value2: q_empty
+    case value3: q_obj_UserDefFlatUnion2:value3-branch
 object UserDefNativeListUnion
     member type: UserDefNativeListUnionKind optional=False
     tag type
@@ -179,6 +180,9 @@ object q_obj_UserDefFlatUnion2-base
     member integer: int optional=True
     member string: str optional=False
     member enum1: QEnumTwo optional=False
+object q_obj_UserDefFlatUnion2:value3-branch
+    member boolean: bool optional=False
+    member number: number optional=True
 object q_obj___org.qemu_x-command-arg
     member a: __org.qemu_x-EnumList optional=False
     member b: __org.qemu_x-StructList optional=False
diff --git a/tests/qapi-schema/union-inline.err b/tests/qapi-schema/union-inline.err
new file mode 100644
index 0000000..6c5389a
--- /dev/null
+++ b/tests/qapi-schema/union-inline.err
@@ -0,0 +1 @@
+tests/qapi-schema/union-inline.json:2: Member 'value1' of union 'TestUnion' should be a type name
diff --git a/tests/qapi-schema/union-inline.exit b/tests/qapi-schema/union-inline.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/union-inline.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/union-inline.json b/tests/qapi-schema/union-inline.json
new file mode 100644
index 0000000..b8c5df6
--- /dev/null
+++ b/tests/qapi-schema/union-inline.json
@@ -0,0 +1,4 @@
+# simple unions cannot have anonymous branches (only flat unions can)
+{ 'union': 'TestUnion',
+  'data': { 'value1': { 'string': 'str' },
+            'value2': { 'kind': 'int' } } }
diff --git a/tests/qapi-schema/union-inline.out b/tests/qapi-schema/union-inline.out
new file mode 100644
index 0000000..e69de29
-- 
2.5.5