Skip to content

Commit

Permalink
qapi: Permit omitting all flat union branches
Browse files Browse the repository at this point in the history
Absent flat union branches default to the empty struct (since commit
800877b "qapi: allow empty branches in flat unions").  But 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 <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190913201349.24332-11-armbru@redhat.com>
[Commit message typo fixed]
  • Loading branch information
Markus Armbruster committed Sep 24, 2019
1 parent f032553 commit 0ced953
Show file tree
Hide file tree
Showing 8 changed files with 27 additions and 14 deletions.
3 changes: 1 addition & 2 deletions docs/devel/qapi-code-gen.txt
Expand Up @@ -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:
Expand Down
16 changes: 8 additions & 8 deletions scripts/qapi/common.py
Expand Up @@ -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" %
Expand Down Expand Up @@ -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)

Expand All @@ -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'"
Expand Down Expand Up @@ -1578,7 +1579,6 @@ def __init__(self, tag_name, tag_member, variants):
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
Expand Down
2 changes: 1 addition & 1 deletion 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
2 changes: 1 addition & 1 deletion 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': { } }
5 changes: 5 additions & 0 deletions tests/qapi-schema/qapi-schema-test.json
Expand Up @@ -25,6 +25,11 @@
{ 'struct': 'Empty1', 'data': { } }
{ 'struct': 'Empty2', 'base': 'Empty1', 'data': { } }

# Likewise for an empty flat union
{ 'union': 'Union',
'base': { 'type': 'EnumOne' }, 'discriminator': 'type',
'data': { } }

{ 'command': 'user_def_cmd0', 'data': 'Empty2', 'returns': 'Empty2' }

# for testing override of default naming heuristic
Expand Down
9 changes: 9 additions & 0 deletions tests/qapi-schema/qapi-schema-test.out
Expand Up @@ -23,6 +23,15 @@ enum MyEnum
object Empty1
object Empty2
base Empty1
object q_obj_Union-base
member type: EnumOne optional=False
object Union
base q_obj_Union-base
tag type
case value1: q_empty
case value2: q_empty
case value3: q_empty
case value4: q_empty
command user_def_cmd0 Empty2 -> Empty2
gen=True success_response=True boxed=False oob=False preconfig=False
enum QEnumTwo
Expand Down
2 changes: 1 addition & 1 deletion tests/qapi-schema/union-empty.err
@@ -1 +1 @@
tests/qapi-schema/union-empty.json:2: Union 'Union' cannot have empty 'data'
tests/qapi-schema/union-empty.json:2: Union 'Union' has no branches
2 changes: 1 addition & 1 deletion tests/qapi-schema/union-empty.json
@@ -1,2 +1,2 @@
# unions cannot be empty
# simple unions cannot be empty
{ 'union': 'Union', 'data': { } }

0 comments on commit 0ced953

Please sign in to comment.