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. Still missing, until now, was a call to variant.type.check() during Variants.check() (to populate variant.type.members), and a new Variants.check_clash() which clones the seen map for each variant before checking that variant's members for clashes. Note that cloning '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 non-empy 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). However, since variants can have non-object branches, it did mean that the addition of v.type.check() in Variants.check() had to be conditional on visiting an object type. No change to generated code. Signed-off-by: Eric Blake <ebl...@redhat.com> --- v11: keep type.check() in check(), add comment to alternate v10: create new Variants.check_clash() rather than piggybacking on .check() v9: new patch, split off from v8 7/17 --- scripts/qapi.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/scripts/qapi.py b/scripts/qapi.py index c6cb17b..b2d071f 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() @@ -1056,6 +1057,18 @@ class QAPISchemaObjectTypeVariants(object): assert isinstance(self.tag_member.type, QAPISchemaEnumType) for v in self.variants: v.check(schema, self.tag_member.type) + if isinstance(v.type, QAPISchemaObjectType): + v.type.check(schema) + + 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 + for m in v.type.members: + m.check_clash(vseen) class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): @@ -1086,6 +1099,8 @@ class QAPISchemaAlternateType(QAPISchemaType): def check(self, schema): self.variants.tag_member.check(schema) + # Not calling self.variants.check_clash(), because there's nothing + # to clash with self.variants.check(schema, {}) def json_type(self): -- 2.4.3