From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37318) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw1XS-0004XL-JA for qemu-devel@nongnu.org; Tue, 10 Nov 2015 00:32:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zw1XO-0000Rb-H4 for qemu-devel@nongnu.org; Tue, 10 Nov 2015 00:32:14 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44352) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw1XO-0000RE-9k for qemu-devel@nongnu.org; Tue, 10 Nov 2015 00:32:10 -0500 References: <1446791754-23823-1-git-send-email-eblake@redhat.com> <1446791754-23823-25-git-send-email-eblake@redhat.com> <87r3jzdy8p.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <56418158.9000403@redhat.com> Date: Mon, 9 Nov 2015 22:32:08 -0700 MIME-Version: 1.0 In-Reply-To: <87r3jzdy8p.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="W9Uor27QlIksXSk9m0TJrLTadX9EISsa7" 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) --W9Uor27QlIksXSk9m0TJrLTadX9EISsa7 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/09/2015 07:49 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) >> - 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) >> for m in self.local_members: >> m.check(schema) >> m.check_clash(seen) >> @@ -994,6 +990,12 @@ class QAPISchemaObjectType(QAPISchemaType): >> assert self.variants.tag_member in self.members >> self.variants.check_clash(schema, seen) >> >> + def check_clash(self, schema, seen): >> + self.check(schema) >=20 > Do we want to hide this .check() inside .check_clash()? >=20 > QAPISchemaObjectTypeMember.check() doesn't. I think the two better > behave the same. >=20 >> + assert not self.variants # not implemented >> + for m in self.members: >> + m.check_clash(seen) The self.check(schema) call is necessary to get self.members populated. We cannot iterate over self.members if the type has not had check() called; this is true for both callers of type.check_clash() (ObjectType.check(), and Variants.check_clash()). You are correct that neither Member.check() nor Member.check_clash() call a form of type.check() - but that's because at that level, there is no need to populate a type.members list. On the other hand, we've been arguing that check() should populate everything after construction prior to anything else being run; and not running Variant.type.check() during Variants.check() of flat unions feels like we may have a hole (a flat union will have to inline its types to the overall JSON object, and inlining types requires access to type.members - but as written, we aren't populating them until Variants.check_clash()). I can play with hoisting the type.check() out of type.check_clash() and instead keep base.check() in type.check(), and add variant.type.check() in Variants.check() (but only for unions, not for alternates), if you are interested. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --W9Uor27QlIksXSk9m0TJrLTadX9EISsa7 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/ iQEcBAEBCAAGBQJWQYFZAAoJEKeha0olJ0NqymUH/2E9xBU5cnYx9MimgOoxpjQS 2cyOb/qgC58P1N/I7liFXntQbWn3xHjxd/IsVR8/8ggvjqt/sz5qud2KGbRonx3l Fny0xYMfmx+bhqC0pWBKhdwvYZ28Y3Sc5a69rQk/rKUzu+9dXD56cHflQWRRWH0L vsSm/YdY9sf4JtoLmzYy7qXtqNePE+05+PWMkbPaaL2VC7Q05Nss2MoG3iacand1 kHgoHU9eoNQDeeOmv8Km2qt6Scd08L3lpmHzh0cojNLevYf3KnilGnFnU6UDsiUw jVG7oUmJmbFO3ecbeE9XpVbdKieZiB1G35WcSMnO6GBOMIQYD9bsHiPQcsO9SQ4= =f3w3 -----END PGP SIGNATURE----- --W9Uor27QlIksXSk9m0TJrLTadX9EISsa7--