Signed-off-by: Wenchao Xia <xiaw...@linux.vnet.ibm.com> --- docs/qapi-code-gen.txt | 8 +++----- scripts/qapi.py | 6 ++++++ tests/qapi-schema/qapi-schema-test.json | 9 ++++++--- tests/qapi-schema/qapi-schema-test.out | 5 +++-- tests/test-qmp-input-strict.c | 5 ++++- tests/test-qmp-input-visitor.c | 10 +++++++--- tests/test-qmp-output-visitor.c | 10 ++++++---- 7 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index a2e7921..c92add9 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -123,11 +123,9 @@ And it looks like this on the wire: Flat union types avoid the nesting on the wire. They are used whenever a specific field of the base type is declared as the discriminator ('type' is -then no longer generated). The discriminator can be a string field or a -predefined enum field. If it is a string field, a hidden enum type will be -generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check -will be done to verify the correctness. It is recommended to use an enum field. -The above example can then be modified as follows: +then no longer generated). The discriminator should be a predefined enum field, +and a compile time check will be done to verify the correctness. The above +example can then be modified as follows: { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] } { 'type': 'BlockdevCommonOptions', diff --git a/scripts/qapi.py b/scripts/qapi.py index 2a5eb59..c3c118b 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -215,6 +215,7 @@ def check_union(expr_elem): enum_define = discriminator_find_enum_define(expr_elem) name = expr_elem['expr']['union'] members = expr_elem['expr']['data'] + discriminator = expr_elem['expr'].get('discriminator') if enum_define: for key in members: if not key in enum_define['enum_values']: @@ -228,6 +229,11 @@ def check_union(expr_elem): "Enum value '%s' is not covered by a " "branch of union '%s'" % (key, name)) + elif discriminator: + # Do not allow string discriminator + raise QAPIExprError(expr_elem, + "Discriminator '%s' is not a pre-defined enum " + "type" % discriminator) def check_exprs(schema): for expr_elem in schema.exprs: diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index 471ba47..818c06d 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -37,10 +37,13 @@ 'base': 'UserDefZero', 'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } } +{ 'type': 'UserDefUnionBase', + 'data': { 'string': 'str', 'enum1': 'EnumOne' } } + { 'union': 'UserDefFlatUnion', - 'base': 'UserDefOne', - 'discriminator': 'string', - 'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } } + 'base': 'UserDefUnionBase', + 'discriminator': 'enum1', + 'data': { 'value1' : 'UserDefA', 'value2' : 'UserDefB', 'value3' : 'UserDefB' } } # FIXME generated struct UserDefFlatUnion has members for direct base # UserDefOne, but lacks members for indirect base UserDefZero diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 01685d4..6cd03f3 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -7,7 +7,8 @@ OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]), OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]), OrderedDict([('union', 'UserDefUnion'), ('base', 'UserDefZero'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]), - OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefOne'), ('discriminator', 'string'), ('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', 'UserDefAnonUnion'), ('discriminator', OrderedDict()), ('data', OrderedDict([('uda', 'UserDefA'), ('s', 'str'), ('i', 'int')]))]), OrderedDict([('union', 'UserDefNativeListUnion'), ('data', OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), ('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), ('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', ['bool']), ('string', ['str'])]))]), OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]), @@ -17,7 +18,6 @@ OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])] [{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']}, {'enum_name': 'UserDefUnionKind', 'enum_values': None}, - {'enum_name': 'UserDefFlatUnionKind', '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')]))]), @@ -27,4 +27,5 @@ OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]), OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]), OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]), + OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]), OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))])] diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c index 09fc1ef..c715a50 100644 --- a/tests/test-qmp-input-strict.c +++ b/tests/test-qmp-input-strict.c @@ -146,7 +146,10 @@ static void test_validate_union_flat(TestInputVisitorData *data, Visitor *v; Error *errp = NULL; - v = validate_test_init(data, "{ 'string': 'a', 'boolean': true }"); + v = validate_test_init(data, + "{ 'enum1': 'value1',\ + 'string': 'str', \ + 'boolean': true }"); /* TODO when generator bug is fixed, add 'integer': 41 */ visit_type_UserDefFlatUnion(v, &tmp, NULL, &errp); diff --git a/tests/test-qmp-input-visitor.c b/tests/test-qmp-input-visitor.c index f1ab541..b549746 100644 --- a/tests/test-qmp-input-visitor.c +++ b/tests/test-qmp-input-visitor.c @@ -310,14 +310,18 @@ static void test_visitor_in_union_flat(TestInputVisitorData *data, Error *err = NULL; UserDefFlatUnion *tmp; - v = visitor_input_test_init(data, "{ 'string': 'a', 'boolean': true }"); + v = visitor_input_test_init(data, + "{ 'enum1': 'value1',\ + 'string': 'str', \ + 'boolean': true }"); /* TODO when generator bug is fixed, add 'integer': 41 */ visit_type_UserDefFlatUnion(v, &tmp, NULL, &err); g_assert(err == NULL); - g_assert_cmpint(tmp->kind, ==, USER_DEF_UNION_KIND_A); + g_assert_cmpint(tmp->kind, ==, ENUM_ONE_VALUE1); + g_assert_cmpstr(tmp->string, ==, "str"); /* TODO g_assert_cmpint(tmp->integer, ==, 41); */ - g_assert_cmpint(tmp->a->boolean, ==, true); + g_assert_cmpint(tmp->value1->boolean, ==, true); qapi_free_UserDefFlatUnion(tmp); } diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c index 5613396..6f4abb7 100644 --- a/tests/test-qmp-output-visitor.c +++ b/tests/test-qmp-output-visitor.c @@ -449,10 +449,11 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data, Error *err = NULL; UserDefFlatUnion *tmp = g_malloc0(sizeof(UserDefFlatUnion)); - tmp->kind = USER_DEF_UNION_KIND_A; - tmp->a = g_malloc0(sizeof(UserDefA)); + tmp->kind = ENUM_ONE_VALUE1; + tmp->string = g_strdup("str"); + tmp->value1 = g_malloc0(sizeof(UserDefA)); /* TODO when generator bug is fixed: tmp->integer = 41; */ - tmp->a->boolean = true; + tmp->value1->boolean = true; visit_type_UserDefFlatUnion(data->ov, &tmp, NULL, &err); g_assert(err == NULL); @@ -461,7 +462,8 @@ static void test_visitor_out_union_flat(TestOutputVisitorData *data, g_assert(qobject_type(arg) == QTYPE_QDICT); qdict = qobject_to_qdict(arg); - g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "a"); + g_assert_cmpstr(qdict_get_str(qdict, "enum1"), ==, "value1"); + g_assert_cmpstr(qdict_get_str(qdict, "string"), ==, "str"); /* TODO g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 41); */ g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, true); -- 1.7.1