From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43767) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwuCh-0001Rw-PW for qemu-devel@nongnu.org; Thu, 12 Nov 2015 10:54:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZwuCe-00042l-D0 for qemu-devel@nongnu.org; Thu, 12 Nov 2015 10:54:27 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35562) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwuCe-00042e-6E for qemu-devel@nongnu.org; Thu, 12 Nov 2015 10:54:24 -0500 From: Markus Armbruster References: <1447224690-9743-1-git-send-email-eblake@redhat.com> <1447224690-9743-23-git-send-email-eblake@redhat.com> Date: Thu, 12 Nov 2015 16:54:21 +0100 In-Reply-To: <1447224690-9743-23-git-send-email-eblake@redhat.com> (Eric Blake's message of "Tue, 10 Nov 2015 23:51:24 -0700") Message-ID: <87d1vfi576.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v11 22/28] qapi: Simplify visiting of alternate types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, Michael Roth Eric Blake writes: > Previously, working with alternates required two lookup arrays > and some indirection: for type Foo, we created Foo_qtypes[] > which maps each qtype to a value of the generated FooKind enum, > then look up that value in FooKind_lookup[] like we do for other > union types. [...] > diff --git a/scripts/qapi.py b/scripts/qapi.py > index d4ef08e..1790e8f 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -627,15 +627,15 @@ def check_union(expr, expr_info): > def check_alternate(expr, expr_info): > name = expr['alternate'] > members = expr['data'] > - values = {'MAX': '(automatic)'} > + values = {} > types_seen = {} > > # Check every branch > for (key, value) in members.items(): > check_name(expr_info, "Member of alternate '%s'" % name, key) > > - # Check for conflicts in the generated enum > - c_key = camel_to_upper(key) > + # Check for conflicts in the branch names > + c_key = c_name(key) > if c_key in values: > raise QAPIExprError(expr_info, > "Alternate '%s' member '%s' clashes with '%s'" > @@ -1090,8 +1090,11 @@ class QAPISchemaObjectTypeVariants(object): > assert isinstance(self.tag_member.type, QAPISchemaEnumType) > for v in self.variants: > v.check(schema) > - assert v.name in self.tag_member.type.values > - if isinstance(v.type, QAPISchemaObjectType): > + # Union names must match enum values; alternate names are > + # checked separately. Use 'seen' to tell the two apart. > + if seen: > + assert v.name in self.tag_member.type.values > + assert isinstance(v.type, QAPISchemaObjectType) > v.type.check(schema) > > def check_clash(self, schema, info, seen): > @@ -1133,6 +1136,11 @@ class QAPISchemaAlternateType(QAPISchemaType): > # Not calling self.variants.check_clash(), because there's nothing > # to clash with > self.variants.check(schema, {}) > + # Alternate branch names have no relation to the tag enum values; > + # so we have to check for potential name collisions ourselves. > + cases = {} > + for var in self.variants.variants: > + var.check_clash(self.info, cases) Nitpick: elsewhere in this file, we use "for v" to iterate over variants. > > def json_type(self): > return 'value' > @@ -1340,7 +1348,7 @@ class QAPISchema(object): > data = expr['data'] > variants = [self._make_variant(key, value) > for (key, value) in data.iteritems()] > - tag_member = self._make_implicit_tag(name, info, variants) > + tag_member = QAPISchemaObjectTypeMember('type', 'QTypeCode', False) > self._def_entity( > QAPISchemaAlternateType(name, info, > QAPISchemaObjectTypeVariants(None, [...]