From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35626) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YczjH-0006sV-2W for qemu-devel@nongnu.org; Tue, 31 Mar 2015 13:13:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YczjC-0001gN-23 for qemu-devel@nongnu.org; Tue, 31 Mar 2015 13:13:31 -0400 Received: from mx1.redhat.com ([209.132.183.28]:53808) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YczjB-0001gI-QB for qemu-devel@nongnu.org; Tue, 31 Mar 2015 13:13:25 -0400 Date: Tue, 31 Mar 2015 19:13:20 +0200 From: Kevin Wolf Message-ID: <20150331171320.GD4748@noname.redhat.com> References: <1427227433-5030-1-git-send-email-eblake@redhat.com> <1427227433-5030-7-git-send-email-eblake@redhat.com> <87h9t76gng.fsf@blackfin.pond.sub.org> <55142006.50606@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="sm4nu43k4a2Rpi4c" Content-Disposition: inline In-Reply-To: <55142006.50606@redhat.com> Subject: Re: [Qemu-devel] [PATCH v5 06/28] qapi: Add some union tests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: lcapitulino@redhat.com, famz@redhat.com, Markus Armbruster , wenchaoqemu@gmail.com, qemu-devel@nongnu.org --sm4nu43k4a2Rpi4c Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Am 26.03.2015 um 16:04 hat Eric Blake geschrieben: > On 03/26/2015 07:18 AM, Markus Armbruster wrote: > > Eric Blake writes: > >> Meanwhile, it would be nice to allow > >> 'discriminator':'EnumType' without 'base' for creating a simple union > >> that is type-safe rather than open-coded to a generated enum (right > >> now, we are only type-safe when using a flat union, but that uses > >> a different syntax of 'discriminator':'member-name' which requires > >> a base class containing a 'member-name' enum field). > >=20 > > I'm afraid I don't get you. Can you give examples? >=20 > Using this common code with the appropriate union for each example: > { 'command':'foo', 'data':UNION } >=20 > Right now, we have flat unions which are required to be type-safe (all > branches MUST map back to the enum type of the discriminator, enforced > by the generator, so that if the enum later adds a member, the union > must also be updated to match): >=20 > [1] > { 'union':'Safe', 'base':'Base', 'discriminator':'switch', > 'data':{ 'one':'str', 'two':'int' }} >=20 > {"execute":"foo", "arguments":{'switch':'one', 'data':'hello'}} >=20 > and simple unions which cannot be typesafe (the branches of the union > are open-coded - even if they correlate to an existing enum, there is > nothing enforcing that extensions to the enum be reflected into the union= ): >=20 > [2] > { 'union':'SimpleButOpenCoded', > 'data':{ 'one': 'str', 'two':'int' }} >=20 > {"execute":"foo", "arguments":{'type':'one', 'data':'hello'}} >=20 > I'm hoping to add as a followup series a variant of simple unions that > is type-safe: >=20 > [3] > { 'union':'SimpleAndSafe', 'discriminator':'MyEnum', > 'data':{ 'one':'str', 'two':'int' }} >=20 > {"execute":"foo", "arguments":{'type':'one', 'data':'hello'}} This would overload 'discriminator' with two different meanings: * In case [1] it refers to one key in the JSON object which contains the name of the union branch to select. That is, it is the _name_ of the key used as a discriminator. * In case [3], the 'type' key is used in the JSON object to select a union branch. 'discriminator' describes the _valid values_ of it, i.e. the branch names. We shouldn't mix these meanings. If you need [3], we could call it 'discriminator-type' or something like that. If both options are consistently used for simple and flat unions, you end up with these rules: * 'discriminator' is the key that is used to select the union branch. - For flat unions it is required and must refer to an explicitly declared key in the base type. - For simple unions it is optional and defaults to 'type'. The key is implicitly created in the union type. * 'discrimiator-type' describes the valid values of 'discriminator', either by referring to an enum type or to 'str'. - For flat unions, this must match the type of the explicit declaration of the discriminator field. It is optional and defauls to the only valid value. - For simple unions it is optional, too, and defaults to 'str'. - If it is the name of an enum type, that enum type is reused and the declared union branches must match the valid values of the enum. - If it is 'str', a new enum is generated, and all the declared union branches are used as valid values. There's quite some duplication in it where we need to make sure that the schema matches in all places, but without an explicit declaration of the disriminator field in simple unions, this seems to be the best I can come up with. > But the existing, unused-except-in-testsuite, notion of a simple union > with a base class looks like: >=20 > [4] > { 'type':'Shared', 'data':{'common':'int'}} > { 'union':'SimpleWithBase', 'base':'Shared', > 'data':{ 'one':'str', 'two':'int' }} >=20 > {"execute":"foo", "arguments":{'common':1, 'type':'one', 'data':'hello'}} >=20 > If we were to allow the addition of 'discriminator':'EnumType' to a > simple union [3], but then add that discriminator to an existing case of > a simple union with base [4], it would look like: >=20 > { 'type':'Shared', 'data':{'common':'int'}} > { 'union':'SimpleWithBaseAndDiscriminator', 'base':'Shared', > 'discriminator':'MyEnum', > 'data':{ 'one':'str', 'two':'int' }} >=20 > Yuck. That is indistinguishable from flat unions [1], except by whether > discriminator names an enum type or a member of the base class. Which could even be ambiguous, couldn't it? > > In particular, I can define simple unions in terms of flat ones by > > restricting all union cases to a single member named 'data'. They're > > not implemented that way, but that's a historical accident. Simple > > unions are a redundant concept. >=20 > Cool. Or more concretely, >=20 > { 'union': 'Simple', 'data': { 'one': 'str', 'two': 'int' } } >=20 > is identical on the wire to: >=20 > { 'enum': 'MyEnum', 'data': ['one', 'two'] } > { 'type': 'Base', 'data': { 'type': 'MyEnum' } } > { 'type': 'Branch1', 'data': { 'data': 'str' } } > { 'type': 'Branch2', 'data': { 'data': 'int' } } > { 'union': 'Flat', 'base': 'Base', 'discriminator': 'type', > 'data': { 'one': 'Branch1', 'two': 'Branch2' } } Perhaps we should expose all unions for schema introspection in a way similar to this (possibly not splitting out Branch1 and Branch2 as independent types, though). We would just have to give a name to implicitly generated enums and base types. Kevin --sm4nu43k4a2Rpi4c Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJVGtWwAAoJEH8JsnLIjy/W9KEQAK/MxH2nzcqEVoGF8ZSbN8y3 nBiiVaCLSJspUC8EjSQFek+63SJsbMHUGtztCoFalbitmcIKE4CuIZe72ptACK8u iKY+ED4ph7evWTLFn3NTWJFQv+OEQHh1UrlHW2n6CX76eQwtyYNbSTKh02O6inUg /1kRjVibTWELD/Y7nowrDTyxPV9Dc96pygz8yictLb22YihJrIq4NN0Q2Aum9rfl fEHZDeAmA/Om3ES+tlt5C/zjtg8KxyQk0p7jSF+h1KKgKih3jJS3dc6+7EaFx9fw dK7Xqh870urQcWFYscadBshhMEptarMKnPoMgVOOQLa8oUu30UOKinGlbQ2f73Fn koAjW5Ut21RnWBUkrprkS34rN1Qi0uNePFqfnem1hTq9ihxt1NdW0OoMeaGU2G7q MpCImIw1iMj583FS35Nhvky1Uee9Fj4dyB6vDhWTLSbC4U4wbcOPI2pRTm6zCjOZ hGrVmzWdJJRaVaS6NEaVlLzTbFdROMNTsonXj+C31tgrwjWpZSzq2JrjquUS2BGO EO2wmyqUXTTjCYdFjduxmNAkzJCl0p2WIbCPkFXxnEAUOfeU2ewNX3EOBxZOied5 5y8Jr3TWq7tHGmKzRqoPUt3rezs4326j+FvzFGBGAg2iqYyXU2QaQF1FAXne5+Qp NSJ4MBUZ4uzgFku31qvm =sKXn -----END PGP SIGNATURE----- --sm4nu43k4a2Rpi4c--