All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Max Reitz <mreitz@redhat.com>, Markus Armbruster <armbru@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	qemu-devel@nongnu.org, qemu-block@nongnu.org,
	Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH v3 1/8] qapi: Add default-variant for flat unions
Date: Wed, 5 Jun 2019 14:00:45 -0500	[thread overview]
Message-ID: <325f9894-89d2-c267-9d8c-90df62a28b20@redhat.com> (raw)
In-Reply-To: <4f75a6e9-5f5b-9161-cbb0-91c8034a7abc@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 4586 bytes --]

On 2/8/19 12:21 PM, Max Reitz wrote:
> On 07.02.19 07:56, Markus Armbruster wrote:
>> Max Reitz <mreitz@redhat.com> writes:
>>
>>> This patch allows specifying a discriminator that is an optional member
>>> of the base struct.  In such a case, a default value must be provided
>>> that is used when no value is given.
>>>
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>> ---

>>> +++ b/qapi/introspect.json
>>> @@ -168,6 +168,13 @@
>>>  # @tag: the name of the member serving as type tag.
>>>  #       An element of @members with this name must exist.
>>>  #
>>> +# @default-variant: if the @tag element of @members is optional, this
>>> +#                   is the default value for choosing a variant.  Its
>>> +#                   value will be a valid value for @tag.
>>> +#                   Present exactly when @tag is present and the
>>> +#                   associated element of @members is optional.
>>> +#                   (Since: 4.0)
>>> +#
>>>  # @variants: variant members, i.e. additional members that
>>>  #            depend on the type tag's value.  Present exactly when
>>>  #            @tag is present.  The variants are in no particular order,
>>> @@ -181,6 +188,7 @@
>>>  { 'struct': 'SchemaInfoObject',
>>>    'data': { 'members': [ 'SchemaInfoObjectMember' ],
>>>              '*tag': 'str',
>>> +            '*default-variant': 'str',
>>>              '*variants': [ 'SchemaInfoObjectVariant' ] } }
>>>  
>>>  ##
>>
>> I'm afraid this could become awkward later on.  Let me explain.
>>
>> In many programming languages, absent optional arguments / members
>> default to a default value specified in the declaration.  Simple.
>>
>> In others, 'absent' is effectively an additional value.  The code
>> consuming the argument / member can interpret 'absent' however it wants.
>> It may eliminate the additional value by mapping it to a default value,
>> but it can also interpret 'absent' unlike any value.  If there's more
>> than one consumer, their interpretations need not be consistent.  The
>> declaration provides no clue on semantics of 'absent'.
>>
>> QAPI is in the latter camp.  I trust you can already sense how much I
>> like that.
>>
>> Now you want to permit optional variant discriminators.  As per general
>> rule, their interpretation is left to the code consuming it.  One
>> instance of such code is the generated union visitor, which needs to
>> decide which variant to visit.  Your default-variant tells it which
>> variant to visit.  Other code interpreting the discriminator better be
>> consistent with that, but that's the other code's problem.  Hmm.
>>
>> If I could go back in time, I'd flip QAPI to "'absent' defaults to a
>> default value".  Lacking a time machine, we're stuck with cases of
>> "'absent' means something you can't express with a value" and "'absent'
>> means different things in different contexts" that have been enshrined
>> in external interfaces.  Is there anything we could do to improve
>> matters for saner cases?
>>
>> I think there is: we could provide for an *optional* default value.  If
>> the schema specifies it, that's what 'absent' means.  If it doesn't, all
>> bets are off, just like they are now.
>>
>> Say we do that (for what it's worth, introspect.json is already prepared
>> for it).
> 
> If somebody(tm) does that, great. :-)
> 
>> How would it play with your default-variant?
>>
>> If an optional discriminator specifies a default value, then that's the
>> default variant.  But wait, if there's also a default-variant, *that's*
>> the default variant!  Awkward clash.  To resolve it, we could either
>> forbid combining the two, or rule default-variant overrides the default.
>> Feels needlessly complicated.
> 
> I agree on the needless, not so sure about the complicated.  (Other than
> it being double the work, but, well, the default-variant work is already
> right here.)
> 
>> Could we get away with "if you want a default variant, the discriminator
>> must specify a default"?
> 
> I think so, yes.  I agree that it'd be the better solution.  But to be
> honest I'd really rather not delve any deeper into the QAPI dungeon than
> I've done in this series so far...

I've now hit a case where I'd like a default-variant (namely, improving
nbd-server-add to avoid SocketAddressLegacy); maybe I'll find time to
revive at least this part of the series.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

  reply	other threads:[~2019-06-05 19:02 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-06 19:55 [Qemu-devel] [PATCH v3 0/8] block: Try to create well-typed json:{} filenames Max Reitz
2019-02-06 19:55 ` [Qemu-devel] [PATCH v3 1/8] qapi: Add default-variant for flat unions Max Reitz
2019-02-06 20:13   ` Eric Blake
2019-02-07  6:56   ` Markus Armbruster
2019-02-07 14:01     ` Eric Blake
2019-02-07 14:46       ` Eric Blake
2019-02-08 18:21     ` Max Reitz
2019-06-05 19:00       ` Eric Blake [this message]
2019-02-06 19:55 ` [Qemu-devel] [PATCH v3 2/8] docs/qapi: Document optional discriminators Max Reitz
2019-02-06 19:55 ` [Qemu-devel] [PATCH v3 3/8] tests: Add QAPI optional discriminator tests Max Reitz
2019-02-06 20:20   ` Eric Blake
2019-02-06 19:55 ` [Qemu-devel] [PATCH v3 4/8] qapi: Formalize qcow2 encryption probing Max Reitz
2019-02-06 20:23   ` Eric Blake
2019-02-06 19:55 ` [Qemu-devel] [PATCH v3 5/8] qapi: Formalize qcow " Max Reitz
2019-02-06 19:55 ` [Qemu-devel] [PATCH v3 6/8] block: Try to create well typed json:{} filenames Max Reitz
2019-02-06 20:43   ` Eric Blake
2019-02-06 21:06     ` Eric Blake
2019-02-08 18:11     ` Max Reitz
2019-02-06 19:55 ` [Qemu-devel] [PATCH v3 7/8] iotests: Test internal option typing Max Reitz
2019-02-06 21:28   ` Eric Blake
2019-02-06 19:55 ` [Qemu-devel] [PATCH v3 8/8] iotests: qcow2's encrypt.format is now optional Max Reitz
2019-02-06 21:31   ` Eric Blake

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=325f9894-89d2-c267-9d8c-90df62a28b20@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.