From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37539) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZlqvP-0007ki-8x for qemu-devel@nongnu.org; Tue, 13 Oct 2015 00:10:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZlqvI-0000jF-BE for qemu-devel@nongnu.org; Tue, 13 Oct 2015 00:10:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:35643) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZlqvI-0000jB-3e for qemu-devel@nongnu.org; Tue, 13 Oct 2015 00:10:48 -0400 References: <1443930073-19359-1-git-send-email-eblake@redhat.com> <1443930073-19359-12-git-send-email-eblake@redhat.com> <8737xgqe19.fsf@blackfin.pond.sub.org> <561BDE44.4000407@redhat.com> From: Eric Blake Message-ID: <561C8446.1050104@redhat.com> Date: Mon, 12 Oct 2015 22:10:46 -0600 MIME-Version: 1.0 In-Reply-To: <561BDE44.4000407@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="v0jTg55pFCFMQquoEVnlafJaTx0al0l6b" Subject: Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Michael Roth , marcandre.lureau@redhat.com, qemu-devel@nongnu.org, ehabkost@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --v0jTg55pFCFMQquoEVnlafJaTx0al0l6b Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/12/2015 10:22 AM, Eric Blake wrote: >=20 >>> + def check(self, schema, info, tag_type, seen, flat): >>> QAPISchemaObjectTypeMember.check(self, schema, info, [], see= n) seen gets modified here... >>> assert self.name in tag_type.values >>> + if flat: >>> + # For flat unions, check that each QMP member does not >>> + # collide with any non-variant members. No type can >>> + # contain itself as a flat variant >>> + self.type.check(schema) >>> + assert not self.type.variants # not implemented >>> + for m in self.type.members: >>> + if m.c_name() in seen: =2E..and it is the modified seen here that causes... >> union-clash-data.json:6: Member 'data' of branch 'data' collides with= 'data' (branch of TestUnion) =2E..the incorrect error. >> >> The FIXME in union-clash-data.json actually asks for an error, because= >> 'data' clashes with our stupid filler to avoid empty unions. But that= 's >> not what this error message reports! I think what happens is this: >> QAPISchemaObjectTypeVariant.check() adds the case name self.name (here= : >> 'data') to seen. When we then check the case's 'data': 'int' member, = it >> finds the with the case name, and reports a clash. I can't see why it= >> should. >> >> This clashing business is awfully confusing, because we have to consid= er >> (flat unions, simple unions, alternates) times (QMP, C). >> >> It would be simpler if we could make C clash only when QMP clashes. >=20 > If a QMP clash always caused a C clash, life would be a bit simpler. We= > aren't going to get there (because in a flat union, hiding the members > of the branch type behind a single pointer means those members don't > clash with any C names of the overall union), but I can certainly try t= o > make the comments explain what is going on. I've managed to fix it with a dict(seen) and some comments, and will post a v8. Great catch, by the way. >=20 >> >> We might want to separate out alternates. Dunno. >=20 > And to some extent, subset C already does some of that separation > (because I try to convert from 'FooKind type' to 'qtype_code type' > without even creating an implicit 'FooKind' enum). It sounds like > you're okay with any well-documented TODO warts related to alternates > for the purposes of this subset B, especially if I can then clean those= > warts back up in subset C. But what v8 of subset B needs to avoid is > any warts based on simple vs. flat unions. I can live with that. >=20 > I'm still trying to figure out if hoisting the kind=3D>type rename into= > subset B makes life easier or harder (it certainly causes me more rebas= e > churn, but that's not necessarily a bad thing). At this point, it was easier to live with a temporary hack for QAPISchemaObjectTypeUnionTag than it was to rebase my kind=3D>type rename= patch to be this early in the overall series. We'll get there, but I agree with the approach we've been taking of one subset at a time. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --v0jTg55pFCFMQquoEVnlafJaTx0al0l6b Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJWHIRGAAoJEKeha0olJ0NqtTYH/0LpF1h2JdgpgNaiqDpBqNyD UGoaehq0H1u3maFPFGGGotAIVzIGsm4zQE15onuQdLHKkBTEUnWai2MibaQnlyfH guIAYhRhwNajX/8IQyX3iadXQUST5/GZaJ3+DC/L3DN++FXbHb/GnZjBPUK3CtjI AvQXujYjpHgqpZOBqQsDLJcH1g1cbFeWmTKVQBUXfqK2aITEB1nGM0z+cQeqBQX4 GMtGURwMMpzk2lepkse0UpS67qRAE1zPRlcIoVfba0K+EoBEh4wz2d+0MjyEZ3Je DbOcPNg4fXD0tVgKVwXThbhURP5cUcnnlkOoJ0LhVEr/CnP0riwGfy3c4e+p9og= =20p0 -----END PGP SIGNATURE----- --v0jTg55pFCFMQquoEVnlafJaTx0al0l6b--