[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Anton Nefedov
Subject: Re: [Qemu-devel] [PATCH 1/2] qapi: allow flat unions with empty branches
Date: Wed, 16 May 2018 18:05:13 +0300
User-agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0

On 15/5/2018 8:40 PM, Markus Armbruster wrote:
Eric Blake <address@hidden> writes:

On 05/15/2018 02:01 AM, Markus Armbruster wrote:

QAPI language design alternatives:

1. Having unions cover all discriminator values explicitly is useful.

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).

I'd prefer a more opinionated design here.

Either we opine making people repeat the tag values is an overall win.
Then make them repeat them always, don't give them the option to not
repeat them.  Drop this patch.  To reduce verbosity, we can add a
suitable way to denote a predefined empty type.

Or we opine it's not.  Then let them not repeat them, don't give them
the option to make themselves repeat them.  Evolve this patch into one
that makes flat union variants optional and default to empty.  If we
later find we still want a way to denote a predefined empty type, we can
add it then.

My personal opinion is it's not.

I followed the arguments up to the last sentence, but then I got lost
on whether you meant:

This patch is not an overall win, so let's drop it and keep status quo
and/or implement a way to write 'branch':{} (option 1 above)


Forcing repetition is not an overall win, so let's drop that
requirement by using something similar to this patch (option 2 above)
but without adding a 'partial-data' key.

Sorry about that.  I meant the latter.

But you've convinced me that option 3 (supporting a compact branch
representation AND supporting missing branches as defaulting to an
empty type) is more of a maintenance burden (any time there is more
than one way to write something, it requires more testing that both
ways continue to work) and thus not worth doing without strong
evidence that we need both ways (which we do not currently have).

I agree that neither option 3 nor this patch are the best way to handle
this, so it's 1 or 2.

(2) sure brings some prettiness into jsons; I wonder when it might harm;
e.g. a person adds another block driver: it would be difficult to get
round BlockdevOptionsFoo, and what can be missed is something
optional like BlockdevStatsFoo, which is harmless if left empty and
probably would be made an empty branch anyway. The difference is that
an empty branch is something one might notice during a review and

I think I'd vote for 2 (never enforce all-branches coverage) as well.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]