Re: [Qemu-devel] [PATCH v10 25/30] qapi: Hoist tag collision check to Variants.check()

[Eric Blake]

On 11/09/2015 06:07 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> Checking that a given QAPISchemaObjectTypeVariant.name is a
>> member of the corresponding QAPISchemaEnumType of the owning
>> QAPISchemaObjectTypeVariants.tag_member ensures that there are
>> no collisions in the generated C union for those tag values
>> (since the enum itself should have no collisions).
>>
>> However, this check was the only thing that Variant.check() was
>> doing beyond the work of the superclass ObjectTypeMember.check(),
>> and resulted in a difference of the .check() signatures just to
>> pass the enum type down.
>>
>> Simplify things by instead doing the tag name check as part of
>> Variants.check(), at which point we can rely on inheritance
>> instead of overriding Variant.check().
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>
>> ---

>> -        assert isinstance(self.tag_member.type, QAPISchemaEnumType)
>> +        tag_type = self.tag_member.type
>> +        assert isinstance(tag_type, QAPISchemaEnumType)
>>          for v in self.variants:
>> -            v.check(schema, self.tag_member.type)
>> +            v.check(schema)
>> +            assert v.name in tag_type.values
> 
> Two changes squashed together:
> 
> * Move the assertion from QAPISchemaObjectTypeVariant.check(),
> 
> * Capture self.tag_member.type in tag_type
> 
> The second part makes the patch slightly less obvious.  Matter of taste.

I'm dropping the tag_type temporary member.  Once I played more with
making qtype_code a qapi builtin type, it became much more elegant to
revisit this code (the use of tag_type here was for when my later
patches were trying to key off of tag_type==None as a witness of
alternates).

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

signature.asc
Description: OpenPGP digital signature