From ab916faddd16f0165e9cc2551f90699be8efde53 Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Mon, 4 May 2015 09:05:13 -0600 Subject: [PATCH] qapi: Use 'alternate' to replace anonymous union Previous patches have led up to the point where I create the new meta-type "'alternate':'Foo'". See the previous patches for documentation; I intentionally split as much work into earlier patches to minimize the size of this patch, but a lot of it is churn due to testsuite fallout after updating to the new type. Signed-off-by: Eric Blake Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- qapi/block-core.json | 6 ++-- scripts/qapi-types.py | 26 ++++++++++----- scripts/qapi-visit.py | 17 ++++++---- scripts/qapi.py | 32 ++++++++----------- tests/qapi-schema/alternate-array.err | 2 +- tests/qapi-schema/alternate-array.json | 5 ++- tests/qapi-schema/alternate-base.err | 2 +- tests/qapi-schema/alternate-base.json | 5 ++- tests/qapi-schema/alternate-clash.err | 2 +- tests/qapi-schema/alternate-clash.json | 5 ++- tests/qapi-schema/alternate-conflict-dict.err | 2 +- .../qapi-schema/alternate-conflict-dict.json | 5 ++- .../qapi-schema/alternate-conflict-string.err | 2 +- .../alternate-conflict-string.json | 5 ++- tests/qapi-schema/alternate-good.json | 5 ++- tests/qapi-schema/alternate-good.out | 4 +-- tests/qapi-schema/alternate-nested.err | 2 +- tests/qapi-schema/alternate-nested.json | 10 +++--- tests/qapi-schema/alternate-unknown.err | 2 +- tests/qapi-schema/alternate-unknown.json | 5 ++- .../flat-union-bad-discriminator.err | 2 +- .../flat-union-bad-discriminator.json | 3 +- tests/qapi-schema/qapi-schema-test.json | 3 +- tests/qapi-schema/qapi-schema-test.out | 2 +- 24 files changed, 77 insertions(+), 77 deletions(-) diff --git a/qapi/block-core.json b/qapi/block-core.json index 1c17224c77..3d20e61dee 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -1503,8 +1503,7 @@ # # Since: 2.2 ## -{ 'union': 'Qcow2OverlapChecks', - 'discriminator': {}, +{ 'alternate': 'Qcow2OverlapChecks', 'data': { 'flags': 'Qcow2OverlapCheckFlags', 'mode': 'Qcow2OverlapCheckMode' } } @@ -1795,8 +1794,7 @@ # # Since: 1.7 ## -{ 'union': 'BlockdevRef', - 'discriminator': {}, +{ 'alternate': 'BlockdevRef', 'data': { 'definition': 'BlockdevOptions', 'reference': 'str' } } diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index c9e0201d10..9c8d68cfb4 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -172,7 +172,7 @@ typedef enum %(name)s def generate_alternate_qtypes(expr): - name = expr['union'] + name = expr['alternate'] members = expr['data'] ret = mcgen(''' @@ -182,7 +182,7 @@ const int %(name)s_qtypes[QTYPE_MAX] = { for key in members: qtype = find_alternate_member_qtype(members[key]) - assert qtype, "Invalid anonymous union member" + assert qtype, "Invalid alternate member" ret += mcgen(''' [ %(qtype)s ] = %(abbrev)s_KIND_%(enum)s, @@ -197,9 +197,9 @@ const int %(name)s_qtypes[QTYPE_MAX] = { return ret -def generate_union(expr): +def generate_union(expr, meta): - name = expr['union'] + name = expr[meta] typeinfo = expr['data'] base = expr.get('base') @@ -243,7 +243,7 @@ struct %(name)s ret += mcgen(''' }; ''') - if discriminator == {}: + if meta == 'alternate': ret += mcgen(''' extern const int %(name)s_qtypes[]; ''', @@ -407,8 +407,12 @@ for expr in exprs: ret += generate_enum('%sKind' % expr['union'], expr['data'].keys()) fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys())) - if expr.get('discriminator') == {}: - fdef.write(generate_alternate_qtypes(expr)) + elif expr.has_key('alternate'): + ret += generate_fwd_struct(expr['alternate'], expr['data']) + "\n" + ret += generate_enum('%sKind' % expr['alternate'], expr['data'].keys()) + fdef.write(generate_enum_lookup('%sKind' % expr['alternate'], + expr['data'].keys())) + fdef.write(generate_alternate_qtypes(expr)) else: continue fdecl.write(ret) @@ -438,11 +442,17 @@ for expr in exprs: ret += generate_type_cleanup_decl(expr['type']) fdef.write(generate_type_cleanup(expr['type']) + "\n") elif expr.has_key('union'): - ret += generate_union(expr) + ret += generate_union(expr, 'union') ret += generate_type_cleanup_decl(expr['union'] + "List") fdef.write(generate_type_cleanup(expr['union'] + "List") + "\n") ret += generate_type_cleanup_decl(expr['union']) fdef.write(generate_type_cleanup(expr['union']) + "\n") + elif expr.has_key('alternate'): + ret += generate_union(expr, 'alternate') + ret += generate_type_cleanup_decl(expr['alternate'] + "List") + fdef.write(generate_type_cleanup(expr['alternate'] + "List") + "\n") + ret += generate_type_cleanup_decl(expr['alternate']) + fdef.write(generate_type_cleanup(expr['alternate']) + "\n") elif expr.has_key('enum'): ret += generate_type_cleanup_decl(expr['enum'] + "List") fdef.write(generate_type_cleanup(expr['enum'] + "List") + "\n") diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 6bd2b6bfab..9222671ff8 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -256,7 +256,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e ''', name=name) - # For anon union, always use the default enum type automatically generated + # For alternate, always use the default enum type automatically generated # as "'%sKind' % (name)" disc_type = '%sKind' % (name) @@ -264,7 +264,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e assert (members[key] in builtin_types.keys() or find_struct(members[key]) or find_union(members[key]) - or find_enum(members[key])), "Invalid anonymous union member" + or find_enum(members[key])), "Invalid alternate member" enum_full_value = generate_enum_full_value(disc_type, key) ret += mcgen(''' @@ -300,10 +300,6 @@ def generate_visit_union(expr): base = expr.get('base') discriminator = expr.get('discriminator') - if discriminator == {}: - assert not base - return generate_visit_alternate(name, members) - enum_define = discriminator_find_enum_define(expr) if enum_define: # Use the enum type as discriminator @@ -568,6 +564,15 @@ for expr in exprs: expr['data'].keys()) ret += generate_declaration(expr['union'], expr['data']) fdecl.write(ret) + elif expr.has_key('alternate'): + ret = generate_visit_alternate(expr['alternate'], expr['data']) + ret += generate_visit_list(expr['alternate'], expr['data']) + fdef.write(ret) + + ret = generate_decl_enum('%sKind' % expr['alternate'], + expr['data'].keys()) + ret += generate_declaration(expr['alternate'], expr['data']) + fdecl.write(ret) elif expr.has_key('enum'): ret = generate_visit_list(expr['enum'], expr['data']) ret += generate_visit_enum(expr['enum'], expr['data']) diff --git a/scripts/qapi.py b/scripts/qapi.py index 0b88325abd..05c38c5b00 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -281,7 +281,6 @@ def check_union(expr, expr_info): "along with base" %name) # Two types of unions, determined by discriminator. - assert discriminator != {} # With no discriminator it is a simple union. if discriminator is None: @@ -344,17 +343,14 @@ def check_union(expr, expr_info): values[c_key] = key def check_alternate(expr, expr_info): - name = expr['union'] - base = expr.get('base') - discriminator = expr.get('discriminator') + name = expr['alternate'] members = expr['data'] values = { 'MAX': '(automatic)' } types_seen = {} - assert discriminator == {} - if base is not None: + if expr.get('base') is not None: raise QAPIExprError(expr_info, - "Anonymous union '%s' must not have a base" + "Alternate '%s' must not have a base" % name) # Check every branch @@ -363,23 +359,23 @@ def check_alternate(expr, expr_info): c_key = _generate_enum_string(key) if c_key in values: raise QAPIExprError(expr_info, - "Anonymous union '%s' member '%s' clashes " - "with '%s'" % (name, key, values[c_key])) + "Alternate '%s' member '%s' clashes with '%s'" + % (name, key, values[c_key])) values[c_key] = key # Ensure alternates have no type conflicts. if isinstance(value, list): raise QAPIExprError(expr_info, - "Anonymous union '%s' member '%s' must " + "Alternate '%s' member '%s' must " "not be array type" % (name, key)) qtype = find_alternate_member_qtype(value) if not qtype: raise QAPIExprError(expr_info, - "Anonymous union '%s' member '%s' has " + "Alternate '%s' member '%s' has " "invalid type '%s'" % (name, key, value)) if qtype in types_seen: raise QAPIExprError(expr_info, - "Anonymous union '%s' member '%s' can't " + "Alternate '%s' member '%s' can't " "be distinguished from member '%s'" % (name, key, types_seen[qtype])) types_seen[qtype] = key @@ -412,10 +408,9 @@ def check_exprs(schema): if expr.has_key('enum'): check_enum(expr, info) elif expr.has_key('union'): - if expr.get('discriminator') == {}: - check_alternate(expr, info) - else: - check_union(expr, info) + check_union(expr, info) + elif expr.has_key('alternate'): + check_alternate(expr, info) elif expr.has_key('event'): check_event(expr, info) @@ -447,6 +442,8 @@ def parse_schema(input_file): if expr.has_key('union'): if not discriminator_find_enum_define(expr): add_enum('%sKind' % expr['union']) + elif expr.has_key('alternate'): + add_enum('%sKind' % expr['alternate']) # Final pass - validate that exprs make sense check_exprs(schema) @@ -557,8 +554,7 @@ def find_struct(name): def add_union(definition): global union_types - if definition.get('discriminator') != {}: - union_types.append(definition) + union_types.append(definition) def find_union(name): global union_types diff --git a/tests/qapi-schema/alternate-array.err b/tests/qapi-schema/alternate-array.err index 8d6ccc7325..e2a5fc29bf 100644 --- a/tests/qapi-schema/alternate-array.err +++ b/tests/qapi-schema/alternate-array.err @@ -1 +1 @@ -tests/qapi-schema/alternate-array.json:5: Anonymous union 'MyUnion' member 'two' must not be array type +tests/qapi-schema/alternate-array.json:5: Alternate 'Alt' member 'two' must not be array type diff --git a/tests/qapi-schema/alternate-array.json b/tests/qapi-schema/alternate-array.json index 0da1a641b3..fc0632f128 100644 --- a/tests/qapi-schema/alternate-array.json +++ b/tests/qapi-schema/alternate-array.json @@ -1,8 +1,7 @@ -# we do not allow array branches in anonymous unions +# we do not allow array branches in alternates # TODO: should we support this? { 'type': 'One', 'data': { 'name': 'str' } } -{ 'union': 'MyUnion', - 'discriminator': {}, +{ 'alternate': 'Alt', 'data': { 'one': 'One', 'two': [ 'int' ] } } diff --git a/tests/qapi-schema/alternate-base.err b/tests/qapi-schema/alternate-base.err index a2486b8407..4a2566eed3 100644 --- a/tests/qapi-schema/alternate-base.err +++ b/tests/qapi-schema/alternate-base.err @@ -1 +1 @@ -tests/qapi-schema/alternate-base.json:4: Anonymous union 'MyUnion' must not have a base +tests/qapi-schema/alternate-base.json:4: Alternate 'Alt' must not have a base diff --git a/tests/qapi-schema/alternate-base.json b/tests/qapi-schema/alternate-base.json index dad7f02998..66edc89640 100644 --- a/tests/qapi-schema/alternate-base.json +++ b/tests/qapi-schema/alternate-base.json @@ -1,7 +1,6 @@ -# we reject anonymous union with base type +# we reject alternate with base type { 'type': 'Base', 'data': { 'string': 'str' } } -{ 'union': 'MyUnion', +{ 'alternate': 'Alt', 'base': 'Base', - 'discriminator': {}, 'data': { 'number': 'int' } } diff --git a/tests/qapi-schema/alternate-clash.err b/tests/qapi-schema/alternate-clash.err index 8949f52b3e..51bea3e272 100644 --- a/tests/qapi-schema/alternate-clash.err +++ b/tests/qapi-schema/alternate-clash.err @@ -1 +1 @@ -tests/qapi-schema/alternate-clash.json:2: Anonymous union 'Union1' member 'ONE' clashes with 'one' +tests/qapi-schema/alternate-clash.json:2: Alternate 'Alt1' member 'ONE' clashes with 'one' diff --git a/tests/qapi-schema/alternate-clash.json b/tests/qapi-schema/alternate-clash.json index fa2d27ed5e..39479353bb 100644 --- a/tests/qapi-schema/alternate-clash.json +++ b/tests/qapi-schema/alternate-clash.json @@ -1,4 +1,3 @@ -# we detect C enum collisions in an anonymous union -{ 'union': 'Union1', - 'discriminator': {}, +# we detect C enum collisions in an alternate +{ 'alternate': 'Alt1', 'data': { 'one': 'str', 'ONE': 'int' } } diff --git a/tests/qapi-schema/alternate-conflict-dict.err b/tests/qapi-schema/alternate-conflict-dict.err index 2d4550c565..0f411f4faf 100644 --- a/tests/qapi-schema/alternate-conflict-dict.err +++ b/tests/qapi-schema/alternate-conflict-dict.err @@ -1 +1 @@ -tests/qapi-schema/alternate-conflict-dict.json:6: Anonymous union 'MyUnion' member 'two' can't be distinguished from member 'one' +tests/qapi-schema/alternate-conflict-dict.json:6: Alternate 'Alt' member 'two' can't be distinguished from member 'one' diff --git a/tests/qapi-schema/alternate-conflict-dict.json b/tests/qapi-schema/alternate-conflict-dict.json index ded302edae..fcb3e36cd9 100644 --- a/tests/qapi-schema/alternate-conflict-dict.json +++ b/tests/qapi-schema/alternate-conflict-dict.json @@ -1,9 +1,8 @@ -# we reject anonymous unions with multiple object branches +# we reject alternates with multiple object branches { 'type': 'One', 'data': { 'name': 'str' } } { 'type': 'Two', 'data': { 'value': 'int' } } -{ 'union': 'MyUnion', - 'discriminator': {}, +{ 'alternate': 'Alt', 'data': { 'one': 'One', 'two': 'Two' } } diff --git a/tests/qapi-schema/alternate-conflict-string.err b/tests/qapi-schema/alternate-conflict-string.err index 271ddcdec8..fc523b0879 100644 --- a/tests/qapi-schema/alternate-conflict-string.err +++ b/tests/qapi-schema/alternate-conflict-string.err @@ -1 +1 @@ -tests/qapi-schema/alternate-conflict-string.json:4: Anonymous union 'MyUnion' member 'two' can't be distinguished from member 'one' +tests/qapi-schema/alternate-conflict-string.json:4: Alternate 'Alt' member 'two' can't be distinguished from member 'one' diff --git a/tests/qapi-schema/alternate-conflict-string.json b/tests/qapi-schema/alternate-conflict-string.json index 3834a3ddfe..72f04a820a 100644 --- a/tests/qapi-schema/alternate-conflict-string.json +++ b/tests/qapi-schema/alternate-conflict-string.json @@ -1,7 +1,6 @@ -# we reject anonymous unions with multiple string-like branches +# we reject alternates with multiple string-like branches { 'enum': 'Enum', 'data': [ 'hello', 'world' ] } -{ 'union': 'MyUnion', - 'discriminator': {}, +{ 'alternate': 'Alt', 'data': { 'one': 'str', 'two': 'Enum' } } diff --git a/tests/qapi-schema/alternate-good.json b/tests/qapi-schema/alternate-good.json index 73d0993b39..99d614f138 100644 --- a/tests/qapi-schema/alternate-good.json +++ b/tests/qapi-schema/alternate-good.json @@ -1,10 +1,9 @@ -# Working example of anonymous union +# Working example of alternate { 'type': 'Data', 'data': { '*number': 'int', '*name': 'str' } } { 'enum': 'Enum', 'data': [ 'hello', 'world' ] } -{ 'union': 'MyUnion', - 'discriminator': {}, +{ 'alternate': 'Alt', 'data': { 'value': 'int', 'string': 'Enum', 'struct': 'Data' } } diff --git a/tests/qapi-schema/alternate-good.out b/tests/qapi-schema/alternate-good.out index b5117d1ab2..c3a6b7723c 100644 --- a/tests/qapi-schema/alternate-good.out +++ b/tests/qapi-schema/alternate-good.out @@ -1,6 +1,6 @@ [OrderedDict([('type', 'Data'), ('data', OrderedDict([('*number', 'int'), ('*name', 'str')]))]), OrderedDict([('enum', 'Enum'), ('data', ['hello', 'world'])]), - OrderedDict([('union', 'MyUnion'), ('discriminator', OrderedDict()), ('data', OrderedDict([('value', 'int'), ('string', 'Enum'), ('struct', 'Data')]))])] + OrderedDict([('alternate', 'Alt'), ('data', OrderedDict([('value', 'int'), ('string', 'Enum'), ('struct', 'Data')]))])] [{'enum_name': 'Enum', 'enum_values': ['hello', 'world']}, - {'enum_name': 'MyUnionKind', 'enum_values': None}] + {'enum_name': 'AltKind', 'enum_values': None}] [OrderedDict([('type', 'Data'), ('data', OrderedDict([('*number', 'int'), ('*name', 'str')]))])] diff --git a/tests/qapi-schema/alternate-nested.err b/tests/qapi-schema/alternate-nested.err index 59df96e132..00b05c601e 100644 --- a/tests/qapi-schema/alternate-nested.err +++ b/tests/qapi-schema/alternate-nested.err @@ -1 +1 @@ -tests/qapi-schema/alternate-nested.json:5: Anonymous union 'Union2' member 'nested' has invalid type 'Union1' +tests/qapi-schema/alternate-nested.json:4: Alternate 'Alt2' member 'nested' has invalid type 'Alt1' diff --git a/tests/qapi-schema/alternate-nested.json b/tests/qapi-schema/alternate-nested.json index ed2b6b7055..c4233b9f33 100644 --- a/tests/qapi-schema/alternate-nested.json +++ b/tests/qapi-schema/alternate-nested.json @@ -1,7 +1,5 @@ -# we reject a nested anonymous union branch -{ 'union': 'Union1', - 'discriminator': {}, +# we reject a nested alternate branch +{ 'alternate': 'Alt1', 'data': { 'name': 'str', 'value': 'int' } } -{ 'union': 'Union2', - 'discriminator': {}, - 'data': { 'nested': 'Union1' } } +{ 'alternate': 'Alt2', + 'data': { 'nested': 'Alt1' } } diff --git a/tests/qapi-schema/alternate-unknown.err b/tests/qapi-schema/alternate-unknown.err index bf8e9aef64..7af1b4c569 100644 --- a/tests/qapi-schema/alternate-unknown.err +++ b/tests/qapi-schema/alternate-unknown.err @@ -1 +1 @@ -tests/qapi-schema/alternate-unknown.json:2: Anonymous union 'Union' member 'unknown' has invalid type 'MissingType' +tests/qapi-schema/alternate-unknown.json:2: Alternate 'Alt' member 'unknown' has invalid type 'MissingType' diff --git a/tests/qapi-schema/alternate-unknown.json b/tests/qapi-schema/alternate-unknown.json index 0c305c282c..ad5c103028 100644 --- a/tests/qapi-schema/alternate-unknown.json +++ b/tests/qapi-schema/alternate-unknown.json @@ -1,4 +1,3 @@ -# we reject an anonymous union with unknown type in branch -{ 'union': 'Union', - 'discriminator': {}, +# we reject an alternate with unknown type in branch +{ 'alternate': 'Alt', 'data': { 'unknown': 'MissingType' } } diff --git a/tests/qapi-schema/flat-union-bad-discriminator.err b/tests/qapi-schema/flat-union-bad-discriminator.err index 1661c52b03..507e2bab4a 100644 --- a/tests/qapi-schema/flat-union-bad-discriminator.err +++ b/tests/qapi-schema/flat-union-bad-discriminator.err @@ -1 +1 @@ -tests/qapi-schema/flat-union-bad-discriminator.json:10: Flat union 'TestUnion' discriminator must be a string +tests/qapi-schema/flat-union-bad-discriminator.json:11: Flat union 'TestUnion' discriminator must be a string diff --git a/tests/qapi-schema/flat-union-bad-discriminator.json b/tests/qapi-schema/flat-union-bad-discriminator.json index 3ce43e8356..982f072555 100644 --- a/tests/qapi-schema/flat-union-bad-discriminator.json +++ b/tests/qapi-schema/flat-union-bad-discriminator.json @@ -1,4 +1,5 @@ # we require the discriminator to be a string naming a base-type member +# this tests the old syntax for anonymous unions before we added alternates { 'enum': 'TestEnum', 'data': [ 'value1', 'value2' ] } { 'type': 'TestBase', @@ -9,6 +10,6 @@ 'data': { 'integer': 'int' } } { 'union': 'TestUnion', 'base': 'TestBase', - 'discriminator': [], + 'discriminator': {}, 'data': { 'kind1': 'TestTypeA', 'kind2': 'TestTypeB' } } diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index e1d35e1248..dec8a7c108 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -53,8 +53,7 @@ 'discriminator': 'enum1', 'data': { 'value1' : 'UserDefC', 'value2' : 'UserDefB', 'value3' : 'UserDefA' } } -{ 'union': 'UserDefAlternate', - 'discriminator': {}, +{ 'alternate': 'UserDefAlternate', 'data': { 'uda': 'UserDefA', 's': 'str', 'i': 'int' } } # for testing native lists diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index b55ab8de10..313ecf3ad3 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -10,7 +10,7 @@ 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')]))]), - OrderedDict([('union', 'UserDefAlternate'), ('discriminator', OrderedDict()), ('data', OrderedDict([('uda', 'UserDefA'), ('s', 'str'), ('i', 'int')]))]), + OrderedDict([('alternate', 'UserDefAlternate'), ('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']), ('sizes', ['size'])]))]), OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]), OrderedDict([('command', 'user_def_cmd1'), ('data', OrderedDict([('ud1a', 'UserDefOne')]))]),