From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34663) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw1IU-00056F-KP for qemu-devel@nongnu.org; Tue, 10 Nov 2015 00:16:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zw1IP-0005yP-Ke for qemu-devel@nongnu.org; Tue, 10 Nov 2015 00:16:46 -0500 Received: from mx1.redhat.com ([209.132.183.28]:46894) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zw1IP-0005yK-CY for qemu-devel@nongnu.org; Tue, 10 Nov 2015 00:16:41 -0500 References: <1446791754-23823-1-git-send-email-eblake@redhat.com> <1446791754-23823-24-git-send-email-eblake@redhat.com> <87vb9bgwlj.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <56417DB2.4090601@redhat.com> Date: Mon, 9 Nov 2015 22:16:34 -0700 MIME-Version: 1.0 In-Reply-To: <87vb9bgwlj.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="2D2PviF6QN2r1CIi11tm7H3K04HLMqqtB" Subject: Re: [Qemu-devel] [PATCH v10 23/30] qapi: Check for qapi collisions of flat union branches 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) --2D2PviF6QN2r1CIi11tm7H3K04HLMqqtB Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 11/09/2015 05:56 AM, Markus Armbruster wrote: > Eric Blake writes: >=20 >> Right now, our ad hoc parser ensures that we cannot have a >> flat union that introduces any qapi member names that would >> conflict with the non-variant qapi members already present >> from the union's base type (see flat-union-clash-member.json). >> We want QAPISchemaObjectType.check() to make the same check, >> so we can later reduce some of the ad hoc checks. >> >> In general, a type used as a branch of a flat union cannot >> also be the base type of the flat union, so even though we are >> adding a call to variant.type.check() in order to populate >> variant.type.members, this is merely a case of gaining >> topological sorting of how types are visited (and type.check() >> is already set up to allow multiple calls due to base types). >=20 > Yes, a type cannot contain itself, neither as base nor as variant. >=20 > We have tests covering attempts to do the former > (struct-cycle-direct.json, struct-cycle-indirect.json). As far as I ca= n > see, we don't have tests covering the latter. Do we catch it? Yes, at least by virtue of the ad hoc tests: attempting to reuse a base type of the flat union as a variant member will cause the qapi members of the base type to appear more than once in the JSON object (that is, the checks that reject flat-union-clash-member.json would also reject this scenario). To test: diff --git i/tests/qapi-schema/qapi-schema-test.json w/tests/qapi-schema/qapi-schema-test.json index 44638da..16b2ffb 100644 --- i/tests/qapi-schema/qapi-schema-test.json +++ w/tests/qapi-schema/qapi-schema-test.json @@ -67,7 +67,7 @@ 'discriminator': 'enum1', 'data': { 'value1' : 'UserDefA', 'value2' : 'UserDefB', - 'value3' : 'UserDefB' } } + 'value3' : 'UserDefUnionBase' } } { 'struct': 'UserDefUnionBase', 'base': 'UserDefZero', GEN tests/test-qapi-types.h /home/eblake/qemu/tests/qapi-schema/qapi-schema-test.json:65: Member name 'string' of branch 'value3' clashes with base 'UserDefUnionBase' /home/eblake/qemu/tests/Makefile:415: recipe for target 'tests/test-qapi-types.h' failed But you have me curious if this collision is still caught when the ad hoc tests are gone. If so, great; if not, I'll add a test here. (I'll know later when I get through rebasing to all of your comments.) >> No change to generated code. >> >> Signed-off-by: Eric Blake >=20 > Patch looks good. Yay; it's nice to see results after all our mental gymnastics over how collision testing should work. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --2D2PviF6QN2r1CIi11tm7H3K04HLMqqtB 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/ iQEcBAEBCAAGBQJWQX2yAAoJEKeha0olJ0NqyEcH/1oV6e1ykkOtF87HjEMlw/OF mgpoMd4J5Fc3wbQkPgmTzPpLTAv7lipLWfHNBB7G5WLylgA1wygwIZFBD9OvFVy2 F4wKQb3dVaIEAp2ZPtKKFvCfibMDZubQn5EdhTNsGNU5jlXX+tX4IBsIgInKaYTC g/oVSE8WD8ggZza33dUAqXvTztMXImKUq9cD2AWh0vnHVpqwqsjrx7iOyAtUXqTb BekAbCG9y2Ihh+HJqausOxPTJdsbWrM3YUOL9mJh/pV+rodjkIf2wYqWILCMSjHk nr+arLQgg6vNSuz8hq/gOjFz3FHEdZM9Yd727O6Gc2pm1TEywbsK+MyC0n8w9JE= =qGGZ -----END PGP SIGNATURE----- --2D2PviF6QN2r1CIi11tm7H3K04HLMqqtB--