From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43444) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaV3a-0000OR-Tn for qemu-devel@nongnu.org; Tue, 24 Mar 2015 16:04:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YaV3Z-00043t-2v for qemu-devel@nongnu.org; Tue, 24 Mar 2015 16:04:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57868) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaV3Y-00043m-Qe for qemu-devel@nongnu.org; Tue, 24 Mar 2015 16:04:09 -0400 From: Eric Blake Date: Tue, 24 Mar 2015 14:03:35 -0600 Message-Id: <1427227433-5030-11-git-send-email-eblake@redhat.com> In-Reply-To: <1427227433-5030-1-git-send-email-eblake@redhat.com> References: <1427227433-5030-1-git-send-email-eblake@redhat.com> Subject: [Qemu-devel] [PATCH v5 10/28] qapi: Segregate anonymous unions into alternates in generator List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: kwolf@redhat.com, famz@redhat.com, armbru@redhat.com, wenchaoqemu@gmail.com, lcapitulino@redhat.com Special-casing 'discriminator == {}' for handling anonymous unions is getting awkward; since this particular type is not always a dictionary on the wire, it is easier to treat it as a completely different class of type, "alternate", so that if a type is listed in the union_types array, we know it is not an anonymous union. This patch just further segregates union handling, to make sure that anonymous unions are not stored in union_types, and splitting up check_union() into separate functions. A future patch will change the qapi grammar, and having the segregation already in place will make it easier to deal with the distinct meta-type. Signed-off-by: Eric Blake --- scripts/qapi-types.py | 6 ++-- scripts/qapi-visit.py | 4 +-- scripts/qapi.py | 94 +++++++++++++++++++++++++++++---------------------- 3 files changed, 58 insertions(+), 46 deletions(-) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 2390887..c9e0201 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -170,7 +170,7 @@ typedef enum %(name)s return lookup_decl + enum_decl -def generate_anon_union_qtypes(expr): +def generate_alternate_qtypes(expr): name = expr['union'] members = expr['data'] @@ -181,7 +181,7 @@ const int %(name)s_qtypes[QTYPE_MAX] = { name=name) for key in members: - qtype = find_anonymous_member_qtype(members[key]) + qtype = find_alternate_member_qtype(members[key]) assert qtype, "Invalid anonymous union member" ret += mcgen(''' @@ -408,7 +408,7 @@ for expr in exprs: fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys())) if expr.get('discriminator') == {}: - fdef.write(generate_anon_union_qtypes(expr)) + fdef.write(generate_alternate_qtypes(expr)) else: continue fdecl.write(ret) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 3f82bd4..77b0a1f 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -237,7 +237,7 @@ void visit_type_%(name)s(Visitor *m, %(name)s *obj, const char *name, Error **er ''', name=name) -def generate_visit_anon_union(name, members): +def generate_visit_alternate(name, members): ret = mcgen(''' void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **errp) @@ -302,7 +302,7 @@ def generate_visit_union(expr): if discriminator == {}: assert not base - return generate_visit_anon_union(name, members) + return generate_visit_alternate(name, members) enum_define = discriminator_find_enum_define(expr) if enum_define: diff --git a/scripts/qapi.py b/scripts/qapi.py index 39cc88b..17252e9 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -224,21 +224,16 @@ def find_base_fields(base): return None return base_struct_define['data'] -# Return the qtype of an anonymous union branch, or None on error. -def find_anonymous_member_qtype(qapi_type): +# Return the qtype of an alternate branch, or None on error. +def find_alternate_member_qtype(qapi_type): if builtin_types.has_key(qapi_type): return builtin_types[qapi_type] elif find_struct(qapi_type): return "QTYPE_QDICT" elif find_enum(qapi_type): return "QTYPE_QSTRING" - else: - union = find_union(qapi_type) - if union: - discriminator = union.get('discriminator') - if discriminator == {}: - return None - return "QTYPE_QDICT" + elif find_union(qapi_type): + return "QTYPE_QDICT" return None # Return the discriminator enum define if discriminator is specified as an @@ -276,22 +271,13 @@ def check_union(expr, expr_info): discriminator = expr.get('discriminator') members = expr['data'] values = { 'MAX': '(automatic)' } - types_seen = {} - # Three types of unions, determined by discriminator. + # Two types of unions, determined by discriminator. + assert discriminator != {} - # If the value of member 'discriminator' is {}, it's an - # anonymous union, and must not have a base. - if discriminator == {}: - enum_define = None - if base: - raise QAPIExprError(expr_info, - "Anonymous union '%s' must not have a base" - % name) - - # Else if the union object has no member 'discriminator', it's an + # If the union object has no member 'discriminator', it's an # ordinary union. For now, it must not have a base. - elif not discriminator: + if not discriminator: enum_define = None if base: raise QAPIExprError(expr_info, @@ -346,24 +332,46 @@ def check_union(expr, expr_info): % (name, key, values[c_key])) values[c_key] = key - # Ensure anonymous unions have no type conflicts. - if discriminator == {}: - if isinstance(value, list): - raise QAPIExprError(expr_info, - "Anonymous union '%s' member '%s' must " - "not be array type" % (name, key)) - qtype = find_anonymous_member_qtype(value) - if not qtype: - raise QAPIExprError(expr_info, - "Anonymous union '%s' member '%s' has " - "invalid type '%s'" % (name, key, value)) - if qtype in types_seen: - raise QAPIExprError(expr_info, - "Anonymous union '%s' member '%s' has " - "same QObject type as member '%s'" - % (name, key, types_seen[qtype])) - types_seen[qtype] = key +def check_alternate(expr, expr_info): + name = expr['union'] + base = expr.get('base') + discriminator = expr.get('discriminator') + members = expr['data'] + values = { 'MAX': '(automatic)' } + types_seen = {} + assert discriminator == {} + if base: + raise QAPIExprError(expr_info, + "Anonymous union '%s' must not have a base" + % name) + + # Check every branch + for (key, value) in members.items(): + # Check for conflicts in the generated enum + c_key = _generate_enum_string(key) + if c_key in values: + raise QAPIExprError(expr_info, + "Union '%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 " + "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 " + "invalid type '%s'" % (name, key, value)) + if qtype in types_seen: + raise QAPIExprError(expr_info, + "Anonymous union '%s' member '%s' has " + "same QObject type as member '%s'" + % (name, key, types_seen[qtype])) + types_seen[qtype] = key def check_enum(expr, expr_info): name = expr['enum'] @@ -393,7 +401,10 @@ def check_exprs(schema): if expr.has_key('enum'): check_enum(expr, info) elif expr.has_key('union'): - check_union(expr, info) + if expr.get('discriminator') == {}: + check_alternate(expr, info) + else: + check_union(expr, info) elif expr.has_key('event'): check_event(expr, info) @@ -535,7 +546,8 @@ def find_struct(name): def add_union(definition): global union_types - union_types.append(definition) + if definition.get('discriminator') != {}: + union_types.append(definition) def find_union(name): global union_types -- 2.1.0