From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33513) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwJ6k-0002Lp-E9 for qemu-devel@nongnu.org; Tue, 10 Nov 2015 19:17:51 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZwJ6h-00015U-3O for qemu-devel@nongnu.org; Tue, 10 Nov 2015 19:17:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50359) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwJ6g-00015K-Rc for qemu-devel@nongnu.org; Tue, 10 Nov 2015 19:17:47 -0500 References: <1446791754-23823-1-git-send-email-eblake@redhat.com> <1446791754-23823-28-git-send-email-eblake@redhat.com> <87d1vjgsg2.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <56428924.3010209@redhat.com> Date: Tue, 10 Nov 2015 17:17:40 -0700 MIME-Version: 1.0 In-Reply-To: <87d1vjgsg2.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="4FMVTrWGPF4ljPfO4Xv4fNVCk24cjt8c4" Subject: Re: [Qemu-devel] [PATCH v10 27/30] 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) --4FMVTrWGPF4ljPfO4Xv4fNVCk24cjt8c4 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/09/2015 07:26 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 _pretty_owner(self): >> + # See QAPISchema._make_implicit_object_type() - reverse the >> + # mapping there to create a nice human-readable description >=20 > This comment confused me briefly. It applies to the following > conditionals if-part, but not its else-part. Move it into the if-part?= Done. >> @@ -1082,9 +1117,11 @@ class QAPISchemaAlternateType(QAPISchemaType): >> QAPISchemaType.__init__(self, name, info) >> assert isinstance(variants, QAPISchemaObjectTypeVariants) >> assert not variants.tag_name >> + variants.set_owner(name) >> self.variants =3D variants >> >> def check(self, schema): >> + self.variants.tag_member.set_owner(self.name) >> self.variants.tag_member.check(schema) >> self.variants.check(schema, {}) >> >=20 > Odd: all other .set_owner() calls are in .__init__() or .set_owner(). > Can we move this one to __init__() for consistency? Yes, done. >=20 > I think we can: __init__() requires its variants argument to have a > tag_member (it even asserts not variants.tag_name). >=20 >> @@ -1212,6 +1249,7 @@ class QAPISchema(object): >> def _make_implicit_object_type(self, name, info, role, members): >> if not members: >> return None >> + # See also QAPISchemaObjectTypeMember.describe() >=20 > Should this point to ._pretty_owner() instead? Good call. >=20 >> name =3D ':obj-%s-%s' % (name, role) >> if not self.lookup_entity(name, QAPISchemaObjectType): >> self._def_entity(QAPISchemaObjectType(name, info, None, >> @@ -1315,7 +1353,7 @@ class QAPISchema(object): >> data =3D expr.get('data') >> if isinstance(data, OrderedDict): >> data =3D self._make_implicit_object_type( >> - name, info, 'arg', self._make_members(data, info)) >> + name, info, 'data', self._make_members(data, info)) >=20 > This is necessary only to make .pretty_owner() say 'data of EVENT_NAME'= > instead of 'argument of EVENT_NAME'. Do we really need different > wording for commands and events? >=20 > I'd make it say 'parameter of' for both commands and events. I > habitually use "parameter" for formal parameters, and "argument" for > actual arguments. Guess I've worked with the C standard too much... 'parameter' sounds better for both, at which point the messages no longer differ,... >=20 > For what it's worth, the QAPI schema language uses 'data' for both (and= > many other things, too), and the introspection schema uses 'arg-type' > for both. >=20 >> self._def_entity(QAPISchemaEvent(name, info, data)) >> >> def _def_exprs(self): >> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schem= a/qapi-schema-test.out >> index 786024e..f78ef04 100644 >> --- a/tests/qapi-schema/qapi-schema-test.out >> +++ b/tests/qapi-schema/qapi-schema-test.out >> @@ -1,9 +1,9 @@ >> object :empty >> -object :obj-EVENT_C-arg >> +object :obj-EVENT_C-data >> member a: int optional=3DTrue >> member b: UserDefOne optional=3DTrue >> member c: str optional=3DFalse >> -object :obj-EVENT_D-arg >> +object :obj-EVENT_D-data =2E..so I no longer need to rename the implicit struct for events to allo= w the differentiation. v11 is thus a smaller patch :) --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --4FMVTrWGPF4ljPfO4Xv4fNVCk24cjt8c4 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/ iQEcBAEBCAAGBQJWQokkAAoJEKeha0olJ0NqVDEIAKWsnQ6RKd31jDH5PbakXdyp eLhwZLtALxCh8RfM36JNg+kaUqvRVWsjfx2xK5hsM/Vkuj0mBIDEz3zbAzBelZU6 XH/bm3SCZG/jzrJ9E0r4Ccgq4Rl8Tt5cj35U0zC7NQFoPJtnBQ5Cr6MWJAGhjsz8 PcmjrUI4Qh72ywAnyMHW8oJ6ydBnG/0XnzUkYEod94RnRrylawAowhxmgYcwlqdg SJ9/kiYo60B0e6d8u9we0jQNuSvGLD90hCzR417EnqYPih/9sflnohqA94CU+iGH 81jvmFnZHdX/ujMLf1/XAILh8YOY4cs6rztvnLwOrpz+VXvUAupLr13eqCK5faE= =kCbW -----END PGP SIGNATURE----- --4FMVTrWGPF4ljPfO4Xv4fNVCk24cjt8c4--