Right now, our ad hoc parser ensures that we cannot have a flat union that introduces any QMP member names that would conflict with the non-variant QMP members already present from the union's base type (see flat-union-clash-member.json). However, we want QAPISchema*.check() to make the same check, so we can later reduce some of the ad hoc checks.
Basically, all branches of a flat union must be qapi structs with no variants, at which point those members appear in the same JSON object as all non-variant members. 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); all that is additionally needed is to actually check the each member of the variant type do not add any collisions. For simple unions, the same code happens to work (however, our synthesized wrapper type has a single member 'data' which will never collide with the one non-variant member 'type'). 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 have to pass in an extra flag to distinguish whether we are working with a union or an alternate. The flag is temporary; a later patch will rework how alternates are laid out by creating a new subclass of QAPISchemaObjectTypeMember, and detecting the use of this subclass for variants.tag_member will serve the same purpose. Note that an early proposal [1] for what eventually became commit ac88219a had QAPISchemaObjectTypeVariant.check() ensure that the variant type was complete, although it was later removed [2]; the checks added here happen to match what that earlier attempt meant to do. [1] https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg00394.html [2] https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00450.html Signed-off-by: Eric Blake <ebl...@redhat.com> --- v9: new patch, split off from v8 7/17 --- scripts/qapi.py | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index fff4adb..3cf051f 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -1047,7 +1047,8 @@ class QAPISchemaObjectTypeVariants(object): self.tag_member = tag_member self.variants = variants - def check(self, schema, seen): + # TODO drop 'union' param once tag_member is sufficient to spot alternates + def check(self, schema, seen, union=True): if self.tag_name: # flat union self.tag_member = seen[self.tag_name] assert self.tag_member @@ -1055,17 +1056,29 @@ 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 QMP names from one + # branch do not affect another branch + v.check(schema, self.tag_member.type, dict(seen), union) class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): def __init__(self, name, typ): QAPISchemaObjectTypeMember.__init__(self, name, typ, False) - def check(self, schema, tag_type, seen): - QAPISchemaObjectTypeMember.check(self, schema, seen) + # TODO drop 'union' param once tag_type is sufficient to spot alternates + def check(self, schema, tag_type, seen, union): + QAPISchemaObjectTypeMember.check(self, schema, dict(seen)) assert self.name in tag_type.values + if union: + # If this variant is used within a union, then each member + # field must avoid collisions with the non-variant members + # already present in the union. + assert isinstance(self.type, QAPISchemaObjectType) + assert not self.type.variants # not implemented + self.type.check(schema) + for m in self.type.members: + assert c_name(m.name) not in seen + seen[m.name] = m # This function exists to support ugly simple union special cases # TODO get rid of them, and drop the function @@ -1088,7 +1101,7 @@ class QAPISchemaAlternateType(QAPISchemaType): def check(self, schema): seen = {} self.variants.tag_member.check(schema, seen) - self.variants.check(schema, seen) + self.variants.check(schema, seen, False) def json_type(self): return 'value' -- 2.4.3
