From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33878) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zhzqx-0007b1-JO for qemu-devel@nongnu.org; Fri, 02 Oct 2015 08:54:24 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zhzqu-0004CL-AQ for qemu-devel@nongnu.org; Fri, 02 Oct 2015 08:54:23 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56358) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zhzqu-0004Bu-3B for qemu-devel@nongnu.org; Fri, 02 Oct 2015 08:54:20 -0400 References: <1443760312-656-1-git-send-email-eblake@redhat.com> <1443760312-656-3-git-send-email-eblake@redhat.com> <87eghdhhxy.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <560E7E74.5090600@redhat.com> Date: Fri, 2 Oct 2015 06:54:12 -0600 MIME-Version: 1.0 In-Reply-To: <87eghdhhxy.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ucdSl0jThQ0qVMBhniiXNWVFE2IXhq3LV" Subject: Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: marcandre.lureau@redhat.com, qemu-devel@nongnu.org, ehabkost@redhat.com, Michael Roth This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --ucdSl0jThQ0qVMBhniiXNWVFE2IXhq3LV Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/02/2015 01:02 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> A future patch will enable error reporting from the various >> QAPISchema*.check() methods. But to report an error related >> to an implicit type, we'll need to associate a location with >> the type (the same location as the top-level entity that is >> causing the creation of the implicit type), and once we do >> that, keying off of whether foo.info exists is no longer a >> viable way to determine if foo is an implicit type. >> >> Instead, add an is_implicit() method to QAPISchemaObjectType, >> and use that function where needed. (Done at the ObjectType >> level, since we already know all builtins and arrays are >> implicit, no commands or events are implicit, and we don't >> have any differences in generated code for regular vs. >> implicit enums.) >> +++ b/scripts/qapi-types.py >> @@ -234,7 +234,8 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):= >> self._btin =3D None >> >> def visit_predicate(self, entity): >> - return not isinstance(entity, QAPISchemaObjectType) or entity= =2Einfo >> + return not (isinstance(entity, QAPISchemaObjectType) and >> + entity.is_implicit()) >=20 > Aha, now the left hand side becomes necessary to guard the > .is_implicit(). It stays superfluous if you make .is_implicit() a > QAPISchemaEntity method. See discussion on 1/12 - actually, it _becomes_ superfluous in this patch once we make it a QAPISchemaEntity method. >> +++ b/scripts/qapi.py >> @@ -970,12 +970,15 @@ class QAPISchemaObjectType(QAPISchemaType): >> self.variants.check(schema, members, seen) >> self.members =3D members >> >> + def is_implicit(self): >> + return self.name[0] =3D=3D ':' Actually, this only works for implicit objects. Implicit enums instead have self.name[-4:] =3D=3D 'Kind'. But qapi-types cares about implicit objects only. So if I hoist this, I may need something like: def is_implicit(self, type=3DNone): if type and not isinstance(self, type): return Fals if isinstance(self, QAPISchemaObjectType): return self.name[0] =3D=3D ':' if isinstance(self, QAPISchemaEnumType): return self.name[-4:] =3D=3D 'Kind' return False where qapi-types would call entity.is_implicit(QAPISchemaObjectType). >> + >=20 > If this test is here to stay, perhaps add a comment pointing to > _make_implicit_object_type(). Indeed. >> @@ -1043,7 +1046,8 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObje= ctTypeMember): >> # This function exists to support ugly simple union special cases= >> # TODO get rid of them, and drop the function >> def simple_union_type(self): >> - if isinstance(self.type, QAPISchemaObjectType) and not self.t= ype.info: >> + if isinstance(self.type, >> + QAPISchemaObjectType) and self.type.is_implicit= (): >=20 > I figure you break this line in the argument list to avoid a backslash > for line continuation. I know PEP8 doesn't like them, but I like line > breaks away from top level operators even less. I feel it should be > broken after the and operator, even though that'll require wrapping the= > condition in parenthesis. Sadly, this form causes pep8 to complain about continuation at the same indentation as the body: if (cond1 and cond2): body But it seems that pep8 and pylint don't mind the backslash in: if cond1 and \ cond2: body where the indentation is okay. I guess trying to avoid the \ is not worth it, if the tools don't complain about it, and that this was a case of me prematurely guessing (incorrectly) about what the tools don't like.= --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --ucdSl0jThQ0qVMBhniiXNWVFE2IXhq3LV 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/ iQEcBAEBCAAGBQJWDn50AAoJEKeha0olJ0NqedoH/AoHZJakRWzH+77hk680yMxH ZfJQ9gThjx3EYXoPTP4IiV3wridBy0daE4KvN+1Yd3gKldLTN5CsCK47uRrQLTWJ frSTCrbeKeFKoQzFyepym3cSQ1yFeUjT0aBapCNrr+fJ9xB2TrnWGnORGBdKIhKD Wsyk7ZmHK5R+IHok6IH0Lcc+dlgTRvLMW0PAfQqmFD58aHXnM171WuXsxW8OefVz w0FLPcISbkxmv+WfIhSAYTjVqn/goYBTY48A7th08kO81ub+lBxMhxGFP1InpWxO xj3LSwBYJKaFA0JYNE2dX9UyckEWiPnQ7zMILfsAHIMT9afAWSNFTpKFuM3579Y= =Tid3 -----END PGP SIGNATURE----- --ucdSl0jThQ0qVMBhniiXNWVFE2IXhq3LV--