Eric Blake <ebl...@redhat.com> writes: > On 11/09/2015 10:16 PM, Eric Blake wrote: > >>> 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' } } > > Another test I just tried is creating a flat union with: > > empty -> base -> union > > then use empty as one of the union branches. In that case, there is no > conflict (although base is included twice, neither inclusion adds > members to the JSON object; and the inclusion is not circular so things > compile just fine). So probably not worth adding a test for it.
Member name clashes occur because: 1. Two separately defined members happen to have a clashing name. Whether the members are local or inherited, variant or non-variant doesn't matter. 2. The same type gets spliced in twice, and conflict. By "spliced", I mean the members get included. A bit like unboxed, but also "unwrapped". Base types and variant types get spliced in. Variant types don't conflict with each other. Empty type can't conflict (outlawing it anyway would be pointless). Therefore, the only conflicting splice is a non-empty type occuring both as (base type of the) base type and as (base type of a) variant type. We detect this kind of clash exactly like 1. That's fine, because the resulting error messages are plainly good enough. 3. Type contains itself. The type gets spliced into itself, or has itself as unboxed member. We don't unbox object members so far. That leaves the spliced ones: base types and variants. Thus, a type contains itself if it's its own base or one of its own variants, or a direct or indirect base of either. We detect this kind of clash separately, when we guard against infinite check() recursion. Reporting it right there will be easiest. We want to cover all this with tests. I guess we're lacking only 3.