From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52301) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZywXM-0007Rc-HV for qemu-devel@nongnu.org; Wed, 18 Nov 2015 01:48:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZywXI-0001og-Eq for qemu-devel@nongnu.org; Wed, 18 Nov 2015 01:48:12 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48751) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZywXI-0001oc-7V for qemu-devel@nongnu.org; Wed, 18 Nov 2015 01:48:08 -0500 References: <1447224690-9743-1-git-send-email-eblake@redhat.com> <1447224690-9743-28-git-send-email-eblake@redhat.com> <87oaezi5ks.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <564C1F26.5010001@redhat.com> Date: Tue, 17 Nov 2015 23:48:06 -0700 MIME-Version: 1.0 In-Reply-To: <87oaezi5ks.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="337poDxElWhVbR7TBeEFFlAbQbMqOXcGq" Subject: Re: [Qemu-devel] [PATCH v11 27/28] qapi: Move duplicate enum value checks to schema check() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, Michael Roth This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --337poDxElWhVbR7TBeEFFlAbQbMqOXcGq Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/12/2015 08:46 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> Similar to the previous commit, move the detection of a collision >> in enum values from parse time to QAPISchemaEnumType.check(). >> This happens to also detect collisions in union branch names >> mapping to the same enum value, even when the names do not >> collide case-wise. So for a decent error message, we have to >> determine if the enum is implicit (and if so where the real >> collision lies). >> >> + def _describe(self, schema): >> + # If the enum is implicit, report the error on behalf of >> + # the union or alternate that triggered the enum >> + if self.is_implicit(): >> + owner =3D schema.lookup_type(self.name[:-4]) >> + assert owner >> + if isinstance(owner, QAPISchemaAlternateType): >> + return "Alternate '%s' branch" % owner.name >=20 > Didn't we just drop this kind of implicit enum? >=20 >> + else: >> + return "Union '%s' branch" % owner.name >> + else: >> + return "Enum '%s' value" % self.name >=20 > I like to call it "member" rather than value, because it avoids > confusion with the numeric value of the C enumeration constant generate= d > for it. >=20 > The conditional isn't exactly elegant, but it would do. I'm not 100% > convinced we need it, though. self.info already points to whatever > defined this enum, either an explicit enum definition, or a simple unio= n > definition. How do the error messages come out if we dumb down to > "Member '%s'"? >=20 > A method with a similar purpose exists in QAPISchemaObjectTypeMember, > but it's spelled describe(). It's used only from within the class. > Rename it to match this one? [as much notes to myself as anything else...] We chatted some on IRC, and had some more ideas, thus I will delay patches 26-28 to a later date while posting the rest of v12. Among those ideas - getting rid of the enum _MAX clash will simplify things (to be done in v12) - create a new QAPISchemaMember class as the superclass of QAPISchemaObjectTypeMember, and have enum members be instances of this type rather than a straight string (at which point the describe() method and collision checking lives in the superclass, while the associated type only lives in the subclass). Less code duplication that way - the idea of multiline error messages, only when info !=3D member.owner.info; as in: foo.json:4: Member 'one' clashes with 'One' foo.json:2: Member 'one' defined here --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --337poDxElWhVbR7TBeEFFlAbQbMqOXcGq 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/ iQEcBAEBCAAGBQJWTB8mAAoJEKeha0olJ0NqLxoH/ikbAKfapyYgAjTTP37XTc4J F6egWhdA0GRkck6B6Cem+/uHw/VY/ia92K5Rr5nmzYY0nIBDoJS0NOwRAoMaf1Oy i70W/x7LhD3UtLXWJVxTdgPPV8Y8v/e9RBgkWAgRvaVZJ221GMyYhMKTPtrES2HN mgv7EBbJeEOF3yEGKrOPzGsolkgHwBWJ80VMD4uaLOkBl99dMsTuEMdARQ2WV5WD Lwaje8ZfBZSq/pqEoPJ7vJiNUBtfuIlPgcE15Rm7IFb0n5EioiLD6dlBvepaRsCW /Mizx3bCGb2wdxeqRrFM2b+Wco44edNLm/3NCxtNcBGmOcS5ohSbnqKUrijch3g= =Q3Ba -----END PGP SIGNATURE----- --337poDxElWhVbR7TBeEFFlAbQbMqOXcGq--