qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>,
	Michael Roth <mdroth@linux.vnet.ibm.com>,
	qemu-block@nongnu.org, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v4 03/14] qapi: Introduce default values for struct members
Date: Fri, 22 Nov 2019 08:29:00 +0100	[thread overview]
Message-ID: <874kyw74jn.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <817914b7-79d7-a2c9-142b-0a040b1da7ff@redhat.com> (Eric Blake's message of "Thu, 21 Nov 2019 13:56:04 -0600")

Eric Blake <eblake@redhat.com> writes:

> On 11/21/19 1:46 PM, Markus Armbruster wrote:
>
>>>> The '*' is redundant in this form.
>>>>
>>>> Can anyone think of reasons for keeping it anyway?  Against?
>>>
>>> Is there ever a reason to allow an optional member but without a
>>> 'default' value?  Or can we just blindly state that if 'default' is
>>> not present, that is the same as 'default':0/'default':null?
>>>
>>> Or, applying your statement further,
>>>
>>> 'data': { '*a':'int', '*b':'str' }
>>>
>>> is shorthand for:
>>>
>>> 'data': { 'a': { 'type':'int', 'default':0 },
>>>            'b': { 'type':'str', 'default':null } }
>>
>> You propose to default 'default' to a type-specific value.
>>
>> I don't think that's a good idea.
>
> ...
>
>
>> When an absent member behaves just like it was present with a certain
>> value DFLT, we want to be able to say in the schema 'default': DFLT.
>>
>> But the schema language also needs to let us say "absent member behaves
>> unlike any value".
>>
>> If we define 'default' to default to a value, then that value must have
>> this special meaning.
>>
>> Where that value is also a valid value, the schema language cannot
>> express "absent member behaves like it was present with that value".
>>
>> I think this makes 0 a poor default value for 'default': "behaves like
>> member was present with value 0" is fairly common, I think.
>>
>> Defaulting 'default' to null regardless of member type could work.
>>
>> null is a valid value of the 'null' type and of alternate types with a
>> member of type 'null'.  For optional members of such types, the schema
>> language then can't express ""absent member behaves like it was present
>> with value null".  I think the need to say that is much less common than
>> the needs to say "like value 0".
>>
>> Checking...  *sigh*: there are a few optional members that can take null
>> values, e.g. MigrateSetParameters member @tls-creds.  I read its doc
>> comment twice, and I have to admit I can't make heads or tails of it.
>> Can't say for sure whether absent behaves like null, or some other
>> value, or unlike any value.
>>
>> QAPI/QMP introspection already specifies null to have exactly this
>> special meaning.
>
> Maybe that means we need '*name':'t' to expand into something longer, maybe
>  'name':{'type':'t', 'optional':true}
> which in  turn would be synonymous with your idea of ALL types
> accepting a default of null:
>  'name':{'type':'t', 'optional':true, 'default':null}

Yes, this is something we can consider.

Currently, we normalize away the '*' prefix when we go from the abstract
parse tree (which we call "expressions") to the internal representation:

    def _make_member(self, name, typ, ifcond, info):
        optional = False
        if name.startswith('*'):
            name = name[1:]
            optional = True
        if isinstance(typ, list):
            assert len(typ) == 1
            typ = self._make_array_type(typ[0], info)
        return QAPISchemaObjectTypeMember(name, info, typ, optional, ifcond)

This keeps the normalization internal.

Other normalizations we perform on the abstract parse tree.  For
instance, here's the one that normalizes shorthand member "'KEY': 'ARG'"
to longhand "'KEY': { 'type': 'ARG' }":

    def normalize_members(members):
        if isinstance(members, OrderedDict):
            for key, arg in members.items():
                if isinstance(arg, dict):
                    continue
                members[key] = {'type': arg}

For these normalizations, both the shorthand and the longhand form are
part of the schema language.  We could do the same for '*'.

> At any rate, your counterpoint is taken - whatever we pick, we'll want
> to make sure that introspection can expose semantics, and whether we
> can make '*' redundant with some other form of longhand in the qapi
> language is in part determined by whether we also reflect that through
> introspection.

Introspection has the true member name, without the '*' prefix.

We'll also want to avoid unnecessary compromises on QAPI schema
expressiveness.  If we use null to mean "schema does not specify
behavior when member is absent", we can't use it to mean "absent member
behaves like the value null".  A bit of a blemish, but I think it's a
tolerable one.

>                 If that means that keeping '*' in the longhand form of
> optional members (whether or not those members have a default value),
> then so be it.

I believe both

    '*KEY': { 'type': ARG': 'default': null }

and

    'KEY': { 'type': ARG': 'default': null }

are viable longhand forms for '*KEY': 'ARG'.

I prefer the latter, but I'm open to arguments.



  reply	other threads:[~2019-11-22  7:30 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-24 17:39 [Qemu-devel] [PATCH v4 00/14] block: Try to create well-typed json:{} filenames Max Reitz
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 01/14] qapi: Parse numeric values Max Reitz
2019-11-14  9:15   ` Markus Armbruster
2019-11-14  9:50     ` Max Reitz
2019-11-14 12:01       ` Markus Armbruster
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 02/14] qapi: Move to_c_string() to common.py Max Reitz
2019-11-14  9:20   ` Markus Armbruster
2019-11-14  9:58     ` Max Reitz
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 03/14] qapi: Introduce default values for struct members Max Reitz
2019-11-14 15:53   ` Markus Armbruster
2019-11-21 15:07   ` Markus Armbruster
2019-11-21 15:24     ` Eric Blake
2019-11-21 19:46       ` Markus Armbruster
2019-11-21 19:56         ` Eric Blake
2019-11-22  7:29           ` Markus Armbruster [this message]
2019-11-22 10:25             ` Kevin Wolf
2019-11-22 14:40               ` Markus Armbruster
2019-11-22 16:12                 ` Kevin Wolf
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 04/14] qapi: Allow optional discriminators Max Reitz
2019-11-21 15:13   ` Markus Armbruster
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 05/14] qapi: Document default values for struct members Max Reitz
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 06/14] test-qapi: Print struct members' default values Max Reitz
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 07/14] tests: Test QAPI default values for struct members Max Reitz
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 08/14] tests: Add QAPI optional discriminator tests Max Reitz
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 09/14] qapi: Formalize qcow2 encryption probing Max Reitz
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 10/14] qapi: Formalize qcow " Max Reitz
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 11/14] block: Try to create well typed json:{} filenames Max Reitz
2019-06-24 20:06   ` Max Reitz
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 12/14] iotests: Test internal option typing Max Reitz
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 13/14] iotests: qcow2's encrypt.format is now optional Max Reitz
2019-06-24 17:39 ` [Qemu-devel] [PATCH v4 14/14] block: Make use of QAPI defaults Max Reitz
2019-06-24 18:35 ` [Qemu-devel] [PATCH v4 00/14] block: Try to create well-typed json:{} filenames no-reply
2019-06-24 19:04   ` Max Reitz
2019-06-24 19:06     ` Max Reitz
2019-06-24 19:00 ` no-reply
2019-06-24 20:06   ` Max Reitz
2019-08-19 16:49 ` Max Reitz
2019-09-13 11:49 ` Max Reitz
2019-11-06 13:01   ` Max Reitz
2019-11-14  8:54     ` Markus Armbruster
2019-11-21 15:17       ` Markus Armbruster

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=874kyw74jn.fsf@dusky.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=eblake@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 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).