From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45086) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZXqCW-00066f-Kn for qemu-devel@nongnu.org; Fri, 04 Sep 2015 08:34:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZXqCT-0005dX-BE for qemu-devel@nongnu.org; Fri, 04 Sep 2015 08:34:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41331) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZXqCT-0005cx-4D for qemu-devel@nongnu.org; Fri, 04 Sep 2015 08:34:37 -0400 References: <1441290623-13631-1-git-send-email-armbru@redhat.com> <1441290623-13631-3-git-send-email-armbru@redhat.com> From: Eric Blake Message-ID: <55E98FD2.1000503@redhat.com> Date: Fri, 4 Sep 2015 06:34:26 -0600 MIME-Version: 1.0 In-Reply-To: <1441290623-13631-3-git-send-email-armbru@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="EHmg0kiVucSU5iqGNelKv6ItCN33lfuAN" Subject: Re: [Qemu-devel] [PATCH RFC v4 02/32] qapi: New QAPISchema intermediate reperesentation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: mdroth@linux.vnet.ibm.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --EHmg0kiVucSU5iqGNelKv6ItCN33lfuAN Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/03/2015 08:29 AM, Markus Armbruster wrote: > The QAPI code generators work with a syntax tree (nested dictionaries) > plus a few symbol tables (also dictionaries) on the side. >=20 > + > +class QAPISchemaArrayType(QAPISchemaType): > + def __init__(self, name, info, element_type): > + QAPISchemaType.__init__(self, name, info) > + assert isinstance(element_type, str) > + self.element_type_name =3D element_type > + self.element_type =3D None Would it make sense to have: self._element_type_name =3D element_type self.element_type =3D None > + def check(self, schema): > + self.element_type =3D schema.lookup_type(self.element_type_nam= e) > + assert self.element_type > + and the corresponding self.element_type =3D schema.lookup_type(self._element_type_name) to make it obvious that ._element_type_name is for internal use only, while .element_type is the preferred member for queries/manipulation by qapi-*.py, which should only be using this object after check() has been run? > +class QAPISchemaObjectType(QAPISchemaType): > + def __init__(self, name, info, base, local_members, variants): > + QAPISchemaType.__init__(self, name, info) > + assert base =3D=3D None or isinstance(base, str) > + for m in local_members: > + assert isinstance(m, QAPISchemaObjectTypeMember) > + assert variants =3D=3D None \ > + or isinstance(variants, QAPISchemaObjectTypeVariants) > + self.base_name =3D base > + self.base =3D None > + self.local_members =3D local_members > + self.variants =3D variants > + self.members =3D None Similarly for self._base_name > + > +class QAPISchemaObjectTypeMember(object): > + def __init__(self, name, typ, optional): > + assert isinstance(name, str) > + assert isinstance(typ, str) > + assert isinstance(optional, bool) > + self.name =3D name > + self.type_name =3D typ > + self.type =3D None > + self.optional =3D optional and for self._type_name > +class QAPISchemaObjectTypeVariants(object): > + def __init__(self, tag_name, tag_enum, variants): > + assert tag_name =3D=3D None or isinstance(tag_name, str) > + assert tag_enum =3D=3D None or isinstance(tag_enum, str) > + for v in variants: > + assert isinstance(v, QAPISchemaObjectTypeVariant) > + self.tag_name =3D tag_name > + if tag_name: > + assert not tag_enum > + self.tag_member =3D None > + else: > + self.tag_member =3D QAPISchemaObjectTypeMember('kind', tag= _enum, > + False) and eventually for self._tag_name (except that we MUST expose self._tag_name until after we fix the C struct to use 'type' instead of 'kind'). > +class QAPISchemaCommand(QAPISchemaEntity): > + def __init__(self, name, info, arg_type, ret_type, gen, success_re= sponse): > + QAPISchemaEntity.__init__(self, name, info) > + assert not arg_type or isinstance(arg_type, str) > + assert not ret_type or isinstance(ret_type, str) > + self.arg_type_name =3D arg_type > + self.arg_type =3D None > + self.ret_type_name =3D ret_type > + self.ret_type =3D None > + self.gen =3D gen > + self.success_response =3D success_response and for self._art_type_name and self._ret_type_name > + > +class QAPISchemaEvent(QAPISchemaEntity): > + def __init__(self, name, info, arg_type): > + QAPISchemaEntity.__init__(self, name, info) > + assert not arg_type or isinstance(arg_type, str) > + self.arg_type_name =3D arg_type > + self.arg_type =3D None and for self._arg_type_name Basically, if I'm understanding the python conventions correctly, naming an instance member with leading underscore implies that it is for private use only; and if nothing else, the rename proves whether any of the other python files are relying on a leaky abstraction. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --EHmg0kiVucSU5iqGNelKv6ItCN33lfuAN 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/ iQEcBAEBCAAGBQJV6Y/SAAoJEKeha0olJ0Nq05IH/3z2p3fx/5Wc9YE18CmR9Kav iJ12EZ8qF3IYfAR/YWT7FAct8iuthhfn9I0rW4k7XdOiNbeHgJVq4aBCBnadFRAC 4y9MJA04aCjC6/xpJ1CDFVfZt31Y5bXjjbgplosw49GePwCo74lZITG+XwQb6KRP RtgQGkcq0rtzUyF2TA9J041pFoAzuW+u7I4ChNic7X8ErlndjQELX1EvWBYWhvh2 1jWBcyfWAWx/mhmQxM7NjtiZbt0UvnJ2+3IzrRfKbkntvDEXHcy9E2P5tYkI/crC mWpZsurp8v7IZ6Tjx/0bVXxpVsX3SD4e5sUrjShe8QIye9lALv5aae/e6pW258Q= =U9QQ -----END PGP SIGNATURE----- --EHmg0kiVucSU5iqGNelKv6ItCN33lfuAN--