Re: [Qemu-devel] [PATCH v3 10/16] qapi: Permit omitting all flat union branches

2019-09-23 Thread Markus Armbruster
Eric Blake  writes:

> On 9/13/19 3:13 PM, Markus Armbruster wrote:
>> Absent flat union branches default to the empty struct (since commit
>> 800877bb16 "qapi: allow empty branches in flat unions").  But am
>
> s/am/an/

Will fix, thanks!

>> attempt to omit all of them is rejected with "Union 'FOO' has no
>> branches".  Harmless oddity, but it's easy to avoid, so do that.
>> 
>> Signed-off-by: Markus Armbruster 
>> Reviewed-by: Eric Blake 
>> ---



Re: [Qemu-devel] [PATCH v3 10/16] qapi: Permit omitting all flat union branches

2019-09-17 Thread Eric Blake
On 9/13/19 3:13 PM, Markus Armbruster wrote:
> Absent flat union branches default to the empty struct (since commit
> 800877bb16 "qapi: allow empty branches in flat unions").  But am

s/am/an/

> attempt to omit all of them is rejected with "Union 'FOO' has no
> branches".  Harmless oddity, but it's easy to avoid, so do that.
> 
> Signed-off-by: Markus Armbruster 
> Reviewed-by: Eric Blake 
> ---
-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3226
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH v3 10/16] qapi: Permit omitting all flat union branches

2019-09-13 Thread Markus Armbruster
Absent flat union branches default to the empty struct (since commit
800877bb16 "qapi: allow empty branches in flat unions").  But am
attempt to omit all of them is rejected with "Union 'FOO' has no
branches".  Harmless oddity, but it's easy to avoid, so do that.

Signed-off-by: Markus Armbruster 
Reviewed-by: Eric Blake 
---
 docs/devel/qapi-code-gen.txt|  3 +--
 scripts/qapi/common.py  | 16 
 tests/qapi-schema/flat-union-empty.err  |  2 +-
 tests/qapi-schema/flat-union-empty.json |  2 +-
 tests/qapi-schema/qapi-schema-test.json |  5 +
 tests/qapi-schema/qapi-schema-test.out  |  9 +
 tests/qapi-schema/union-empty.err   |  2 +-
 tests/qapi-schema/union-empty.json  |  2 +-
 8 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 4ce67752a7..ec2d374483 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -436,8 +436,7 @@ Union types are used to let the user choose between several 
different
 variants for an object.  There are two flavors: simple (no
 discriminator or base), and flat (both discriminator and base).  A union
 type is defined using a data dictionary as explained in the following
-paragraphs.  The data dictionary for either type of union must not
-be empty.
+paragraphs.  Unions must have at least one branch.
 
 A simple union type defines a mapping from automatic discriminator
 values to data types like in this example:
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 99db18f3d6..3393a049cc 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -852,7 +852,7 @@ def check_union(expr, info):
 
 # With no discriminator it is a simple union.
 if discriminator is None:
-enum_define = None
+enum_values = members.keys()
 allow_metas = ['built-in', 'union', 'alternate', 'struct', 'enum']
 if base is not None:
 raise QAPISemError(info, "Simple union '%s' must not have a base" %
@@ -885,16 +885,17 @@ def check_union(expr, info):
'must not be conditional' %
(base, discriminator, name))
 enum_define = enum_types.get(discriminator_value['type'])
-allow_metas = ['struct']
 # Do not allow string discriminator
 if not enum_define:
 raise QAPISemError(info,
"Discriminator '%s' must be of enumeration "
"type" % discriminator)
+enum_values = enum_get_names(enum_define)
+allow_metas = ['struct']
+
+if (len(enum_values) == 0):
+raise QAPISemError(info, "Union '%s' has no branches" % name)
 
-# Check every branch; don't allow an empty union
-if len(members) == 0:
-raise QAPISemError(info, "Union '%s' cannot have empty 'data'" % name)
 for (key, value) in members.items():
 check_name(info, "Member of union '%s'" % name, key)
 
@@ -907,8 +908,8 @@ def check_union(expr, info):
 
 # If the discriminator names an enum type, then all members
 # of 'data' must also be members of the enum type.
-if enum_define:
-if key not in enum_get_names(enum_define):
+if discriminator is not None:
+if key not in enum_values:
 raise QAPISemError(info,
"Discriminator value '%s' is not found in "
"enum '%s'"
@@ -1578,7 +1579,6 @@ class QAPISchemaObjectTypeVariants(object):
 assert bool(tag_member) != bool(tag_name)
 assert (isinstance(tag_name, str) or
 isinstance(tag_member, QAPISchemaObjectTypeMember))
-assert len(variants) > 0
 for v in variants:
 assert isinstance(v, QAPISchemaObjectTypeVariant)
 self._tag_name = tag_name
diff --git a/tests/qapi-schema/flat-union-empty.err 
b/tests/qapi-schema/flat-union-empty.err
index 15754f54eb..fedbc0d1cf 100644
--- a/tests/qapi-schema/flat-union-empty.err
+++ b/tests/qapi-schema/flat-union-empty.err
@@ -1 +1 @@
-tests/qapi-schema/flat-union-empty.json:4: Union 'Union' cannot have empty 
'data'
+tests/qapi-schema/flat-union-empty.json:4: Union 'Union' has no branches
diff --git a/tests/qapi-schema/flat-union-empty.json 
b/tests/qapi-schema/flat-union-empty.json
index 77f1d9abfb..83e1cc7b96 100644
--- a/tests/qapi-schema/flat-union-empty.json
+++ b/tests/qapi-schema/flat-union-empty.json
@@ -1,4 +1,4 @@
-# flat unions cannot be empty
+# flat union discriminator cannot be empty
 { 'enum': 'Empty', 'data': [ ] }
 { 'struct': 'Base', 'data': { 'type': 'Empty' } }
 { 'union': 'Union', 'base': 'Base', 'discriminator': 'type', 'data': { } }
diff --git a/tests/qapi-schema/qapi-schema-test.json 
b/tests/qapi-schema/qapi-schema-test.json
index 8b0d47c4ab..75c42eb0e3 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++