On 11/09/2015 06:00 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> Consolidate two common sequences of clash detection into a >> new QAPISchemaObjectType.check_clash() helper method. >> >> No change to generated code. >> >> Signed-off-by: Eric Blake <ebl...@redhat.com> >> >> ---
>> @@ -980,11 +980,7 @@ class QAPISchemaObjectType(QAPISchemaType): >> seen = OrderedDict() >> if self._base_name: >> self.base = schema.lookup_type(self._base_name) >> - assert isinstance(self.base, QAPISchemaObjectType) > > This assertion is lost. > >> - assert not self.base.variants # not implemented >> - self.base.check(schema) >> - for m in self.base.members: >> - m.check_clash(seen) >> + self.base.check_clash(schema, seen) Directly lost, but indirectly still present. The new code is calling QAPISchemaObjectType.check_clash(), which won't exist unless self.base is a QAPISchemaObjectType. Folding the assert into the refactored function makes no sense (the condition isinstance(self, QAPISchemaObjectType) would always be true), and leaving the assert prior to calling self.base.check_clash() adds no real protection against programming bugs. >> @@ -1062,12 +1064,7 @@ class QAPISchemaObjectTypeVariants(object): >> 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) > > This assertion is lost. > >> - assert not v.type.variants # not implemented >> - v.type.check(schema) >> - for m in v.type.members: >> - m.check_clash(vseen) >> + v.type.check_clash(schema, dict(seen)) Same explanation. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature