On 11/04/2015 12:01 PM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> Right now, our ad hoc parser ensures that we cannot have a >> flat union that introduces any qapi member names that would >> conflict with the non-variant qapi members already present >> from the union's base type (see flat-union-clash-member.json). >> We want QAPISchema*.check() to make the same check, so we can >> later reduce some of the ad hoc checks. >>
>> @@ -1068,6 +1069,14 @@ class
>> QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
>> def check(self, schema, tag_type, seen):
>> QAPISchemaObjectTypeMember.check(self, schema)
>> assert self.name in tag_type.values
>> + if seen:
>> + # This variant is used within a union; ensure each qapi member
>> + # field does not collide with the union's non-variant members.
>> + assert isinstance(self.type, QAPISchemaObjectType)
>> + assert not self.type.variants # not implemented
>> + self.type.check(schema)
>> + for m in self.type.members:
>> + m.check_clash(seen)
>>
>> # This function exists to support ugly simple union special cases
>> # TODO get rid of them, and drop the function
>
> Two call chains:
>
> * QAPISchemaObjectType.check() via QAPISchemaObjectTypeVariants.check()
>
> In this case, the new conditional executes.
>
> * QAPISchemaAlternateType.check() via same
>
> In this case, it doesn't.
>
> Why can't we simply add the new code to QAPISchemaObjectType.check()?
We could, but we'd need a nested for-loop:
for v in variants.variants:
v.type.check(schema)
assert not v.type.variants
vseen = dict(seen)
for m in v.type.members:
m.check_clash(seen)
> The rest of the clash checking is already there...
You do have a point there. I guess it wouldn't be the end of the world
to have the loop nesting be explicit rather than indirect through the
intermediate Variants.check().
I'll play with it; and depending on what I do, that may mean I don't
have to munge your other patches quite so heavily.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
