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. :-) Max
signature.asc
Description: OpenPGP digital signature
