[Qemu-devel] [PATCH v5 08/28] qapi: Better error messages for bad unions

[Eric Blake <eblake@redhat.com>]

Previous commits demonstrated that the generator had several
flaws with less-than-perfect unions:
- make the use of a base without discriminator a hard error,
since the previous patch removed all remaining uses of it
- a simple union that listed the same branch twice (or two variant
names that map to the same C enumerator, including the implicit
MAX sentinel) ended up generating invalid C code
- checking 'if discriminator' prior to 'if discriminator == {}'
leads to dead code in python, and ended up processing anonymous
unions as if they were simple unions
- an anonymous union that listed two branches with the same qtype
ended up generating invalid C code
- the generator crashed on anonymous union attempts to use an
array type
- the generator was silently ignoring a base type for anonymous
unions
- the generator allowed unknown types or nested anonymous unions
as a branch in an anonymous union

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-types.py                              | 13 +--
 scripts/qapi.py                                    | 96 +++++++++++++++++-----
 tests/qapi-schema/alternate-array.err              |  1 +
 tests/qapi-schema/alternate-array.exit             |  2 +-
 tests/qapi-schema/alternate-array.json             |  1 -
 tests/qapi-schema/alternate-array.out              |  4 -
 tests/qapi-schema/alternate-base.err               |  1 +
 tests/qapi-schema/alternate-base.exit              |  2 +-
 tests/qapi-schema/alternate-base.json              |  2 +-
 tests/qapi-schema/alternate-base.out               |  4 -
 tests/qapi-schema/alternate-clash.err              |  1 +
 tests/qapi-schema/alternate-clash.exit             |  2 +-
 tests/qapi-schema/alternate-clash.json             |  2 +-
 tests/qapi-schema/alternate-clash.out              |  3 -
 tests/qapi-schema/alternate-conflict-dict.err      |  1 +
 tests/qapi-schema/alternate-conflict-dict.exit     |  2 +-
 tests/qapi-schema/alternate-conflict-dict.json     |  2 +-
 tests/qapi-schema/alternate-conflict-dict.out      |  6 --
 tests/qapi-schema/alternate-conflict-string.err    |  1 +
 tests/qapi-schema/alternate-conflict-string.exit   |  2 +-
 tests/qapi-schema/alternate-conflict-string.json   |  2 +-
 tests/qapi-schema/alternate-conflict-string.out    |  5 --
 tests/qapi-schema/alternate-nested.err             |  1 +
 tests/qapi-schema/alternate-nested.exit            |  2 +-
 tests/qapi-schema/alternate-nested.json            |  2 +-
 tests/qapi-schema/alternate-nested.out             |  5 --
 tests/qapi-schema/alternate-unknown.err            |  1 +
 tests/qapi-schema/alternate-unknown.exit           |  2 +-
 tests/qapi-schema/alternate-unknown.json           |  2 +-
 tests/qapi-schema/alternate-unknown.out            |  3 -
 tests/qapi-schema/flat-union-bad-discriminator.err |  1 +
 .../qapi-schema/flat-union-bad-discriminator.exit  |  2 +-
 tests/qapi-schema/flat-union-bad-discriminator.out | 10 ---
 tests/qapi-schema/union-bad-branch.err             |  1 +
 tests/qapi-schema/union-bad-branch.exit            |  2 +-
 tests/qapi-schema/union-bad-branch.json            |  2 +-
 tests/qapi-schema/union-bad-branch.out             |  6 --
 tests/qapi-schema/union-base-no-discriminator.err  |  1 +
 tests/qapi-schema/union-base-no-discriminator.exit |  2 +-
 tests/qapi-schema/union-base-no-discriminator.json |  3 +-
 tests/qapi-schema/union-base-no-discriminator.out  |  8 --
 tests/qapi-schema/union-max.err                    |  1 +
 tests/qapi-schema/union-max.exit                   |  2 +-
 tests/qapi-schema/union-max.json                   |  2 +-
 tests/qapi-schema/union-max.out                    |  3 -
 45 files changed, 109 insertions(+), 110 deletions(-)

diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index f6fb930..2390887 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -181,17 +181,8 @@ const int %(name)s_qtypes[QTYPE_MAX] = {
     name=name)

     for key in members:
-        qapi_type = members[key]
-        if builtin_types.has_key(qapi_type):
-            qtype = builtin_types[qapi_type]
-        elif find_struct(qapi_type):
-            qtype = "QTYPE_QDICT"
-        elif find_union(qapi_type):
-            qtype = "QTYPE_QDICT"
-        elif find_enum(qapi_type):
-            qtype = "QTYPE_QSTRING"
-        else:
-            assert False, "Invalid anonymous union member"
+        qtype = find_anonymous_member_qtype(members[key])
+        assert qtype, "Invalid anonymous union member"

         ret += mcgen('''
     [ %(qtype)s ] = %(abbrev)s_KIND_%(enum)s,
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3ce8c33..fc7b7f1 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -224,6 +224,23 @@ def find_base_fields(base):
         return None
     return base_struct_define['data']

+# Return the qtype of an anonymous union branch, or None on error.
+def find_anonymous_member_qtype(qapi_type):
+    if builtin_types.has_key(qapi_type):
+        return builtin_types[qapi_type]
+    elif find_struct(qapi_type):
+        return "QTYPE_QDICT"
+    elif find_enum(qapi_type):
+        return "QTYPE_QSTRING"
+    else:
+        union = find_union(qapi_type)
+        if union:
+            discriminator = union.get('discriminator')
+            if discriminator == {}:
+                return None
+            return "QTYPE_QDICT"
+    return None
+
 # Return the discriminator enum define if discriminator is specified as an
 # enum type, otherwise return None.
 def discriminator_find_enum_define(expr):
@@ -258,24 +275,28 @@ def check_union(expr, expr_info):
     base = expr.get('base')
     discriminator = expr.get('discriminator')
     members = expr['data']
+    values = { 'MAX': '(automatic)' }
+    types_seen = {}

-    # If the object has a member 'base', its value must name a complex type.
-    if base:
-        base_fields = find_base_fields(base)
-        if not base_fields:
-            raise QAPIExprError(expr_info,
-                                "Base '%s' is not a valid type"
-                                % base)
+    # Three types of unions, determined by discriminator.

-    # If the union object has no member 'discriminator', it's an
-    # ordinary union.
-    if not discriminator:
+    # If the value of member 'discriminator' is {}, it's an
+    # anonymous union, and must not have a base.
+    if discriminator == {}:
         enum_define = None
+        if base:
+            raise QAPIExprError(expr_info,
+                                "Anonymous union '%s' must not have a base"
+                                % name)

-    # Else if the value of member 'discriminator' is {}, it's an
-    # anonymous union.
-    elif discriminator == {}:
+    # Else if the union object has no member 'discriminator', it's an
+    # ordinary union.  For now, it must not have a base.
+    elif not discriminator:
         enum_define = None
+        if base:
+            raise QAPIExprError(expr_info,
+                                "Simple union '%s' must not have a base"
+                                % name)

     # Else, it's a flat union.
     else:
@@ -284,6 +305,12 @@ def check_union(expr, expr_info):
             raise QAPIExprError(expr_info,
                                 "Flat union '%s' must have a base field"
                                 % name)
+        base_fields = find_base_fields(base)
+        if not base_fields:
+            raise QAPIExprError(expr_info,
+                                "Base '%s' is not a valid type"
+                                % base)
+
         # The value of member 'discriminator' must name a member of the
         # base type.
         discriminator_type = base_fields.get(discriminator)
@@ -301,15 +328,42 @@ def check_union(expr, expr_info):

     # Check every branch
     for (key, value) in members.items():
-        # If this named member's value names an enum type, then all members
+        # If the discriminator names an enum type, then all members
         # of 'data' must also be members of the enum type.
-        if enum_define and not key in enum_define['enum_values']:
-            raise QAPIExprError(expr_info,
-                                "Discriminator value '%s' is not found in "
-                                "enum '%s'" %
-                                (key, enum_define["enum_name"]))
-        # Todo: add checking for values. Key is checked as above, value can be
-        # also checked here, but we need more functions to handle array case.
+        if enum_define:
+            if not key in enum_define['enum_values']:
+                raise QAPIExprError(expr_info,
+                                    "Discriminator value '%s' is not found in "
+                                    "enum '%s'" %
+                                    (key, enum_define["enum_name"]))
+
+        # Otherwise, check for conflicts in the generated enum
+        else:
+            c_key = _generate_enum_string(key)
+            if c_key in values:
+                raise QAPIExprError(expr_info,
+                                    "Union '%s' member '%s' clashes with '%s'"
+                                    % (name, key, values[c_key]))
+            values[c_key] = key
+
+        # Ensure anonymous unions have no type conflicts.
+        if discriminator == {}:
+            if isinstance(value, list):
+                raise QAPIExprError(expr_info,
+                                    "Anonymous union '%s' member '%s' must "
+                                    "not be array type" % (name, key))
+            qtype = find_anonymous_member_qtype(value)
+            if not qtype:
+                raise QAPIExprError(expr_info,
+                                    "Anonymous union '%s' member '%s' has "
+                                    "invalid type '%s'" % (name, key, value))
+            if qtype in types_seen:
+                raise QAPIExprError(expr_info,
+                                    "Anonymous union '%s' member '%s' has "
+                                    "same QObject type as member '%s'"
+                                    % (name, key, types_seen[qtype]))
+            types_seen[qtype] = key
+

 def check_enum(expr, expr_info):
     name = expr['enum']
diff --git a/tests/qapi-schema/alternate-array.err b/tests/qapi-schema/alternate-array.err
index e69de29..2638b85 100644
--- a/tests/qapi-schema/alternate-array.err
+++ b/tests/qapi-schema/alternate-array.err
@@ -0,0 +1 @@
+tests/qapi-schema/alternate-array.json:4: Anonymous union 'MyUnion' member 'two' must not be array type
diff --git a/tests/qapi-schema/alternate-array.exit b/tests/qapi-schema/alternate-array.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/alternate-array.exit
+++ b/tests/qapi-schema/alternate-array.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/alternate-array.json b/tests/qapi-schema/alternate-array.json
index b399500..e330d57 100644
--- a/tests/qapi-schema/alternate-array.json
+++ b/tests/qapi-schema/alternate-array.json
@@ -1,5 +1,4 @@
 # FIXME: we do not support array branches of anonymous unions yet
-# FIXME: this should fail as long as we lack support
 { 'type': 'One',
   'data': { 'name': 'str' } }
 { 'union': 'MyUnion',
diff --git a/tests/qapi-schema/alternate-array.out b/tests/qapi-schema/alternate-array.out
index 90dc22c..e69de29 100644
--- a/tests/qapi-schema/alternate-array.out
+++ b/tests/qapi-schema/alternate-array.out
@@ -1,4 +0,0 @@
-[OrderedDict([('type', 'One'), ('data', OrderedDict([('name', 'str')]))]),
- OrderedDict([('union', 'MyUnion'), ('discriminator', OrderedDict()), ('data', OrderedDict([('one', 'One'), ('two', ['int'])]))])]
-[{'enum_name': 'MyUnionKind', 'enum_values': None}]
-[OrderedDict([('type', 'One'), ('data', OrderedDict([('name', 'str')]))])]
diff --git a/tests/qapi-schema/alternate-base.err b/tests/qapi-schema/alternate-base.err
index e69de29..a2486b8 100644
--- a/tests/qapi-schema/alternate-base.err
+++ b/tests/qapi-schema/alternate-base.err
@@ -0,0 +1 @@
+tests/qapi-schema/alternate-base.json:4: Anonymous union 'MyUnion' must not have a base
diff --git a/tests/qapi-schema/alternate-base.exit b/tests/qapi-schema/alternate-base.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/alternate-base.exit
+++ b/tests/qapi-schema/alternate-base.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/alternate-base.json b/tests/qapi-schema/alternate-base.json
index 2d36db1..dad7f02 100644
--- a/tests/qapi-schema/alternate-base.json
+++ b/tests/qapi-schema/alternate-base.json
@@ -1,4 +1,4 @@
-# FIXME: we should reject anonymous union with base type
+# we reject anonymous union with base type
 { 'type': 'Base',
   'data': { 'string': 'str' } }
 { 'union': 'MyUnion',
diff --git a/tests/qapi-schema/alternate-base.out b/tests/qapi-schema/alternate-base.out
index 7fb31f5..e69de29 100644
--- a/tests/qapi-schema/alternate-base.out
+++ b/tests/qapi-schema/alternate-base.out
@@ -1,4 +0,0 @@
-[OrderedDict([('type', 'Base'), ('data', OrderedDict([('string', 'str')]))]),
- OrderedDict([('union', 'MyUnion'), ('base', 'Base'), ('discriminator', OrderedDict()), ('data', OrderedDict([('number', 'int')]))])]
-[{'enum_name': 'MyUnionKind', 'enum_values': None}]
-[OrderedDict([('type', 'Base'), ('data', OrderedDict([('string', 'str')]))])]
diff --git a/tests/qapi-schema/alternate-clash.err b/tests/qapi-schema/alternate-clash.err
index e69de29..1130c12 100644
--- a/tests/qapi-schema/alternate-clash.err
+++ b/tests/qapi-schema/alternate-clash.err
@@ -0,0 +1 @@
+tests/qapi-schema/alternate-clash.json:2: Union 'Union1' member 'ONE' clashes with 'one'
diff --git a/tests/qapi-schema/alternate-clash.exit b/tests/qapi-schema/alternate-clash.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/alternate-clash.exit
+++ b/tests/qapi-schema/alternate-clash.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/alternate-clash.json b/tests/qapi-schema/alternate-clash.json
index 7e2ef23..fa2d27e 100644
--- a/tests/qapi-schema/alternate-clash.json
+++ b/tests/qapi-schema/alternate-clash.json
@@ -1,4 +1,4 @@
-# FIXME: we should detect C enum collisions in an anonymous union
+# we detect C enum collisions in an anonymous union
 { 'union': 'Union1',
   'discriminator': {},
   'data': { 'one': 'str', 'ONE': 'int' } }
diff --git a/tests/qapi-schema/alternate-clash.out b/tests/qapi-schema/alternate-clash.out
index c6687fa..e69de29 100644
--- a/tests/qapi-schema/alternate-clash.out
+++ b/tests/qapi-schema/alternate-clash.out
@@ -1,3 +0,0 @@
-[OrderedDict([('union', 'Union1'), ('discriminator', OrderedDict()), ('data', OrderedDict([('one', 'str'), ('ONE', 'int')]))])]
-[{'enum_name': 'Union1Kind', 'enum_values': None}]
-[]
diff --git a/tests/qapi-schema/alternate-conflict-dict.err b/tests/qapi-schema/alternate-conflict-dict.err
index e69de29..f256d51 100644
--- a/tests/qapi-schema/alternate-conflict-dict.err
+++ b/tests/qapi-schema/alternate-conflict-dict.err
@@ -0,0 +1 @@
+tests/qapi-schema/alternate-conflict-dict.json:6: Anonymous union 'MyUnion' member 'two' has same QObject type as member 'one'
diff --git a/tests/qapi-schema/alternate-conflict-dict.exit b/tests/qapi-schema/alternate-conflict-dict.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/alternate-conflict-dict.exit
+++ b/tests/qapi-schema/alternate-conflict-dict.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/alternate-conflict-dict.json b/tests/qapi-schema/alternate-conflict-dict.json
index 70fe089..177c163 100644
--- a/tests/qapi-schema/alternate-conflict-dict.json
+++ b/tests/qapi-schema/alternate-conflict-dict.json
@@ -1,4 +1,4 @@
-# FIXME: we should reject anonymous unions with multiple object branches
+# we reject anonymous unions with multiple object branches
 { 'type': 'One',
   'data': { 'name': 'str' } }
 { 'type': 'Two',
diff --git a/tests/qapi-schema/alternate-conflict-dict.out b/tests/qapi-schema/alternate-conflict-dict.out
index b9ac945..e69de29 100644
--- a/tests/qapi-schema/alternate-conflict-dict.out
+++ b/tests/qapi-schema/alternate-conflict-dict.out
@@ -1,6 +0,0 @@
-[OrderedDict([('type', 'One'), ('data', OrderedDict([('name', 'str')]))]),
- OrderedDict([('type', 'Two'), ('data', OrderedDict([('value', 'int')]))]),
- OrderedDict([('union', 'MyUnion'), ('discriminator', OrderedDict()), ('data', OrderedDict([('one', 'One'), ('two', 'Two')]))])]
-[{'enum_name': 'MyUnionKind', 'enum_values': None}]
-[OrderedDict([('type', 'One'), ('data', OrderedDict([('name', 'str')]))]),
- OrderedDict([('type', 'Two'), ('data', OrderedDict([('value', 'int')]))])]
diff --git a/tests/qapi-schema/alternate-conflict-string.err b/tests/qapi-schema/alternate-conflict-string.err
index e69de29..a3b7b6d 100644
--- a/tests/qapi-schema/alternate-conflict-string.err
+++ b/tests/qapi-schema/alternate-conflict-string.err
@@ -0,0 +1 @@
+tests/qapi-schema/alternate-conflict-string.json:4: Anonymous union 'MyUnion' member 'two' has same QObject type as member 'one'
diff --git a/tests/qapi-schema/alternate-conflict-string.exit b/tests/qapi-schema/alternate-conflict-string.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/alternate-conflict-string.exit
+++ b/tests/qapi-schema/alternate-conflict-string.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/alternate-conflict-string.json b/tests/qapi-schema/alternate-conflict-string.json
index 5fd1a47..a1b0bea 100644
--- a/tests/qapi-schema/alternate-conflict-string.json
+++ b/tests/qapi-schema/alternate-conflict-string.json
@@ -1,4 +1,4 @@
-# FIXME: we should reject anonymous unions with multiple string-like branches
+# we reject anonymous unions with multiple string-like branches
 { 'enum': 'Enum',
   'data': [ 'hello', 'world' ] }
 { 'union': 'MyUnion',
diff --git a/tests/qapi-schema/alternate-conflict-string.out b/tests/qapi-schema/alternate-conflict-string.out
index e7b39a2..e69de29 100644
--- a/tests/qapi-schema/alternate-conflict-string.out
+++ b/tests/qapi-schema/alternate-conflict-string.out
@@ -1,5 +0,0 @@
-[OrderedDict([('enum', 'Enum'), ('data', ['hello', 'world'])]),
- OrderedDict([('union', 'MyUnion'), ('discriminator', OrderedDict()), ('data', OrderedDict([('one', 'str'), ('two', 'Enum')]))])]
-[{'enum_name': 'Enum', 'enum_values': ['hello', 'world']},
- {'enum_name': 'MyUnionKind', 'enum_values': None}]
-[]
diff --git a/tests/qapi-schema/alternate-nested.err b/tests/qapi-schema/alternate-nested.err
index e69de29..59df96e 100644
--- a/tests/qapi-schema/alternate-nested.err
+++ b/tests/qapi-schema/alternate-nested.err
@@ -0,0 +1 @@
+tests/qapi-schema/alternate-nested.json:5: Anonymous union 'Union2' member 'nested' has invalid type 'Union1'
diff --git a/tests/qapi-schema/alternate-nested.exit b/tests/qapi-schema/alternate-nested.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/alternate-nested.exit
+++ b/tests/qapi-schema/alternate-nested.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/alternate-nested.json b/tests/qapi-schema/alternate-nested.json
index d5812bf..ed2b6b7 100644
--- a/tests/qapi-schema/alternate-nested.json
+++ b/tests/qapi-schema/alternate-nested.json
@@ -1,4 +1,4 @@
-# FIXME: we should reject a nested anonymous union branch
+# we reject a nested anonymous union branch
 { 'union': 'Union1',
   'discriminator': {},
   'data': { 'name': 'str', 'value': 'int' } }
diff --git a/tests/qapi-schema/alternate-nested.out b/tests/qapi-schema/alternate-nested.out
index 0137c1f..e69de29 100644
--- a/tests/qapi-schema/alternate-nested.out
+++ b/tests/qapi-schema/alternate-nested.out
@@ -1,5 +0,0 @@
-[OrderedDict([('union', 'Union1'), ('discriminator', OrderedDict()), ('data', OrderedDict([('name', 'str'), ('value', 'int')]))]),
- OrderedDict([('union', 'Union2'), ('discriminator', OrderedDict()), ('data', OrderedDict([('nested', 'Union1')]))])]
-[{'enum_name': 'Union1Kind', 'enum_values': None},
- {'enum_name': 'Union2Kind', 'enum_values': None}]
-[]
diff --git a/tests/qapi-schema/alternate-unknown.err b/tests/qapi-schema/alternate-unknown.err
index e69de29..bf8e9ae 100644
--- a/tests/qapi-schema/alternate-unknown.err
+++ b/tests/qapi-schema/alternate-unknown.err
@@ -0,0 +1 @@
+tests/qapi-schema/alternate-unknown.json:2: Anonymous union 'Union' member 'unknown' has invalid type 'MissingType'
diff --git a/tests/qapi-schema/alternate-unknown.exit b/tests/qapi-schema/alternate-unknown.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/alternate-unknown.exit
+++ b/tests/qapi-schema/alternate-unknown.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/alternate-unknown.json b/tests/qapi-schema/alternate-unknown.json
index 0bab9c2..0c305c2 100644
--- a/tests/qapi-schema/alternate-unknown.json
+++ b/tests/qapi-schema/alternate-unknown.json
@@ -1,4 +1,4 @@
-# FIXME: we should reject an anonymous union with unknown type in branch
+# we reject an anonymous union with unknown type in branch
 { 'union': 'Union',
   'discriminator': {},
   'data': { 'unknown': 'MissingType' } }
diff --git a/tests/qapi-schema/alternate-unknown.out b/tests/qapi-schema/alternate-unknown.out
index 0911cdc..e69de29 100644
--- a/tests/qapi-schema/alternate-unknown.out
+++ b/tests/qapi-schema/alternate-unknown.out
@@ -1,3 +0,0 @@
-[OrderedDict([('union', 'Union'), ('discriminator', OrderedDict()), ('data', OrderedDict([('unknown', 'MissingType')]))])]
-[{'enum_name': 'UnionKind', 'enum_values': None}]
-[]
diff --git a/tests/qapi-schema/flat-union-bad-discriminator.err b/tests/qapi-schema/flat-union-bad-discriminator.err
index e69de29..628432f 100644
--- a/tests/qapi-schema/flat-union-bad-discriminator.err
+++ b/tests/qapi-schema/flat-union-bad-discriminator.err
@@ -0,0 +1 @@
+tests/qapi-schema/flat-union-bad-discriminator.json:10: Simple union 'TestUnion' must not have a base
diff --git a/tests/qapi-schema/flat-union-bad-discriminator.exit b/tests/qapi-schema/flat-union-bad-discriminator.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/flat-union-bad-discriminator.exit
+++ b/tests/qapi-schema/flat-union-bad-discriminator.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/flat-union-bad-discriminator.out b/tests/qapi-schema/flat-union-bad-discriminator.out
index b6ce217..e69de29 100644
--- a/tests/qapi-schema/flat-union-bad-discriminator.out
+++ b/tests/qapi-schema/flat-union-bad-discriminator.out
@@ -1,10 +0,0 @@
-[OrderedDict([('enum', 'TestEnum'), ('data', ['value1', 'value2'])]),
- OrderedDict([('type', 'TestBase'), ('data', OrderedDict([('enum1', 'TestEnum'), ('kind', 'str')]))]),
- OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]),
- OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))]),
- OrderedDict([('union', 'TestUnion'), ('base', 'TestBase'), ('discriminator', []), ('data', OrderedDict([('kind1', 'TestTypeA'), ('kind2', 'TestTypeB')]))])]
-[{'enum_name': 'TestEnum', 'enum_values': ['value1', 'value2']},
- {'enum_name': 'TestUnionKind', 'enum_values': None}]
-[OrderedDict([('type', 'TestBase'), ('data', OrderedDict([('enum1', 'TestEnum'), ('kind', 'str')]))]),
- OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]),
- OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))])]
diff --git a/tests/qapi-schema/union-bad-branch.err b/tests/qapi-schema/union-bad-branch.err
index e69de29..8822735 100644
--- a/tests/qapi-schema/union-bad-branch.err
+++ b/tests/qapi-schema/union-bad-branch.err
@@ -0,0 +1 @@
+tests/qapi-schema/union-bad-branch.json:6: Union 'MyUnion' member 'ONE' clashes with 'one'
diff --git a/tests/qapi-schema/union-bad-branch.exit b/tests/qapi-schema/union-bad-branch.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/union-bad-branch.exit
+++ b/tests/qapi-schema/union-bad-branch.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/union-bad-branch.json b/tests/qapi-schema/union-bad-branch.json
index f7aeae8..5b577b8 100644
--- a/tests/qapi-schema/union-bad-branch.json
+++ b/tests/qapi-schema/union-bad-branch.json
@@ -1,4 +1,4 @@
-# FIXME: we should reject normal unions where branches would collide in C
+# we reject normal unions where branches would collide in C
 { 'type': 'One',
   'data': { 'string': 'str' } }
 { 'type': 'Two',
diff --git a/tests/qapi-schema/union-bad-branch.out b/tests/qapi-schema/union-bad-branch.out
index 6baf01b..e69de29 100644
--- a/tests/qapi-schema/union-bad-branch.out
+++ b/tests/qapi-schema/union-bad-branch.out
@@ -1,6 +0,0 @@
-[OrderedDict([('type', 'One'), ('data', OrderedDict([('string', 'str')]))]),
- OrderedDict([('type', 'Two'), ('data', OrderedDict([('number', 'int')]))]),
- OrderedDict([('union', 'MyUnion'), ('data', OrderedDict([('one', 'One'), ('ONE', 'Two')]))])]
-[{'enum_name': 'MyUnionKind', 'enum_values': None}]
-[OrderedDict([('type', 'One'), ('data', OrderedDict([('string', 'str')]))]),
- OrderedDict([('type', 'Two'), ('data', OrderedDict([('number', 'int')]))])]
diff --git a/tests/qapi-schema/union-base-no-discriminator.err b/tests/qapi-schema/union-base-no-discriminator.err
index e69de29..f33392d 100644
--- a/tests/qapi-schema/union-base-no-discriminator.err
+++ b/tests/qapi-schema/union-base-no-discriminator.err
@@ -0,0 +1 @@
+tests/qapi-schema/union-base-no-discriminator.json:12: Simple union 'TestUnion' must not have a base
diff --git a/tests/qapi-schema/union-base-no-discriminator.exit b/tests/qapi-schema/union-base-no-discriminator.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/union-base-no-discriminator.exit
+++ b/tests/qapi-schema/union-base-no-discriminator.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/union-base-no-discriminator.json b/tests/qapi-schema/union-base-no-discriminator.json
index b5da546..548a633 100644
--- a/tests/qapi-schema/union-base-no-discriminator.json
+++ b/tests/qapi-schema/union-base-no-discriminator.json
@@ -1,4 +1,5 @@
-# FIXME: either allow base in non-flat unions, or diagnose missing discriminator
+# for now, we reject a base for non-flat unions
+# FIXME: might be a useful extension to allow later
 { 'type': 'TestTypeA',
   'data': { 'string': 'str' } }

diff --git a/tests/qapi-schema/union-base-no-discriminator.out b/tests/qapi-schema/union-base-no-discriminator.out
index 505fd57..e69de29 100644
--- a/tests/qapi-schema/union-base-no-discriminator.out
+++ b/tests/qapi-schema/union-base-no-discriminator.out
@@ -1,8 +0,0 @@
-[OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]),
- OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))]),
- OrderedDict([('type', 'Base'), ('data', OrderedDict([('string', 'str')]))]),
- OrderedDict([('union', 'TestUnion'), ('base', 'Base'), ('data', OrderedDict([('value1', 'TestTypeA'), ('value2', 'TestTypeB')]))])]
-[{'enum_name': 'TestUnionKind', 'enum_values': None}]
-[OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string', 'str')]))]),
- OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer', 'int')]))]),
- OrderedDict([('type', 'Base'), ('data', OrderedDict([('string', 'str')]))])]
diff --git a/tests/qapi-schema/union-max.err b/tests/qapi-schema/union-max.err
index e69de29..55ce439 100644
--- a/tests/qapi-schema/union-max.err
+++ b/tests/qapi-schema/union-max.err
@@ -0,0 +1 @@
+tests/qapi-schema/union-max.json:2: Union 'Union' member 'max' clashes with '(automatic)'
diff --git a/tests/qapi-schema/union-max.exit b/tests/qapi-schema/union-max.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/union-max.exit
+++ b/tests/qapi-schema/union-max.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/union-max.json b/tests/qapi-schema/union-max.json
index 45648c4..d6ad986 100644
--- a/tests/qapi-schema/union-max.json
+++ b/tests/qapi-schema/union-max.json
@@ -1,3 +1,3 @@
-# FIXME: we should reject 'max' branch in a union, for collision with C enum
+# we reject 'max' branch in a union, for collision with C enum
 { 'union': 'Union',
   'data': { 'max': 'int' } }
diff --git a/tests/qapi-schema/union-max.out b/tests/qapi-schema/union-max.out
index 2757d36..e69de29 100644
--- a/tests/qapi-schema/union-max.out
+++ b/tests/qapi-schema/union-max.out
@@ -1,3 +0,0 @@
-[OrderedDict([('union', 'Union'), ('data', OrderedDict([('max', 'int')]))])]
-[{'enum_name': 'UnionKind', 'enum_values': None}]
-[]
-- 
2.1.0