[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 01/13] qapi: Add default-variant for flat unions

From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH 01/13] qapi: Add default-variant for flat unions
Date: Fri, 11 May 2018 19:59:00 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0

On 2018-05-10 15:18, Eric Blake wrote:
> On 05/10/2018 08:12 AM, Eric Blake wrote:
> Oh, I just had a thought:
>>> +++ b/scripts/qapi/visit.py
>>> @@ -40,10 +40,20 @@ def gen_visit_object_members(name, base, members, 
>>>       if variants:
>>> +        if variants.default_tag_value is None:
>>> +            ret += mcgen('''
>>> +    %(c_name)s = obj->%(c_name)s;
>>> +''',
>>> +                         c_name=c_name(variants.tag_member.name))
>>> +        else:
>>> +            ret += mcgen('''
>>> +    if (obj->has_%(c_name)s) {
>>> +        %(c_name)s = obj->%(c_name)s;
>>> +    } else {
>>> +        %(c_name)s = %(enum_const)s;
> In this branch of code, is it worth also generating:
> %has_(c_name)s = true;

You mean obj->has_%(c_name)s, and then also set obj->%(c_name)s?

> That way, the rest of the C code does not have to check
> has_discriminator, because the process of assigning the default will
> ensure that has_discriminator is always true later on.  It does have the
> effect that output would never omit the discriminator - but that might
> be a good thing: if we ever have an output union that used to have a
> mandatory discriminator and want to now make it optional, we don't want
> to break older clients that expected the discriminator to be present. It
> does obscure whether input relied on the default, but I don't think that
> hurts.

Also, it would make code a bit simpler because it can cover the !has_X
case under X == default_X.

But OTOH, you could make that case for any optional value -- except
where "is missing" has special value.

And that's the tricky part: I think it's hard to say that a missing
discriminator never has special meaning.  We only need the
default-variant so we know which variant of the union to pick; but we
don't know that the fact that the discriminator value was missing does
not have special meaning.

Of course, we can simply foreclose that by setting it here.

I don't have money in this game, so I suppose it's yours and Markus's
call. :-)


Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]