From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56374) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zm0iw-0003YP-C4 for qemu-devel@nongnu.org; Tue, 13 Oct 2015 10:38:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zm0ir-0004o0-LO for qemu-devel@nongnu.org; Tue, 13 Oct 2015 10:38:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36433) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zm0ir-0004nw-DB for qemu-devel@nongnu.org; Tue, 13 Oct 2015 10:38:37 -0400 References: <1444710158-8723-1-git-send-email-eblake@redhat.com> <1444710158-8723-14-git-send-email-eblake@redhat.com> <87vbaa7vwl.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <561D1767.8040100@redhat.com> Date: Tue, 13 Oct 2015 08:38:31 -0600 MIME-Version: 1.0 In-Reply-To: <87vbaa7vwl.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="IdNEkdFNut9wGPe1eR8Dl2qvIWxmdSvwI" Subject: Re: [Qemu-devel] [PATCH v8 13/18] qapi: Track owner of each object member 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) --IdNEkdFNut9wGPe1eR8Dl2qvIWxmdSvwI Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/13/2015 07:14 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> Future commits will migrate semantic checking away from parsing >> and over to the various QAPISchema*.check() methods. But to >> report an error message about an incorrect semantic use of a >> member of an object type, it helps to know which type, command, >> or event owns the member. In particular, when a member is >> inherited from a base type, it is desirable to associate the >> member name with the base type (and not the type calling >> member.check()). >> >> >> + def describe(self): >> + owner =3D self.owner >> + # See QAPISchema._make_implicit_object_type() - reverse the >> + # mapping there to create a nice human-readable description >> + if owner.startswith(':obj-'): >> + owner =3D owner[5:] >> + if owner.endswith('-arg'): >> + source =3D '(argument of %s)' % owner[:4] >> + elif owner.endswith('-data'): >> + source =3D '(data of %s)' % owner[:5] >> + else: >> + assert owner.endswith('-wrapper') >> + source =3D '(branch of %s)' % owner[:8] >> + else: >> + source =3D '(%s of %s)' % (self.role, owner) >> + return "'%s' %s" % (self.name, source) >=20 > Perhaps not exactly pretty, but it gets the job done with minimal fuss.= > Sold. Ouch - I think these should be owner[:-4] (and similar) - I want the slice that excludes the last four bytes, and not one that is four bytes long. (It happened to work in my tests because I was testing on :obj-oops-arg, where owner[:4] and owner[:-4] gave the same 'oops' string= ). It's an easy fixup to squash in, so I'm not yet sure if I need a full v9 respin. >=20 >> + >> + role =3D 'member' >> + >> >=20 > Class variables are usually defined first, before methods. Sure, can do. pep8 didn't complain, and I didn't check if pylint warns about it. >=20 >> # TODO Drop this class once we no longer have the 'type'/'kind' misma= tch >> class QAPISchemaObjectTypeUnionTag(QAPISchemaObjectTypeMember): >> @@ -1034,6 +1061,11 @@ class QAPISchemaObjectTypeUnionTag(QAPISchemaOb= jectTypeMember): >> assert self.type.is_implicit() >> return 'kind' >> >> + def describe(self): >> + # Must override superclass describe() because self.name is 't= ype' >=20 > I don't find this argument convincing. I think 'kind' is exactly as > unhelpful as 'type' is. Specifically, should we report a QMP clash, > calling it 'kind' is confusing (it actually clashes with 'type'). > Conversely 'type' is confusing when we report a C clash. >=20 > The helpful part... >=20 >> + assert self.owner[0] !=3D ':' >> + return "'kind' (implicit tag of %s)" % self.owner >> + >=20 > ... is (implicit tag of FOO). >=20 > Fortunately, you're working towards killing the kind vs. type confusion= =2E > Suggest to either rephrase the comment, or simply drop it. Drop the comment is fine by me. I personally thought the message needed to report 'kind', because 16/18 ends up reporting: +tests/qapi-schema/union-clash-type.json:8: 'kind' (branch of TestUnion) collides with 'kind' (implicit tag of TestUnion) If I didn't override describe() at all, it would report: =2E..: 'kind' (branch of TestUnion) collides with 'type' (member of TestU= nion) If all I did was override 'role =3D "implicit tag"' instead of describe()= , it would report: =2E..: 'kind' (branch of TestUnion) collides with 'type' (implicit tag of= TestUnion) Of course when a later subset fixes the kind/type misnomer, the error message changes: =2E..: 'type' (branch of TestUnion) collides with 'type' (member of TestU= nion) where we lose the "implicit tag" because we completely lose the subclass.= Any preferences on which error message to prefer, and therefore how much (or how little) we need to override in this temporary subclass? Or is this an argument for making the subclass permanent (with a 'role =3D "implicit tag"') even after the kind/type rename? --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --IdNEkdFNut9wGPe1eR8Dl2qvIWxmdSvwI 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/ iQEcBAEBCAAGBQJWHRdnAAoJEKeha0olJ0NqlvQIAJRdiOc4HOkN6p97DWhdx8Dj H7VO69+cLQVRQufNZyJxCDgXtoI0nYQ7HlGq96WF5oVbmGeebbm+stLz+TwNFxXw iMarYaeyFb75YyOhZEx0c3KYHGXH2evQFgn3FIkSZA/3wi5GKADFDBk/eiZrIe5Z xPwVtq+tiHrAhgDxf42ImPN1LniyDly66iX0rBchbJy0AyDdhIJlxqq2baPTHbAY tososAmCOwN13tC6x69udI62IOsYa0z33gXUYrjC7BkxillKBZ1Y9pBmUSlJs1i1 FqhOcVmkZUR4CfXwJQM0VXRdnWqPIbZOh0FbcnxZz4iobWJELUhYTEakmQDZTsk= =yHqS -----END PGP SIGNATURE----- --IdNEkdFNut9wGPe1eR8Dl2qvIWxmdSvwI--