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

[Max Reitz]

On 07.02.19 07:56, Markus Armbruster wrote:
> 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).

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

Max

signature.asc
Description: OpenPGP digital signature