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 QAPISchema*.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 passed in to QAPISchemaObjectTypeVariants.check(), which we clone for each particular variant (since the members of one variant do not clash with another). So all that is additionally needed is to actually check the each member of the variant type do not add any collisions. 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). But for alternates, we do NOT want to check the type members for adding collisions (an alternate has no parent JSON object that is merging in member names, the way a flat union does), so we branch on whether seen is empty to distinguish whether we are checking a union or an alternate. No change to generated code. Signed-off-by: Eric Blake <ebl...@redhat.com> --- v9: new patch, split off from v8 7/17 --- scripts/qapi.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index a20abda..3054628 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -1057,8 +1057,9 @@ class QAPISchemaObjectTypeVariants(object): assert self.tag_member in seen.itervalues() assert isinstance(self.tag_member.type, QAPISchemaEnumType) for v in self.variants: - vseen = dict(seen) - v.check(schema, self.tag_member.type, vseen) + # Reset seen array for each variant, since qapi names from one + # branch do not affect another branch + v.check(schema, self.tag_member.type, dict(seen)) class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): @@ -1068,6 +1069,14 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): def check(self, schema, tag_type, seen): QAPISchemaObjectTypeMember.check(self, schema) assert self.name in tag_type.values + if seen: + # This variant is used within a union; ensure each qapi member + # field does not collide with the union's non-variant members. + assert isinstance(self.type, QAPISchemaObjectType) + assert not self.type.variants # not implemented + self.type.check(schema) + for m in self.type.members: + m.check_clash(seen) # This function exists to support ugly simple union special cases # TODO get rid of them, and drop the function -- 2.4.3