Message ID | 1446791754-23823-26-git-send-email-eblake@redhat.com |
---|---|
State | New |
Headers | show |
Eric Blake <eblake@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 <eblake@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):
On 11/09/2015 06:07 AM, Markus Armbruster wrote: > Eric Blake <eblake@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 <eblake@redhat.com> >> >> --- >> - 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. I'm dropping the tag_type temporary member. Once I played more with making qtype_code a qapi builtin type, it became much more elegant to revisit this code (the use of tag_type here was for when my later patches were trying to key off of tag_type==None as a witness of alternates).
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 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):
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 <eblake@redhat.com> --- v10: new patch --- scripts/qapi.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)