[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v5 46/46] qapi: Allow anonymous base for flat union
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v5 46/46] qapi: Allow anonymous base for flat union |
Date: |
Mon, 21 Sep 2015 15:58:02 -0600 |
Rather than requiring all flat unions to explicitly create
a separate base struct, we want to allow the qapi schema
to specify the common fields via an inline dictionary. This
is similar to how commands can specify inline types for the
arguments.
Now that the feature is legal, we can drop the former
flat-union-bad-base negative test, and instead change the
positive tests in qapi-schema-test to use it.
Signed-off-by: Eric Blake <address@hidden>
---
docs/qapi-code-gen.txt | 28 ++++++++++++++--------------
scripts/qapi-commands.py | 2 +-
scripts/qapi-event.py | 2 +-
scripts/qapi-types.py | 2 +-
scripts/qapi-visit.py | 15 +++++++++++----
scripts/qapi.py | 20 +++++++++++++-------
tests/Makefile | 1 -
tests/qapi-schema/flat-union-bad-base.err | 1 -
tests/qapi-schema/flat-union-bad-base.exit | 1 -
tests/qapi-schema/flat-union-bad-base.json | 13 -------------
tests/qapi-schema/flat-union-bad-base.out | 0
tests/qapi-schema/qapi-schema-test.json | 2 +-
tests/qapi-schema/qapi-schema-test.out | 5 ++++-
13 files changed, 46 insertions(+), 46 deletions(-)
delete mode 100644 tests/qapi-schema/flat-union-bad-base.err
delete mode 100644 tests/qapi-schema/flat-union-bad-base.exit
delete mode 100644 tests/qapi-schema/flat-union-bad-base.json
delete mode 100644 tests/qapi-schema/flat-union-bad-base.out
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
index 49e3586..b14dc98 100644
--- a/docs/qapi-code-gen.txt
+++ b/docs/qapi-code-gen.txt
@@ -278,7 +278,7 @@ better than open-coding the field to be type 'str'.
=== Union types ===
Usage: { 'union': STRING, 'data': DICT }
-or: { 'union': STRING, 'data': DICT, 'base': STRUCT-NAME,
+or: { 'union': STRING, 'data': DICT, 'base': STRUCT-NAME-OR-DICT,
'discriminator': ENUM-MEMBER-OF-BASE }
Union types are used to let the user choose between several different
@@ -314,13 +314,16 @@ an implicit C enum 'NameKind' is created, corresponding
to the union
the union can be named 'max', as this would collide with the implicit
enum. The value for each branch can be of any type.
-A flat union definition specifies a struct as its base, and
-avoids nesting on the wire. All branches of the union must be
-complex types, and the top-level fields of the union dictionary on
-the wire will be combination of fields from both the base type and the
-appropriate branch type (when merging two dictionaries, there must be
-no keys in common). The 'discriminator' field must be the name of an
-enum-typed member of the base struct.
+A flat union definition avoids nesting on the wire, and specifies a
+set of common fields that occur in all variants of the union. The
+'base' key must specifiy either a type name (the type must be a
+struct, not a union), or a dictionary representing an anonymous type.
+All branches of the union must be complex types, and the top-level
+fields of the union dictionary on the wire will be combination of
+fields from both the base type and the appropriate branch type (when
+merging two dictionaries, there must be no keys in common). The
+'discriminator' field must be the name of an enum-typed member of the
+base struct.
The following example enhances the above simple union example by
adding a common field 'readonly', renaming the discriminator to
@@ -328,10 +331,8 @@ something more applicable, and reducing the number of {}
required on
the wire:
{ 'enum': 'BlockdevDriver', 'data': [ 'file', 'qcow2' ] }
- { 'struct': 'BlockdevCommonOptions',
- 'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
{ 'union': 'BlockdevOptions',
- 'base': 'BlockdevCommonOptions',
+ 'base': { 'driver': 'BlockdevDriver', 'readonly': 'bool' },
'discriminator': 'driver',
'data': { 'file': 'FileOptions',
'qcow2': 'Qcow2Options' } }
@@ -349,7 +350,7 @@ code generator can ensure that branches exist for all
values of the
enum (although the order of the keys need not match the declaration of
the enum). In the resulting generated C data types, a flat union is
represented as a struct with the base member fields included directly,
-and then a union of structures for each branch of the struct.
+and then a union of pointers to structures for each branch of the struct.
A simple union can always be re-written as a flat union where the base
class has a single member named 'type', and where each branch of the
@@ -360,10 +361,9 @@ union has a struct with a single member named 'data'.
That is,
is identical on the wire to:
{ 'enum': 'Enum', 'data': ['one', 'two'] }
- { 'struct': 'Base', 'data': { 'type': 'Enum' } }
{ 'struct': 'Branch1', 'data': { 'data': 'str' } }
{ 'struct': 'Branch2', 'data': { 'data': 'int' } }
- { 'union': 'Flat', 'base': 'Base', 'discriminator': 'type',
+ { 'union': 'Flat': 'base': { 'type': 'Enum' }, 'discriminator': 'type',
'data': { 'one': 'Branch1', 'two': 'Branch2' } }
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 0256c27..9d2708f 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -139,7 +139,7 @@ visit_type_%(c_name)s(v, &arg, NULL, %(errp)s);
c_name=arg_type.c_name(), errp=errparg)
ret += gen_err_check(errarg)
else:
- ret += gen_visit_fields(arg_type.members, '', False, errarg)
+ ret += gen_visit_fields(arg_type.members, '', False, errarg, 'out')
if dealloc:
ret += mcgen('''
diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
index b46989c..203ba3b 100644
--- a/scripts/qapi-event.py
+++ b/scripts/qapi-event.py
@@ -90,7 +90,7 @@ def gen_event_send(name, arg_type, box):
''',
name=name)
push_indent()
- ret += gen_visit_fields(arg_type.members, '', True, 'err')
+ ret += gen_visit_fields(arg_type.members, '', True, 'err', 'out')
pop_indent()
ret += mcgen('''
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index e7f7c36..31eccfe 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -102,7 +102,7 @@ struct %(c_name)s {
''',
c_name=c_name(name))
if base:
- ret += gen_struct_fields([], base)
+ ret += gen_struct_fields([], base, base.is_implicit())
tag_name = variants.tag_member.name
elif not variants.tag_member:
ret += mcgen('''
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 00be0dc..3321255 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -98,7 +98,7 @@ if (err) {
''',
c_type=base.c_name())
- ret += gen_visit_fields(members, '(*obj)->', False, 'err')
+ ret += gen_visit_fields(members, '(*obj)->', False, 'err', 'out')
pop_indent()
if base or members:
@@ -244,7 +244,7 @@ out:
def gen_visit_union(name, base, variants):
ret = ''
- if base:
+ if base and not base.is_implicit():
ret += gen_visit_struct_fields(base.name, base.base,
base.local_members)
@@ -272,10 +272,17 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj,
const char *name, Error
tag_key = variants.tag_member.name
if base:
- ret += mcgen('''
+ if not base.is_implicit():
+ ret += mcgen('''
visit_type_%(c_name)s_fields(v, (%(c_name)s **)obj, &err);
''',
- c_name=c_name(base.name))
+ c_name=c_name(base.name))
+ else:
+ push_indent()
+ ret += gen_visit_fields(base.members, '(*obj)->', False,
+ 'err', 'out_obj')
+ pop_indent()
+ tag_key = variants.tag_member.name
else:
ret += mcgen('''
visit_type_%(c_type)s(v, &(*obj)->%(c_name)s, "%(name)s", &err);
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 66d9000..0640e2b 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -317,6 +317,8 @@ class QAPISchemaParser(object):
def find_base_fields(base):
+ if isinstance(base, dict):
+ return base
base_struct_define = find_struct(base)
if not base_struct_define:
return None
@@ -557,9 +559,9 @@ def check_union(expr, expr_info):
# Else, it's a flat union.
else:
- # The object must have a string member 'base'.
+ # The object must have a string or dictionary 'base'.
check_type(expr_info, "'base' for union '%s'" % name,
- base, allow_metas=['struct'])
+ base, allow_dict=True, allow_metas=['struct'])
if not base:
raise QAPIExprError(expr_info,
"Flat union '%s' must have a valid base"
@@ -1277,6 +1279,10 @@ class QAPISchema(object):
base = expr.get('base')
tag_name = expr.get('discriminator')
tag_enum = None
+ if isinstance(base, dict):
+ base = self._make_implicit_object_type(name, info, 'base',
+ self._make_members(base,
+ name))
if tag_name:
variants = [self._make_variant(key, value, name)
for (key, value) in data.iteritems()]
@@ -1596,18 +1602,18 @@ def gen_params(arg_type, box, extra):
return ret
-def gen_err_check(err):
+def gen_err_check(err, label='out'):
if not err:
return ''
return mcgen('''
if (%(err)s) {
- goto out;
+ goto %(label)s;
}
''',
- err=err)
+ err=err, label=label)
-def gen_visit_fields(members, prefix, need_cast, errarg):
+def gen_visit_fields(members, prefix, need_cast, errarg, label):
ret = ''
if errarg:
errparg = '&' + errarg
@@ -1634,7 +1640,7 @@ visit_type_%(c_type)s(v, %(cast)s&%(prefix)s%(c_name)s,
"%(name)s", %(errp)s);
c_type=memb.type.c_name(), prefix=prefix, cast=cast,
c_name=c_name(memb.name), name=memb.name,
errp=errparg)
- ret += gen_err_check(errarg)
+ ret += gen_err_check(errarg, label)
if memb.optional:
pop_indent()
diff --git a/tests/Makefile b/tests/Makefile
index 8ce3665..7276b54 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -275,7 +275,6 @@ qapi-schema += event-case.json
qapi-schema += event-max.json
qapi-schema += event-nest-struct.json
qapi-schema += flat-union-array-branch.json
-qapi-schema += flat-union-bad-base.json
qapi-schema += flat-union-bad-discriminator.json
qapi-schema += flat-union-base-any.json
qapi-schema += flat-union-base-union.json
diff --git a/tests/qapi-schema/flat-union-bad-base.err
b/tests/qapi-schema/flat-union-bad-base.err
deleted file mode 100644
index 79b8a71..0000000
--- a/tests/qapi-schema/flat-union-bad-base.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/flat-union-bad-base.json:9: 'base' for union 'TestUnion'
should be a type name
diff --git a/tests/qapi-schema/flat-union-bad-base.exit
b/tests/qapi-schema/flat-union-bad-base.exit
deleted file mode 100644
index d00491f..0000000
--- a/tests/qapi-schema/flat-union-bad-base.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/flat-union-bad-base.json
b/tests/qapi-schema/flat-union-bad-base.json
deleted file mode 100644
index e2e622b..0000000
--- a/tests/qapi-schema/flat-union-bad-base.json
+++ /dev/null
@@ -1,13 +0,0 @@
-# we require the base to be an existing struct
-# TODO: should we allow an anonymous inline base type?
-{ 'enum': 'TestEnum',
- 'data': [ 'value1', 'value2' ] }
-{ 'struct': 'TestTypeA',
- 'data': { 'string': 'str' } }
-{ 'struct': 'TestTypeB',
- 'data': { 'integer': 'int' } }
-{ 'union': 'TestUnion',
- 'base': { 'enum1': 'TestEnum', 'kind': 'str' },
- 'discriminator': 'enum1',
- 'data': { 'value1': 'TestTypeA',
- 'value2': 'TestTypeB' } }
diff --git a/tests/qapi-schema/flat-union-bad-base.out
b/tests/qapi-schema/flat-union-bad-base.out
deleted file mode 100644
index e69de29..0000000
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index 4d7fb07..ad37013 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -56,7 +56,7 @@
# this variant of UserDefFlatUnion defaults to a union that uses fields with
# allocated types to test corner cases in the cleanup/dealloc visitor
{ 'union': 'UserDefFlatUnion2',
- 'base': 'UserDefUnionBase',
+ 'base': { 'string': 'str', 'enum1': 'EnumOne' },
'discriminator': 'enum1',
'data': { 'value1' : 'UserDefC', # intentional forward reference
'value2' : 'UserDefB',
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index a8730fc..b3b4cb8 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -8,6 +8,9 @@ object :obj-EVENT_D-arg
member b: str optional=False
member c: str optional=True
member enum3: EnumOne optional=True
+object :obj-UserDefFlatUnion2-base
+ member string: str optional=False
+ member enum1: EnumOne optional=False
object :obj-__org.qemu_x-command-arg
member a: __org.qemu_x-EnumList optional=False
member b: __org.qemu_x-StructList optional=False
@@ -113,7 +116,7 @@ object UserDefFlatUnion
case value2: UserDefB
case value3: UserDefB
object UserDefFlatUnion2
- base UserDefUnionBase
+ base :obj-UserDefFlatUnion2-base
tag enum1
case value1: UserDefC
case value2: UserDefB
--
2.4.3
- [Qemu-devel] [PATCH v5 34/46] qapi: Unbox base members, (continued)
- [Qemu-devel] [PATCH v5 34/46] qapi: Unbox base members, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 36/46] qapi: Avoid use of 'data' member of qapi unions, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 38/46] qapi: Drop useless 'data' member of unions, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 37/46] qapi: Forbid empty unions and useless alternates, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 39/46] qapi: Plumb in 'box' to qapi generator lower levels, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 41/46] qapi: Support boxed unions, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 40/46] qapi: Implement boxed structs for commands/events, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 42/46] qapi: support implicit structs in OptsVisitor, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 31/46] qapi: use 'type' in generated C code to match QMP union wire form, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 44/46] net: Use correct type for bool flag, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 46/46] qapi: Allow anonymous base for flat union,
Eric Blake <=
- [Qemu-devel] [PATCH v5 43/46] qapi: Change Netdev into a flat union, Eric Blake, 2015/09/21
- [Qemu-devel] [PATCH v5 45/46] net: Complete qapi-fication of netdev_add, Eric Blake, 2015/09/21
Re: [Qemu-devel] [PATCH v5 00/46] post-introspection cleanups, and qapi-ify netdev_add, Markus Armbruster, 2015/09/29