From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50814) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fHCJw-0002Ix-Nh for qemu-devel@nongnu.org; Fri, 11 May 2018 13:59:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fHCJv-0001Qg-TL for qemu-devel@nongnu.org; Fri, 11 May 2018 13:59:08 -0400 References: <20180509165530.29561-1-mreitz@redhat.com> <20180509165530.29561-2-mreitz@redhat.com> <6027508c-717e-d3c2-6704-0c1b614fca89@redhat.com> From: Max Reitz Message-ID: <4c7382dc-724f-4420-ea8d-b4d1bf0edf84@redhat.com> Date: Fri, 11 May 2018 19:59:00 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="AjR8ZaJ439rCnp4zs4x6h5pknxkdxAX7G" Subject: Re: [Qemu-devel] [PATCH 01/13] qapi: Add default-variant for flat unions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-block@nongnu.org Cc: qemu-devel@nongnu.org, Markus Armbruster , Kevin Wolf , Michael Roth This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --AjR8ZaJ439rCnp4zs4x6h5pknxkdxAX7G From: Max Reitz To: Eric Blake , qemu-block@nongnu.org Cc: qemu-devel@nongnu.org, Markus Armbruster , Kevin Wolf , Michael Roth Message-ID: <4c7382dc-724f-4420-ea8d-b4d1bf0edf84@redhat.com> Subject: Re: [PATCH 01/13] qapi: Add default-variant for flat unions References: <20180509165530.29561-1-mreitz@redhat.com> <20180509165530.29561-2-mreitz@redhat.com> <6027508c-717e-d3c2-6704-0c1b614fca89@redhat.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 2018-05-10 15:18, Eric Blake wrote: > On 05/10/2018 08:12 AM, Eric Blake wrote: >=20 > Oh, I just had a thought: >=20 >>> +++ b/scripts/qapi/visit.py >>> @@ -40,10 +40,20 @@ def gen_visit_object_members(name, base, members,= =20 >=20 >>> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if variants: >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if variants.default_tag_v= alue is None: >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 r= et +=3D mcgen(''' >>> +=C2=A0=C2=A0=C2=A0 %(c_name)s =3D obj->%(c_name)s; >>> +''', >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= c_name=3Dc_name(variants.tag_member.name)) >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 else: >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 r= et +=3D mcgen(''' >>> +=C2=A0=C2=A0=C2=A0 if (obj->has_%(c_name)s) { >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 %(c_name)s =3D obj->%(c_n= ame)s; >>> +=C2=A0=C2=A0=C2=A0 } else { >>> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 %(c_name)s =3D %(enum_con= st)s; >=20 > In this branch of code, is it worth also generating: >=20 > %has_(c_name)s =3D true; You mean obj->has_%(c_name)s, and then also set obj->%(c_name)s? > That way, the rest of the C code does not have to check > has_discriminator, because the process of assigning the default will > ensure that has_discriminator is always true later on.=C2=A0 It does ha= ve the > effect that output would never omit the discriminator - but that might > be a good thing: if we ever have an output union that used to have a > mandatory discriminator and want to now make it optional, we don't want= > to break older clients that expected the discriminator to be present. I= t > does obscure whether input relied on the default, but I don't think tha= t > hurts. Also, it would make code a bit simpler because it can cover the !has_X case under X =3D=3D default_X. But OTOH, you could make that case for any optional value -- except where "is missing" has special value. And that's the tricky part: I think it's hard to say that a missing discriminator never has special meaning. We only need the default-variant so we know which variant of the union to pick; but we don't know that the fact that the discriminator value was missing does not have special meaning. Of course, we can simply foreclose that by setting it here. I don't have money in this game, so I suppose it's yours and Markus's call. :-) Max --AjR8ZaJ439rCnp4zs4x6h5pknxkdxAX7G Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCAAdFiEEkb62CjDbPohX0Rgp9AfbAGHVz0AFAlr12eQACgkQ9AfbAGHV z0CvhwgAwnkQ4MWSWRribvcpFNlXlKE0sfNs5kM5tKaIjId8ZaftUJePN4OLy8ev uFbfQskgdpIsQshaJ51xWhobJj4YGFn+LAwXeqGMDkSFZ/ITDh0p6/16+Pqr+4Nt zRhNYguuK9J3+/uGUBfS21SLbKlLMF2EtPOZR8PlRGhTzcDMQNV1J+kpBl0BYodh 5xJ8AXR/K4bMIiim1Bvt4nozFez/tgqKM9bqUbNrZK7u+oaU2SoEWL/QpjGeOZ+D pVSu/KKcbLU/jQGq6wAFWkb0Pcxp92u16ds4u/5KsHr7Ma9+CYX0o9AeCCkdmTLR ZBIGAjufNNwCOqG8Ruc0JulnuAzuTA== =jKqh -----END PGP SIGNATURE----- --AjR8ZaJ439rCnp4zs4x6h5pknxkdxAX7G--