[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v7 10/39] qapi: Forbid base without discriminator in
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v7 10/39] qapi: Forbid base without discriminator in unions |
Date: |
Wed, 29 Apr 2015 07:06:25 -0600 |
None of the existing QMP or QGA interfaces uses a union with a
base type but no discriminator; it is easier to avoid this in the
generator to save room for other future extensions more likely to
be useful. An earlier commit added a union-base-no-discriminator
test to ensure that we eventually give a decent error message;
likewise, removing UserDefUnion outright is okay, because we moved
all the tests we wish to keep into the tests of the simple union
UserDefNativeListUnion in the previous commit. Now is the time to
actually forbid simple union with base, and remove the last
vestiges from the testsuite.
Signed-off-by: Eric Blake <address@hidden>
Reviewed-by: Markus Armbruster <address@hidden>
---
v7: improve commit message
---
scripts/qapi-types.py | 7 ++---
scripts/qapi-visit.py | 11 +++----
scripts/qapi.py | 20 ++++++------
tests/qapi-schema/qapi-schema-test.json | 4 ---
tests/qapi-schema/qapi-schema-test.out | 2 --
tests/qapi-schema/union-base-no-discriminator.err | 1 +
tests/qapi-schema/union-base-no-discriminator.exit | 2 +-
tests/qapi-schema/union-base-no-discriminator.json | 2 +-
tests/qapi-schema/union-base-no-discriminator.out | 8 -----
tests/test-qmp-input-visitor.c | 19 ------------
tests/test-qmp-output-visitor.c | 36 ----------------------
11 files changed, 21 insertions(+), 91 deletions(-)
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
index e400b03..f6fb930 100644
--- a/scripts/qapi-types.py
+++ b/scripts/qapi-types.py
@@ -242,10 +242,9 @@ struct %(name)s
''')
if base:
- base_fields = find_struct(base)['data']
- if discriminator:
- base_fields = base_fields.copy()
- del base_fields[discriminator]
+ assert discriminator
+ base_fields = find_struct(base)['data'].copy()
+ del base_fields[discriminator]
ret += generate_struct_fields(base_fields)
else:
assert not discriminator
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index 41596bb..dbf0101 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -310,16 +310,15 @@ def generate_visit_union(expr):
ret = ""
disc_type = enum_define['enum_name']
else:
- # There will always be a discriminator in the C switch code, by
default it
- # is an enum type generated silently as "'%sKind' % (name)"
+ # There will always be a discriminator in the C switch code, by default
+ # it is an enum type generated silently as "'%sKind' % (name)"
ret = generate_visit_enum('%sKind' % name, members.keys())
disc_type = '%sKind' % (name)
if base:
- base_fields = find_struct(base)['data']
- if discriminator:
- base_fields = base_fields.copy()
- del base_fields[discriminator]
+ assert discriminator
+ base_fields = find_struct(base)['data'].copy()
+ del base_fields[discriminator]
ret += generate_visit_struct_fields(name, "", "", base_fields)
if discriminator:
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 3ce8c33..438468e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -259,22 +259,22 @@ def check_union(expr, expr_info):
discriminator = expr.get('discriminator')
members = expr['data']
- # If the object has a member 'base', its value must name a complex type.
- if base:
+ # If the object has a member 'base', its value must name a complex type,
+ # and there must be a discriminator.
+ if base is not None:
+ if discriminator is None:
+ raise QAPIExprError(expr_info,
+ "Union '%s' requires a discriminator to go "
+ "along with base" %name)
base_fields = find_base_fields(base)
if not base_fields:
raise QAPIExprError(expr_info,
"Base '%s' is not a valid type"
% base)
- # If the union object has no member 'discriminator', it's an
- # ordinary union.
- if not discriminator:
- enum_define = None
-
- # Else if the value of member 'discriminator' is {}, it's an
- # anonymous union.
- elif discriminator == {}:
+ # If the union object has no member 'discriminator', it's a
+ # simple union. If 'discriminator' is {}, it is an anonymous union.
+ if not discriminator or discriminator == {}:
enum_define = None
# Else, it's a flat union.
diff --git a/tests/qapi-schema/qapi-schema-test.json
b/tests/qapi-schema/qapi-schema-test.json
index 84f0f07..b134f3f 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -36,10 +36,6 @@
{ 'type': 'UserDefC',
'data': { 'string1': 'str', 'string2': 'str' } }
-{ 'union': 'UserDefUnion',
- 'base': 'UserDefZero',
- 'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
-
{ 'type': 'UserDefUnionBase',
'data': { 'string': 'str', 'enum1': 'EnumOne' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out
b/tests/qapi-schema/qapi-schema-test.out
index 915a61b..664ae7b 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -7,7 +7,6 @@
OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean',
'bool')]))]),
OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer',
'int')]))]),
OrderedDict([('type', 'UserDefC'), ('data', OrderedDict([('string1', 'str'),
('string2', 'str')]))]),
- OrderedDict([('union', 'UserDefUnion'), ('base', 'UserDefZero'), ('data',
OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string',
'str'), ('enum1', 'EnumOne')]))]),
OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefUnionBase'),
('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefA'),
('value2', 'UserDefB'), ('value3', 'UserDefB')]))]),
OrderedDict([('union', 'UserDefFlatUnion2'), ('base', 'UserDefUnionBase'),
('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefC'),
('value2', 'UserDefB'), ('value3', 'UserDefA')]))]),
@@ -24,7 +23,6 @@
OrderedDict([('event', 'EVENT_C'), ('data', OrderedDict([('*a', 'int'),
('*b', 'UserDefOne'), ('c', 'str')]))]),
OrderedDict([('event', 'EVENT_D'), ('data', OrderedDict([('a',
'EventStructOne'), ('b', 'str'), ('*c', 'str'), ('*enum3', 'EnumOne')]))])]
[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},
- {'enum_name': 'UserDefUnionKind', 'enum_values': None},
{'enum_name': 'UserDefAnonUnionKind', 'enum_values': None},
{'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]
[OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1',
'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4',
'EnumOne')]))]),
diff --git a/tests/qapi-schema/union-base-no-discriminator.err
b/tests/qapi-schema/union-base-no-discriminator.err
index e69de29..fc8b79c 100644
--- a/tests/qapi-schema/union-base-no-discriminator.err
+++ b/tests/qapi-schema/union-base-no-discriminator.err
@@ -0,0 +1 @@
+tests/qapi-schema/union-base-no-discriminator.json:11: Union 'TestUnion'
requires a discriminator to go along with base
diff --git a/tests/qapi-schema/union-base-no-discriminator.exit
b/tests/qapi-schema/union-base-no-discriminator.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/union-base-no-discriminator.exit
+++ b/tests/qapi-schema/union-base-no-discriminator.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/union-base-no-discriminator.json
b/tests/qapi-schema/union-base-no-discriminator.json
index c8cba3a..052596c 100644
--- a/tests/qapi-schema/union-base-no-discriminator.json
+++ b/tests/qapi-schema/union-base-no-discriminator.json
@@ -1,4 +1,4 @@
-# FIXME: we should reject simple unions with a base
+# we reject simple unions with a base (or flat unions without discriminator)
{ 'type': 'TestTypeA',
'data': { 'string': 'str' } }
diff --git a/tests/qapi-schema/union-base-no-discriminator.out
b/tests/qapi-schema/union-base-no-discriminator.out
index 505fd57..e69de29 100644
--- a/tests/qapi-schema/union-base-no-discriminator.out
+++ b/tests/qapi-schema/union-base-no-discriminator.out
@@ -1,8 +0,0 @@
-[OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string',
'str')]))]),
- OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer',
'int')]))]),
- OrderedDict([('type', 'Base'), ('data', OrderedDict([('string', 'str')]))]),
- OrderedDict([('union', 'TestUnion'), ('base', 'Base'), ('data',
OrderedDict([('value1', 'TestTypeA'), ('value2', 'TestTypeB')]))])]
-[{'enum_name': 'TestUnionKind', 'enum_values': None}]
-[OrderedDict([('type', 'TestTypeA'), ('data', OrderedDict([('string',
'str')]))]),
- OrderedDict([('type', 'TestTypeB'), ('data', OrderedDict([('integer',
'int')]))]),
- OrderedDict([('type', 'Base'), ('data', OrderedDict([('string', 'str')]))])]
diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c
index 0039ff6..cc33f64 100644
--- a/tests/test-qmp-input-visitor.c
+++ b/tests/test-qmp-input-visitor.c
@@ -293,23 +293,6 @@ static void test_visitor_in_list(TestInputVisitorData
*data,
qapi_free_UserDefOneList(head);
}
-static void test_visitor_in_union(TestInputVisitorData *data,
- const void *unused)
-{
- Visitor *v;
- Error *err = NULL;
- UserDefUnion *tmp;
-
- v = visitor_input_test_init(data, "{ 'type': 'b', 'integer': 41, 'data' :
{ 'integer': 42 } }");
-
- visit_type_UserDefUnion(v, &tmp, NULL, &err);
- g_assert(err == NULL);
- g_assert_cmpint(tmp->kind, ==, USER_DEF_UNION_KIND_B);
- g_assert_cmpint(tmp->integer, ==, 41);
- g_assert_cmpint(tmp->b->integer, ==, 42);
- qapi_free_UserDefUnion(tmp);
-}
-
static void test_visitor_in_union_flat(TestInputVisitorData *data,
const void *unused)
{
@@ -679,8 +662,6 @@ int main(int argc, char **argv)
&in_visitor_data, test_visitor_in_struct_nested);
input_visitor_test_add("/visitor/input/list",
&in_visitor_data, test_visitor_in_list);
- input_visitor_test_add("/visitor/input/union",
- &in_visitor_data, test_visitor_in_union);
input_visitor_test_add("/visitor/input/union-flat",
&in_visitor_data, test_visitor_in_union_flat);
input_visitor_test_add("/visitor/input/union-anon",
diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c
index e5bf40f..ebe6ea3 100644
--- a/tests/test-qmp-output-visitor.c
+++ b/tests/test-qmp-output-visitor.c
@@ -422,40 +422,6 @@ static void
test_visitor_out_list_qapi_free(TestOutputVisitorData *data,
qapi_free_UserDefNestedList(head);
}
-static void test_visitor_out_union(TestOutputVisitorData *data,
- const void *unused)
-{
- QObject *arg, *qvalue;
- QDict *qdict, *value;
-
- Error *err = NULL;
-
- UserDefUnion *tmp = g_malloc0(sizeof(UserDefUnion));
- tmp->kind = USER_DEF_UNION_KIND_A;
- tmp->integer = 41;
- tmp->a = g_malloc0(sizeof(UserDefA));
- tmp->a->boolean = true;
-
- visit_type_UserDefUnion(data->ov, &tmp, NULL, &err);
- g_assert(err == NULL);
- arg = qmp_output_get_qobject(data->qov);
-
- g_assert(qobject_type(arg) == QTYPE_QDICT);
- qdict = qobject_to_qdict(arg);
-
- g_assert_cmpstr(qdict_get_str(qdict, "type"), ==, "a");
- g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 41);
-
- qvalue = qdict_get(qdict, "data");
- g_assert(data != NULL);
- g_assert(qobject_type(qvalue) == QTYPE_QDICT);
- value = qobject_to_qdict(qvalue);
- g_assert_cmpint(qdict_get_bool(value, "boolean"), ==, true);
-
- qapi_free_UserDefUnion(tmp);
- QDECREF(qdict);
-}
-
static void test_visitor_out_union_flat(TestOutputVisitorData *data,
const void *unused)
{
@@ -862,8 +828,6 @@ int main(int argc, char **argv)
&out_visitor_data, test_visitor_out_list);
output_visitor_test_add("/visitor/output/list-qapi-free",
&out_visitor_data,
test_visitor_out_list_qapi_free);
- output_visitor_test_add("/visitor/output/union",
- &out_visitor_data, test_visitor_out_union);
output_visitor_test_add("/visitor/output/union-flat",
&out_visitor_data, test_visitor_out_union_flat);
output_visitor_test_add("/visitor/output/union-anon",
--
2.1.0
- [Qemu-devel] [PATCH v7 00/39] drop qapi nested structs, Eric Blake, 2015/04/29
- [Qemu-devel] [PATCH v7 03/39] qapi: Simplify builtin type handling, Eric Blake, 2015/04/29
- [Qemu-devel] [PATCH v7 06/39] qapi: Add some enum tests, Eric Blake, 2015/04/29
- [Qemu-devel] [PATCH v7 10/39] qapi: Forbid base without discriminator in unions,
Eric Blake <=
- [Qemu-devel] [PATCH v7 12/39] qapi: Prepare for catching more semantic parse errors, Eric Blake, 2015/04/29
- [Qemu-devel] [PATCH v7 02/39] qapi: Document type-safety considerations, Eric Blake, 2015/04/29
- [Qemu-devel] [PATCH v7 15/39] qapi: Document new 'alternate' meta-type, Eric Blake, 2015/04/29
- [Qemu-devel] [PATCH v7 22/39] qapi: Unify type bypass and add tests, Eric Blake, 2015/04/29
- [Qemu-devel] [PATCH v7 27/39] qapi: More rigorous checking for type safety bypass, Eric Blake, 2015/04/29
- [Qemu-devel] [PATCH v7 26/39] qapi: Whitelist commands that don't return dictionary, Eric Blake, 2015/04/29
- [Qemu-devel] [PATCH v7 14/39] qapi: Rename anonymous union type in test, Eric Blake, 2015/04/29
- [Qemu-devel] [PATCH v7 19/39] qapi: Add tests of redefined expressions, Eric Blake, 2015/04/29
- [Qemu-devel] [PATCH v7 11/39] qapi: Tighten checking of unions, Eric Blake, 2015/04/29
- [Qemu-devel] [PATCH v7 29/39] qapi: Document 'struct' metatype, Eric Blake, 2015/04/29