Eric Blake <ebl...@redhat.com> writes: > Checking that a given QAPISchemaObjectTypeVariant.name is a > member of the corresponding QAPISchemaEnumType of the owning > QAPISchemaObjectTypeVariants.tag_member ensures that there are > no collisions in the generated C union for those tag values > (since the enum itself should have no collisions). > > However, this check was the only thing that Variant.check() was > doing beyond the work of the superclass ObjectTypeMember.check(), > and resulted in a difference of the .check() signatures just to > pass the enum type down. > > Simplify things by instead doing the tag name check as part of > Variants.check(), at which point we can rely on inheritance > instead of overriding Variant.check(). > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- > v10: new patch > --- > scripts/qapi.py | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 6d8c4c7..798df51 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -1056,9 +1056,11 @@ class QAPISchemaObjectTypeVariants(object): > def check(self, schema, seen): > if self.tag_name: # flat union > self.tag_member = seen[self.tag_name] > - assert isinstance(self.tag_member.type, QAPISchemaEnumType) > + tag_type = self.tag_member.type > + assert isinstance(tag_type, QAPISchemaEnumType) > for v in self.variants: > - v.check(schema, self.tag_member.type) > + v.check(schema) > + assert v.name in tag_type.values
Two changes squashed together: * Move the assertion from QAPISchemaObjectTypeVariant.check(), * Capture self.tag_member.type in tag_type The second part makes the patch slightly less obvious. Matter of taste. > > def check_clash(self, schema, seen): > for v in self.variants: > @@ -1071,10 +1073,6 @@ class > QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): > def __init__(self, name, typ): > QAPISchemaObjectTypeMember.__init__(self, name, typ, False) > > - def check(self, schema, tag_type): > - QAPISchemaObjectTypeMember.check(self, schema) > - assert self.name in tag_type.values > - > # This function exists to support ugly simple union special cases > # TODO get rid of them, and drop the function > def simple_union_type(self):