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). 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> --- v10: create new Variants.check_clash() rather than piggybacking on .check() v9: new patch, split off from v8 7/17 --- scripts/qapi.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/scripts/qapi.py b/scripts/qapi.py index e057408..4c56935 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -992,6 +992,7 @@ class QAPISchemaObjectType(QAPISchemaType): if self.variants: self.variants.check(schema, seen) assert self.variants.tag_member in self.members + self.variants.check_clash(schema, seen) def is_implicit(self): # See QAPISchema._make_implicit_object_type() @@ -1057,6 +1058,17 @@ class QAPISchemaObjectTypeVariants(object): for v in self.variants: v.check(schema, self.tag_member.type) + def check_clash(self, schema, seen): + for v in self.variants: + # Reset seen map for each variant, since qapi names from one + # branch do not affect another branch + vseen = dict(seen) + assert isinstance(v.type, QAPISchemaObjectType) + assert not v.type.variants # not implemented + v.type.check(schema) + for m in v.type.members: + m.check_clash(vseen) + class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): def __init__(self, name, typ): -- 2.4.3