Re: [Qemu-block] [Qemu-devel] [PATCH v3 1/8] qapi: Add default-variant for flat unions

[Markus Armbruster]

Max Reitz <address@hidden> 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 <address@hidden>
> ---
>  qapi/introspect.json           |  8 +++++
>  scripts/qapi/common.py         | 58 ++++++++++++++++++++++++++++------
>  scripts/qapi/doc.py            | 10 ++++--
>  scripts/qapi/introspect.py     | 10 ++++--
>  scripts/qapi/visit.py          | 13 ++++++++
>  tests/qapi-schema/test-qapi.py |  2 ++
>  6 files changed, 86 insertions(+), 15 deletions(-)
>
> diff --git a/qapi/introspect.json b/qapi/introspect.json
> index 3d22166b2b..e4740aa7a0 100644
> --- a/qapi/introspect.json
> +++ 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).  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.

Could we get away with "if you want a default variant, the discriminator
must specify a default"?

[...]