qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Anton Nefedov <anton.nefedov@virtuozzo.com>,
	qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [PATCH 1/2] qapi: allow flat unions with empty branches
Date: Tue, 15 May 2018 10:20:01 -0500	[thread overview]
Message-ID: <95ab24a5-d600-67b5-9c7e-e5290f42f520@redhat.com> (raw)
In-Reply-To: <87h8n9l7wq.fsf@dusky.pond.sub.org>

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)

or

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.

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

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

  reply	other threads:[~2018-05-15 15:20 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-11  9:05 [Qemu-devel] [PATCH 0/2] qapi: allow flat unions with empty branches Anton Nefedov
2018-05-11  9:05 ` [Qemu-devel] [PATCH 1/2] " Anton Nefedov
2018-05-14 18:08   ` Markus Armbruster
2018-05-14 19:34     ` Eric Blake
2018-05-15  7:01       ` Markus Armbruster
2018-05-15 15:20         ` Eric Blake [this message]
2018-05-15 17:40           ` Markus Armbruster
2018-05-16 15:05             ` Anton Nefedov
2018-05-17  8:05               ` Markus Armbruster
2018-05-17 13:21                 ` Eric Blake
2018-05-18  6:45                   ` Markus Armbruster
2018-05-18  8:16                     ` Anton Nefedov
2018-05-11  9:05 ` [Qemu-devel] [PATCH 2/2] qapi: avoid empty CpuInfoOther type Anton Nefedov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=95ab24a5-d600-67b5-9c7e-e5290f42f520@redhat.com \
    --to=eblake@redhat.com \
    --cc=anton.nefedov@virtuozzo.com \
    --cc=armbru@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).