On 11/09/2015 05:56 AM, 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 QAPISchemaObjectType.check() to make the same check, >> so we can later reduce some of the ad hoc checks. >>
>> In general, a type used as a branch of a flat union cannot
>> also be the base type of the flat union, so even though we are
>> adding a call to variant.type.check() in order to populate
>> variant.type.members, this is merely a case of gaining
>> topological sorting of how types are visited (and type.check()
>> is already set up to allow multiple calls due to base types).
>
> Yes, a type cannot contain itself, neither as base nor as variant.
>
> We have tests covering attempts to do the former
> (struct-cycle-direct.json, struct-cycle-indirect.json). As far as I can
> see, we don't have tests covering the latter. Do we catch it?
Yes, at least by virtue of the ad hoc tests: attempting to reuse a base
type of the flat union as a variant member will cause the qapi members
of the base type to appear more than once in the JSON object (that is,
the checks that reject flat-union-clash-member.json would also reject
this scenario). To test:
diff --git i/tests/qapi-schema/qapi-schema-test.json
w/tests/qapi-schema/qapi-schema-test.json
index 44638da..16b2ffb 100644
--- i/tests/qapi-schema/qapi-schema-test.json
+++ w/tests/qapi-schema/qapi-schema-test.json
@@ -67,7 +67,7 @@
'discriminator': 'enum1',
'data': { 'value1' : 'UserDefA',
'value2' : 'UserDefB',
- 'value3' : 'UserDefB' } }
+ 'value3' : 'UserDefUnionBase' } }
{ 'struct': 'UserDefUnionBase',
'base': 'UserDefZero',
GEN tests/test-qapi-types.h
/home/eblake/qemu/tests/qapi-schema/qapi-schema-test.json:65: Member
name 'string' of branch 'value3' clashes with base 'UserDefUnionBase'
/home/eblake/qemu/tests/Makefile:415: recipe for target
'tests/test-qapi-types.h' failed
But you have me curious if this collision is still caught when the ad
hoc tests are gone. If so, great; if not, I'll add a test here. (I'll
know later when I get through rebasing to all of your comments.)
>> No change to generated code.
>>
>> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> Patch looks good.
Yay; it's nice to see results after all our mental gymnastics over how
collision testing should work.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
