[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PULL 10/40] qapi: Forbid base without discriminator in uni
From: |
Markus Armbruster |
Subject: |
[Qemu-devel] [PULL 10/40] qapi: Forbid base without discriminator in unions |
Date: |
Tue, 5 May 2015 18:46:56 +0200 |
From: Eric Blake <address@hidden>
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>
Signed-off-by: Markus Armbruster <address@hidden>
---
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",
--
1.9.3
- [Qemu-devel] [PULL 00/40] drop qapi nested structs, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 01/40] qapi: Add copyright declaration on docs, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 05/40] qapi: Require ASCII in schema, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 03/40] qapi: Simplify builtin type handling, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 12/40] qapi: Prepare for catching more semantic parse errors, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 07/40] qapi: Better error messages for bad enums, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 09/40] qapi: Clean up test coverage of simple unions, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 06/40] qapi: Add some enum tests, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 04/40] qapi: Fix generation of 'size' builtin type, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 10/40] qapi: Forbid base without discriminator in unions,
Markus Armbruster <=
- [Qemu-devel] [PULL 14/40] qapi: Rename anonymous union type in test, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 13/40] qapi: Segregate anonymous unions into alternates in generator, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 15/40] qapi: Document new 'alternate' meta-type, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 11/40] qapi: Tighten checking of unions, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 02/40] qapi: Document type-safety considerations, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 21/40] qapi: Allow true, false and null in schema json, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 19/40] qapi: Add tests of redefined expressions, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 27/40] qapi: More rigorous checking for type safety bypass, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 31/40] qapi: Forbid 'type' in schema, Markus Armbruster, 2015/05/05
- [Qemu-devel] [PULL 18/40] qapi: Better error messages for bad expressions, Markus Armbruster, 2015/05/05