[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v6 10/36] qapi: Forbid base without discriminator in
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v6 10/36] qapi: Forbid base without discriminator in unions |
Date: |
Sat, 4 Apr 2015 22:07:41 -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. A previous commit added a union-base-no-discriminator
test to ensure that we eventually give a decent error message;
now is the time to actually forbid it, and remove the last
vestiges of that usage from the testsuite.
Signed-off-by: Eric Blake <address@hidden>
---
v6: split from v5:7/28; change subject line; hoist hunk from
v5:8/28 into here to make this be the patch that forbids
simple-with-base
---
scripts/qapi-types.py | 7 ++---
scripts/qapi-visit.py | 13 ++++----
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, 22 insertions(+), 92 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 4416677..3f82bd4 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -2,7 +2,7 @@
# QAPI visitor generator
#
# Copyright IBM, Corp. 2011
-# Copyright (C) 2014 Red Hat, Inc.
+# Copyright (C) 2014-2015 Red Hat, Inc.
#
# Authors:
# Anthony Liguori <address@hidden>
@@ -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 v6 01/36] qapi: Add copyright declaration on docs, (continued)
- [Qemu-devel] [PATCH v6 01/36] qapi: Add copyright declaration on docs, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 04/36] qapi: Fix generation of 'size' builtin type, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 03/36] qapi: Simplify builtin type handling, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 05/36] qapi: Require ASCII in schema, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 12/36] qapi: Prepare for catching more semantic parse errors, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 07/36] qapi: Better error messages for bad enums, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 09/36] qapi: Clean up test coverage of simple unions, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 06/36] qapi: Add some enum tests, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 10/36] qapi: Forbid base without discriminator in unions,
Eric Blake <=
- [Qemu-devel] [PATCH v6 02/36] qapi: Document type-safety considerations, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 15/36] qapi: Document new 'alternate' meta-type, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 13/36] qapi: Segregate anonymous unions into alternates in generator, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 14/36] qapi: Rename anonymous union type in test, Eric Blake, 2015/04/05
- [Qemu-devel] [PATCH v6 11/36] qapi: Tighten checking of unions, Eric Blake, 2015/04/05