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 '_empty' struct nor a 'visit_type__empty_members()' corresponding to the special ':empty' type; but when the branch is truly empty, there's nothing to do. 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> --- v7: new patch --- scripts/qapi.py | 21 +++++++++++++++------ scripts/qapi-types.py | 10 +++++++--- scripts/qapi-visit.py | 14 ++++++++++---- tests/Makefile | 1 + tests/qapi-schema/flat-union-inline.err | 2 +- tests/qapi-schema/flat-union-inline.json | 5 ++--- tests/qapi-schema/qapi-schema-test.json | 5 +++-- tests/qapi-schema/qapi-schema-test.out | 8 ++++++-- 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 12 files changed, 51 insertions(+), 21 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 7d568d9..4c531e7 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. @@ -1061,6 +1063,9 @@ class QAPISchemaMember(object): return '(parameter of %s)' % owner[:-4] elif owner.endswith('-base'): return '(base of %s)' % owner[:-5] + elif owner.endswith('-branch'): + return ('(member of %s branch %s)' + % tuple(owner[:-7].split(':'))) else: assert owner.endswith('-wrapper') # Unreachable and not implemented @@ -1335,7 +1340,11 @@ 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): + 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): @@ -1356,7 +1365,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: @@ -1375,7 +1384,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( @@ -1485,7 +1494,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-types.py b/scripts/qapi-types.py index 5a9e2da..f1edab2 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -61,7 +61,8 @@ def gen_object(name, base, members, variants): ret = '' if variants: for v in variants.variants: - if isinstance(v.type, QAPISchemaObjectType): + if (isinstance(v.type, QAPISchemaObjectType) + and not (v.type.is_implicit() and v.type.is_empty())): ret += gen_object(v.type.name, v.type.base, v.type.local_members, v.type.variants) @@ -123,11 +124,14 @@ def gen_variants(variants): c_name=c_name(variants.tag_member.name)) for var in variants.variants: + typ = var.type.c_unboxed_type() + if (isinstance(var.type, QAPISchemaObjectType) and + var.type.is_empty() and var.type.is_implicit()): + typ = 'char' ret += mcgen(''' %(c_type)s %(c_name)s; ''', - c_type=var.type.c_unboxed_type(), - c_name=c_name(var.name)) + c_type=typ, c_name=c_name(var.name)) ret += mcgen(''' } u; diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 07ae6d1..46f8b39 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -79,13 +79,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 b/tests/Makefile index 5cd6177..d7d9597 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -379,6 +379,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 df91f3d..5128b49 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' ] } # for testing nested structs { 'struct': 'UserDefOne', @@ -81,7 +81,8 @@ 'base': { '*integer': 'int', 'string': 'str', 'enum1': 'QEnumTwo' }, 'discriminator': 'enum1', 'data': { 'value1' : 'UserDefC', # intentional forward reference - 'value2' : 'UserDefB' } } + 'value2' : { }, + 'value3' : { 'boolean': 'bool', '*number': 'number' } } } { 'struct': 'WrapAlternate', 'data': { 'alt': 'UserDefAlternate' } } diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 8a00c6b..7fac2da 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -50,7 +50,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'] prefix QENUM_TWO enum QType ['none', 'qnull', 'qint', 'qstring', 'qdict', 'qlist', 'qfloat', 'qbool'] prefix QTYPE @@ -82,7 +82,8 @@ object UserDefFlatUnion2 base q_obj_UserDefFlatUnion2-base tag enum1 case value1: UserDefC - case value2: UserDefB + case value2: q_empty + case value3: q_obj_UserDefFlatUnion2:value3-branch object UserDefNativeListUnion member type: UserDefNativeListUnionKind optional=False tag type @@ -179,6 +180,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:value3-branch + member boolean: bool optional=False + member number: number 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