Re: [Qemu-devel] [PATCH v8 07/17] qapi: Rework collision assertions

[Eric Blake]

On 11/03/2015 12:56 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> On 11/02/2015 08:37 AM, Markus Armbruster wrote:
>>
>>>
>>> Not checked: variant's members don't collide with non-variant members.
>>> I think this check got lost when we simplified
>>> QAPISchemaObjectTypeVariants to hold a single member.
>>
>> Yep, I found the culprit: in your v2 proposal for QAPISchema, you had:
>>
>> +class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
>> +    def __init__(self, name, typ, flat):
>> +        QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
>> +        assert isinstance(flat, bool)
>> +        self.flat = flat
>> +    def check(self, schema, tag_type, seen):
>> +        QAPISchemaObjectTypeMember.check(self, schema, [], seen)
>> +        assert self.name in tag_type.values
>> +        if self.flat:
>> +            self.type.check(schema)
>> +            assert isinstance(self.type, QAPISchemaObjectType)
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg00394.html
>>
>> but the 'if self.flat' clause was lost in v3:
>>
>> https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00450.html
> 
> Quoting v3's change log:
> 
>   - Lower simple variants to flat ones as described on
>     qapi-code-gen.txt.  Replace QAPISchemaObjectType's .flat by
>     .simple_union_type(), for use by pre-existing code-generation
>     warts.
> 
>> I am in fact reinstating it here, but for v9, will do it in a separate
>> patch rather than blended in with the rest of the changes.
> 
> Any "is this union flat or simple" check signals a flaw.  It's either a
> pointless difference in generated code (these should all be marked TODO
> by now), or something's wrong with the desugaring of simple to flat
> unions.

Losing 'if self.flat' was correct, but we still need 'if union'; and
that's what I add in 2/4.

> 
> Therefore, the if self.flat is superfluous.  Good, because otherwise our
> desugaring must be flawed.

And things correctly work on simple unions due to our wrapper type, so
that 'if union' was sufficient.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature