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 'q_empty' struct nor a 'visit_type_q_empty_members()'
corresponding to the special 'q_empty' type (see commit 7ce106a9 for
more reasons why); but when the branch is truly empty, there's nothing
to do. This is easiest by declaring that an empty type member of
another QAPI type is emitted as a 'char' rather than bothering with a
named type, via c_unboxed_type() (this change also affects empty types
used as the branch of a simple union).
In QAPISchemaObjectTypeMember._pretty_owner(), we want to distinguish
which branch introduces any duplicate member with the base of a flat
union. We can't use just the branch name (otherwise, two flat unions
with the same branch name will create a collision in C types), so
we have to include the QAPI type name as well - but since both types
and branches can contain '-' (or even '_'), we need to introduce yet
another character for robust parsing of the generated name back to
the sources, so we use the character ':' in _make_variant() and
transliterate it to '_' in c_name_trans.
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 <ebl...@redhat.com>
---
v8: tweak commit message, drop unneeded hunks, change c_unboxed_type()
handling of empty types, don't lose test of flat union backref
v7: new patch
---
scripts/qapi.py | 26 ++++++++++++++++++++------
scripts/qapi-visit.py | 14 ++++++++++----
tests/Makefile.include | 1 +
tests/qapi-schema/flat-union-inline.err | 2 +-
tests/qapi-schema/flat-union-inline.json | 5 ++---
tests/qapi-schema/qapi-schema-test.json | 8 ++++++--
tests/qapi-schema/qapi-schema-test.out | 7 ++++++-
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
11 files changed, 52 insertions(+), 17 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 e051892..db115eb 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.
@@ -1022,6 +1024,8 @@ class QAPISchemaObjectType(QAPISchemaType):
return c_name(self.name) + pointer_suffix
def c_unboxed_type(self):
+ if self.is_empty():
+ return 'char'
return c_name(self.name)
def json_type(self):
@@ -1067,6 +1071,11 @@ class QAPISchemaMember(object):
return '(parameter of %s)' % owner[:-4]
elif owner.endswith('-base'):
return '(base of %s)' % owner[:-5]
+ elif owner.endswith('-branch'):
+ # See QAPISchema._make_variant() for the further division
+ # of the owner
+ return ('(member of %s branch %s)'
+ % tuple(owner[:-7].split(':')))
else:
assert owner.endswith('-wrapper')
# Unreachable and not implemented
@@ -1362,7 +1371,12 @@ 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):
+ # See also QAPISchemaObjectTypeMember._pretty_owner()
+ 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):
@@ -1383,7 +1397,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:
@@ -1402,7 +1416,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(
@@ -1512,7 +1526,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-visit.py b/scripts/qapi-visit.py
index 96f2491..9fa1f04 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -83,13 +83,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.include b/tests/Makefile.include
index f73f350..8a4c60d 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -394,6 +394,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 919dc097..c527af0 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', 'value4' ] }
# for testing nested structs
{ 'struct': 'UserDefOne',
@@ -81,7 +81,11 @@
'base': { '*integer': 'int', 'string': 'str', 'enum1': 'QEnumTwo' },
'discriminator': 'enum1',
'data': { 'value1' : 'UserDefC', # intentional forward reference
- 'value2' : 'UserDefB' } }
+ 'value2' : 'UserDefB', # intentional back reference
+ 'value3' : { },
+ 'value4' : { 'intb': 'int', '*a-b': 'bool' }
+ # value2 and value4 have same members, but do not collide
+ } }
{ 'struct': 'WrapAlternate',
'data': { 'alt': 'UserDefAlternate' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index e7ea242..fb4ca88 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -52,7 +52,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', 'value4']
prefix QENUM_TWO
enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat',
'qbool']
prefix QTYPE
@@ -85,6 +85,8 @@ object UserDefFlatUnion2
tag enum1
case value1: UserDefC
case value2: UserDefB
+ case value3: q_empty
+ case value4: q_obj_UserDefFlatUnion2:value4-branch
object UserDefNativeListUnion
member type: UserDefNativeListUnionKind optional=False
tag type
@@ -179,6 +181,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:value4-branch
+ member intb: int optional=False
+ member a-b: bool 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
