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. > > We already ensure that all branches of a flat union are qapi > structs with no variants, at which point those members appear > in the same JSON object as all non-variant members. And we > already have a map 'seen' of all non-variant members. All > we need is a new QAPISchemaObjectTypeVariants.check_clash(), > which clones the seen map then checks for clashes with each > member of the variant's qapi type. > > Note that the clone of seen inside Variants.check_clash() > resembles the one we just removed from Variants.check(); the > difference here is that we are now checking for clashes > among the qapi members of the variant type, rather than for > a single clash with the variant tag name itself. > > 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? > For simple unions, the same code happens to work by design, > because of our synthesized wrapper classes (however, the > wrapper has a single member 'data' which will never collide > with the one non-variant member 'type', so it doesn't really > matter). > > There is no impact to alternates, which intentionally do not > need to call variants.check_clash() (there, at most one of > the variant's branches will be an ObjectType, and even if one > exists, we are not inlining the qapi members of that object > into a parent object, the way we do for unions). > > No change to generated code. > > Signed-off-by: Eric Blake <ebl...@redhat.com> Patch looks good.
