From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59526) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZhddB-0003B7-4S for qemu-devel@nongnu.org; Thu, 01 Oct 2015 09:10:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zhdd5-0001Zw-WE for qemu-devel@nongnu.org; Thu, 01 Oct 2015 09:10:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36902) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zhdd5-0001Wt-98 for qemu-devel@nongnu.org; Thu, 01 Oct 2015 09:10:35 -0400 References: <1443565276-4535-1-git-send-email-eblake@redhat.com> <1443565276-4535-6-git-send-email-eblake@redhat.com> <87twqa24e5.fsf@blackfin.pond.sub.org> From: Eric Blake Message-ID: <560D30C9.1000209@redhat.com> Date: Thu, 1 Oct 2015 07:10:33 -0600 MIME-Version: 1.0 In-Reply-To: <87twqa24e5.fsf@blackfin.pond.sub.org> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="qgVTHK6U3A5fVrN3Jig4Wsp89N3wT25rr" Subject: Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions 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) --qgVTHK6U3A5fVrN3Jig4Wsp89N3wT25rr Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 10/01/2015 05:51 AM, Markus Armbruster wrote: >> +++ b/tests/qapi-schema/alternate-clash.json >> @@ -1,3 +1,8 @@ >> -# we detect C enum collisions in an alternate >> +# Alternate branch name collision >> +# Reject an alternate that would result in a collision in generated C= >> +# names (this would try to generate two enum values 'ALT1_KIND_A_B').= >> +# TODO: In the future, if alternates are simplified to not generate >> +# the implicit Alt1Kind enum, we would still have a collision with th= e >> +# resulting C union trying to have two members named 'a_b'. >> { 'alternate': 'Alt1', >> - 'data': { 'one': 'str', 'ONE': 'int' } } >> + 'data': { 'a-b': 'str', 'a_b': 'int' } } >=20 > Ah, you're making the test slightly more robust. Works for me. I just noticed we lack coverage for: { 'alternate': 'Alt', 'data': { 'type': 'int', 'other': 'str' } } (tries to create a C struct with two members named 'type', even after my v5 patches change to a simpler 'qtype_code type'). Can be done in my later patches that simplify alternates if we don't want a v8 spin of this part of the series. >> +++ b/tests/qapi-schema/flat-union-clash-type.json >> @@ -0,0 +1,15 @@ >> +# Flat union branch 'type' >> +# FIXME: this triggers an assertion failure. But even with that fixed= , we >> +# would have a clash in generated C, between the outer tag 'type' and= >=20 > "outer tag"? I guess you mean the member type we inherit from Base. Yes. >> +++ b/tests/qapi-schema/flat-union-cycle.json >> @@ -0,0 +1,8 @@ >> +# Ensure that we have a sane error message for attempts at self-inher= itance >> +# This test currently fails because we don't permit a union base, but= >> +# even if we relax things to allow that in the future (see >> +# flat-union-base-union), self-inheritance should still fail. >=20 > Do we have a test for the simpler case of a struct inheriting from > itself? Not here, but in v5 16/46. That's because it asserts, but while it was easy to fix up in the QAPISchema.check(), I did not find it worth the churn to fix it up in the ad hoc parse code just to rip it back out later, and the QAPISchema.check() code requires several scaffolding patches (so it wasn't as easy as fixing the union 'type' clash asserts). Tracking an assertion failure for any more than one patch at a time is horrible (as any other change to qapi.py changes line numbers that affect the assertion failure). >=20 > I believe us merging struct and union types into a single object type i= s > more likely than us implementing union bases. If I'm correct, we won't= > need this test. Maybe, but even then, we still have to decide if a base object can have variants. >=20 > Found nothing that couldn't be touched up on commit. Your suggestions for wording tweaks are fine; I'm okay if you want to tweak it for your pull request instead of asking me for a v8. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --qgVTHK6U3A5fVrN3Jig4Wsp89N3wT25rr 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/ iQEcBAEBCAAGBQJWDTDJAAoJEKeha0olJ0NqITMH/R94mkzmQ0MYwjykYRYe+8gH Fz1tix2YHv3+T9hxk9arAcGeh34WJSX8hj33bYJidnOCixMvPXhOKJf66HXuFIIq /AatPoPwLBkB3XpMaf4+A8b5en/KESUxNM3yH8vQnmYVv4AFWcGG+H9r2BFPclqs XmsHZ8M5vi7NUmVRhMKQxyE6Jw5bSXsForWXukogZVCX+294m1Mt8ay8sb/C2+88 TED998Voz0TRlOIhvsGjLjnmZV46JDepDoOEbuUTOUZFDYiuYgJNPg5zcJynDHl3 T0ZGh+wt5r/2la7kKH9suk43n+iD6IID3SS3nwmYz5Y51OtuhF3LmaXEOgO/Mns= =Cwct -----END PGP SIGNATURE----- --qgVTHK6U3A5fVrN3Jig4Wsp89N3wT25rr--