From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38833) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZvqMv-00005X-QV for qemu-devel@nongnu.org; Mon, 09 Nov 2015 12:36:39 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZvqMt-0006dn-42 for qemu-devel@nongnu.org; Mon, 09 Nov 2015 12:36:37 -0500 Received: from mx1.redhat.com ([209.132.183.28]:32918) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZvqMs-0006di-Ry for qemu-devel@nongnu.org; Mon, 09 Nov 2015 12:36:35 -0500 References: <1446791754-23823-1-git-send-email-eblake@redhat.com> <1446791754-23823-25-git-send-email-eblake@redhat.com> <87r3jzgwf3.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <5640D99D.1020006@redhat.com> Date: Mon, 9 Nov 2015 10:36:29 -0700 MIME-Version: 1.0 In-Reply-To: <87r3jzgwf3.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="JaBsEGuWHakXj8aMtTiEtAfm1u3ndKmpm" Subject: Re: [Qemu-devel] [PATCH v10 24/30] qapi: Factor out QAPISchemaObjectType.check_clash() 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) --JaBsEGuWHakXj8aMtTiEtAfm1u3ndKmpm Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/09/2015 06:00 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> Consolidate two common sequences of clash detection into a >> new QAPISchemaObjectType.check_clash() helper method. >> >> No change to generated code. >> >> Signed-off-by: Eric Blake >> >> --- >> @@ -980,11 +980,7 @@ class QAPISchemaObjectType(QAPISchemaType): >> seen =3D OrderedDict() >> if self._base_name: >> self.base =3D schema.lookup_type(self._base_name) >> - assert isinstance(self.base, QAPISchemaObjectType) >=20 > This assertion is lost. >=20 >> - assert not self.base.variants # not implemented >> - self.base.check(schema) >> - for m in self.base.members: >> - m.check_clash(seen) >> + self.base.check_clash(schema, seen) Directly lost, but indirectly still present. The new code is calling QAPISchemaObjectType.check_clash(), which won't exist unless self.base is a QAPISchemaObjectType. Folding the assert into the refactored function makes no sense (the condition isinstance(self, QAPISchemaObjectType) would always be true), and leaving the assert prior to calling self.base.check_clash() adds no real protection against programming bugs. >> @@ -1062,12 +1064,7 @@ class QAPISchemaObjectTypeVariants(object): >> for v in self.variants: >> # Reset seen map for each variant, since qapi names from = one >> # branch do not affect another branch >> - vseen =3D dict(seen) >> - assert isinstance(v.type, QAPISchemaObjectType) >=20 > This assertion is lost. >=20 >> - assert not v.type.variants # not implemented >> - v.type.check(schema) >> - for m in v.type.members: >> - m.check_clash(vseen) >> + v.type.check_clash(schema, dict(seen)) Same explanation. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --JaBsEGuWHakXj8aMtTiEtAfm1u3ndKmpm 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/ iQEcBAEBCAAGBQJWQNmeAAoJEKeha0olJ0NqE2MH/2A1lmXFEte9MAbTI3gfD1LU /zXfu9ObPFc5k1dbLwjbbe5SKk4owgJ5Uo6IK3mhpauEAJ7GRRkGd/4Q43nqWuBB jkqo/gnVrvyWezgKkz3e8WgPA9uHEjXZJRaiEbkHeM9e1hLze7YAgl3ymLK0J8ju m+daIZ7h8iEPmM1ZkX75TiOJPX9dB5HzvCPSJVjvXfdLTu9DbrdOqMDwaOem7K/d ACgRHI7wR8nw6SC8ygrFd822OkZZ2d3fV1OJMwG5B/Vd28m8qqoRdnqAOP/K8oqj 3EgMaT2wIsIgKWtVPa3ksZZ7m1S4wbKjrgKuGYFXuxkJYN7QLE73RixKGoMzVFc= =CshC -----END PGP SIGNATURE----- --JaBsEGuWHakXj8aMtTiEtAfm1u3ndKmpm--