Re: [Qemu-devel] [PATCH 1/2] qapi: allow flat unions with empty branches

[Eric Blake]

On 05/14/2018 01:08 PM, Markus Armbruster wrote:
> Anton Nefedov <address@hidden> writes:
>
> > The patch makes possible to avoid introducing dummy empty types
> > for the flat union branches that have no data.

> Some unions have no variant members for certain discriminator values.
> We currently have to use an empty type there, and we've accumulated
> several just for the purpose, which is annoying.
>
> QAPI language design alternatives:
>
> 1. Having unions cover all discriminator values explicitly is useful.
> All we need is a more convenient way to denote empty types.  Eric
> proposed {}, as in 'foo: {}.

And even posted a patch for it once:
https://lists.gnu.org/archive/html/qemu-devel/2016-07/msg00311.html

although it was late enough in that series with other churn in the 
meantime that it would need serious revisiting to apply today.

> 2. Having unions repeat all the discriminator values explicitly is not
> useful.  All we need is replacing the code enforcing that by code
> defaulting missing ones to the empty type.
>
> 3. We can't decide, so we do both (this patch).
>
> Preferences?

Here's some tradeoffs to consider:

Allowing 'foo':{} makes your intent explicit - "I know this branch of 
the union exists, but it specifically adds no further members".  But 
it's a lot of redundant typing - "I already had to type all the branch 
names when declaring the enum type, why do it again here?"

Allowing an easy way to mark all non-listed members of an enum default 
to the empty type is compact - "I told you the enum, so you are 
permitted to fill in an empty type with every branch that does not 
actually need further members; and I had to opt in to that behavior, so 
that I purposefully get an error if I did not opt in but forgot an enum 
member".  But if the enum is likely to change, it can lead to forgotten 
additions later on - "I'm adding member 'bar' to an enum that already 
has member 'foo'; therefore, grepping for 'foo' should tell me all 
places where I should add handling for 'bar', except that it doesn't 
work when handling for 'foo' was implicit but handling for 'bar' should 
not be".

Having said that, my personal preference is that opting in to an 
explicit way to do less typing ("all branches of the enum listed here 
are valid, and add no further members") seems like a nice syntactic 
sugar trick; and the easiest way to actually implement it is to probably 
already have support for an explicit 'foo':{} branch notation.  That 
way, you can always change your mind later and undo the opt-in 
"data-partial" flag and rewrite the union with explicit empty branches 
when needed, to match how the sugar was expanded on your behalf.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org