[PATCH 4/6] qapi: Optionally parse simple unions as flat

[Kevin Wolf <kwolf@redhat.com>]

This extends the Visitor interface with an option that can enable flat
representation (without the 'data' wrapper) for simple unions. This way,
a command line parser can enable a more user friendly syntax while QMP
doesn't enable the option and uses the same representation as before.

We need to disable flat representation for ChardevSpiceChannel, which
has a 'type' option that conflicts with the ChardevBackend 'type'. This
variant will get nesting even when flat parsing is enabled in the
Visitor.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/char.json               |  3 ++-
 docs/devel/qapi-code-gen.txt | 11 ++++++++++-
 include/qapi/visitor-impl.h  |  3 +++
 include/qapi/visitor.h       | 10 ++++++++++
 qapi/qapi-visit-core.c       | 10 ++++++++++
 scripts/qapi/expr.py         |  7 ++++++-
 scripts/qapi/schema.py       | 25 ++++++++++++++++++-------
 scripts/qapi/visit.py        | 20 ++++++++++++++------
 8 files changed, 73 insertions(+), 16 deletions(-)

diff --git a/qapi/char.json b/qapi/char.json
index 43486d1daa..57ec18220b 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -414,7 +414,8 @@
             'stdio': 'ChardevStdio',
             'console': 'ChardevCommon',
             'spicevmc': { 'type': 'ChardevSpiceChannel',
-                          'if': 'defined(CONFIG_SPICE)' },
+                          'if': 'defined(CONFIG_SPICE)',
+                          'allow-flat': false },
             'spiceport': { 'type': 'ChardevSpicePort',
                            'if': 'defined(CONFIG_SPICE)' },
             'vc': 'ChardevVC',
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 9722c1a204..ee34d39a20 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -295,7 +295,9 @@ Syntax:
               '*features': FEATURES }
     BRANCHES = { BRANCH, ... }
     BRANCH = STRING : TYPE-REF
-           | STRING : { 'type': TYPE-REF, '*if': COND }
+           | STRING : { 'type': TYPE-REF,
+                        '*if': COND,
+                        '*allow-flat': BOOL }
 
 Member 'union' names the union type.
 
@@ -334,6 +336,13 @@ values to data types like in this example:
    'data': { 'file': 'BlockdevOptionsFile',
              'qcow2': 'BlockdevOptionsQcow2' } }
 
+Simple unions can support both wrapped and flat representation of
+branches that have a struct type, unless it is explicitly disabled in
+the schema with 'allow-flat': false.  Branches of other types are
+always wrapped.  Which representation is used in the generated visitor
+C code can be configured per visitor.  Flat representation is
+appropriate when parsing command line options.
+
 In the Client JSON Protocol, all simple union branches have wrapped
 representation, as in these examples:
 
diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 7362c043be..f628b6eb36 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -121,6 +121,9 @@ struct Visitor
 
     /* Must be set */
     void (*free)(Visitor *v);
+
+    /* Set to true to make simple unions look like flat unions */
+    bool flat_simple_unions;
 };
 
 #endif
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index ebc19ede7f..d41be4df48 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -496,6 +496,16 @@ bool visit_is_input(Visitor *v);
  */
 bool visit_is_dealloc(Visitor *v);
 
+/*
+ * Check if simple unions should be treated as flat.
+ */
+bool visit_flat_simple_unions(Visitor *v);
+
+/*
+ * Set if simple unions should be treated as flat.
+ */
+void visit_set_flat_simple_unions(Visitor *v, bool flat);
+
 /*** Visiting built-in types ***/
 
 /*
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 7e5f40e7f0..dc6fd78b8c 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -145,6 +145,16 @@ bool visit_is_dealloc(Visitor *v)
     return v->type == VISITOR_DEALLOC;
 }
 
+bool visit_flat_simple_unions(Visitor *v)
+{
+    return v->flat_simple_unions;
+}
+
+void visit_set_flat_simple_unions(Visitor *v, bool flat)
+{
+    v->flat_simple_unions = flat;
+}
+
 bool visit_type_int(Visitor *v, const char *name, int64_t *obj, Error **errp)
 {
     assert(obj);
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 2fcaaa2497..a39092e4a9 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -248,7 +248,12 @@ def check_union(expr, info):
     for (key, value) in members.items():
         source = "'data' member '%s'" % key
         check_name_str(key, info, source)
-        check_keys(value, info, source, ['type'], ['if'])
+        check_keys(value, info, source, ['type'], ['if', 'allow-flat'])
+        if 'allow-flat' in value:
+            if discriminator is not None:
+                raise QAPISemError(info, "'allow-flat' requires simple union")
+            if not isinstance(value['allow-flat'], bool):
+                raise QAPISemError(info, "'allow-flat' must be a boolean")
         check_if(value, info, source)
         check_type(value['type'], info, source, allow_array=not base)
 
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index 17525b4216..981d0d659f 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -721,9 +721,9 @@ class QAPISchemaVariant(QAPISchemaObjectTypeMember):
         self.wrapped = bool(wrapper_type)
         self.wrapper_type = wrapper_type
 
-        # For now, unions are either flat or wrapped, never both
+        # Unions that are both flat and wrapped can look like either one,
+        # depending on Visitor.flat_simple_unions
         assert self.flat or self.wrapped
-        assert not (self.flat and self.wrapped)
 
     def check(self, schema):
         super().check(schema)
@@ -1038,7 +1038,7 @@ class QAPISchema:
     def _make_variant(self, case, typ, ifcond, info):
         return QAPISchemaVariant(case, info, typ, ifcond)
 
-    def _make_simple_variant(self, union_name, case, typ, ifcond, info):
+    def _make_simple_variant(self, union_name, case, typ, ifcond, flat, info):
         if isinstance(typ, list):
             assert len(typ) == 1
             typ = self._make_array_type(typ[0], info)
@@ -1049,7 +1049,14 @@ class QAPISchema:
         wrapper_member = self._make_member('data', typ, None, None, info)
         wrapper_type = QAPISchemaObjectType(wrapper_name, info, None, ifcond,
                                             None, None, [wrapper_member], None)
-        return QAPISchemaVariant(case, info, typ, ifcond, flat=False,
+
+        # Default to allowing flat representation for object types.
+        # Other types require a wrapper, so disable flat for them by default.
+        if flat is None:
+            variant_type = self.resolve_type(typ, info, 'variant type')
+            flat = isinstance(variant_type, QAPISchemaObjectType)
+
+        return QAPISchemaVariant(case, info, typ, ifcond, flat=flat,
                                  wrapper_type=wrapper_type)
 
     def _def_union_type(self, expr, info, doc):
@@ -1070,9 +1077,13 @@ class QAPISchema:
                         for (key, value) in data.items()]
             members = []
         else:
-            variants = [self._make_simple_variant(name, key, value['type'],
-                                                  value.get('if'), info)
-                        for (key, value) in data.items()]
+            variants = [
+                self._make_simple_variant(name, key, value['type'],
+                                          value.get('if'),
+                                          value.get('allow-flat'),
+                                          info)
+                for (key, value) in data.items()
+            ]
             enum = [{'name': v.name, 'if': v.ifcond} for v in variants]
             typ = self._make_implicit_enum_type(name, info, ifcond, enum)
             tag_member = QAPISchemaObjectTypeMember('type', info, typ, False)
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index f72567cbcc..9d05d6bd08 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -127,23 +127,31 @@ bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
                              case=case_str,
                              c_type=var.type.c_name(), c_name=c_name(var.name))
             elif var.wrapped:
+                if var.flat:
+                    cond = "!visit_flat_simple_unions(v)"
+                else:
+                    cond = "true"
                 ret += mcgen('''
     case %(case)s:
     {
         bool ok;
 
-        if (!visit_start_struct(v, "data", NULL, 0, errp)) {
-            return false;
+        if (%(cond)s) {
+            if (!visit_start_struct(v, "data", NULL, 0, errp)) {
+                return false;
+            }
         }
         ok = visit_type_%(c_type)s_members(v, &obj->u.%(c_name)s, errp);
-        if (ok) {
-            ok = visit_check_struct(v, errp);
+        if (%(cond)s) {
+            if (ok) {
+                ok = visit_check_struct(v, errp);
+            }
+            visit_end_struct(v, NULL);
         }
-        visit_end_struct(v, NULL);
         return ok;
     }
 ''',
-                             case=case_str,
+                             case=case_str, cond=cond,
                              c_type=var.type.c_name(), c_name=c_name(var.name))
 
             else:
-- 
2.28.0