On 11/04/2015 04:11 PM, Eric Blake wrote:
> On 11/04/2015 12:01 PM, Markus Armbruster wrote:
>> Eric Blake <eblake@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.
>>>
> 

>> 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.

And of course, as soon as I hit send, I had another thought - your
patches already added Member.check_clash() called separately from the
.check() chain; maybe I am better off adding a Variants.check_clash()
separate from the .check() chain, rather than trying to cram the whole
nested loop directly in ObjectType.check().

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