On 11/03/2015 12:56 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> On 11/02/2015 08:37 AM, Markus Armbruster wrote: >> >>> >>> Not checked: variant's members don't collide with non-variant members. >>> I think this check got lost when we simplified >>> QAPISchemaObjectTypeVariants to hold a single member. >> >> Yep, I found the culprit: in your v2 proposal for QAPISchema, you had: >> >> +class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): >> + def __init__(self, name, typ, flat): >> + QAPISchemaObjectTypeMember.__init__(self, name, typ, False) >> + assert isinstance(flat, bool) >> + self.flat = flat >> + def check(self, schema, tag_type, seen): >> + QAPISchemaObjectTypeMember.check(self, schema, [], seen) >> + assert self.name in tag_type.values >> + if self.flat: >> + self.type.check(schema) >> + assert isinstance(self.type, QAPISchemaObjectType) >> >> https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg00394.html >> >> but the 'if self.flat' clause was lost in v3: >> >> https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00450.html > > Quoting v3's change log: > > - Lower simple variants to flat ones as described on > qapi-code-gen.txt. Replace QAPISchemaObjectType's .flat by > .simple_union_type(), for use by pre-existing code-generation > warts. > >> I am in fact reinstating it here, but for v9, will do it in a separate >> patch rather than blended in with the rest of the changes. > > Any "is this union flat or simple" check signals a flaw. It's either a > pointless difference in generated code (these should all be marked TODO > by now), or something's wrong with the desugaring of simple to flat > unions.
Losing 'if self.flat' was correct, but we still need 'if union'; and that's what I add in 2/4. > > Therefore, the if self.flat is superfluous. Good, because otherwise our > desugaring must be flawed. And things correctly work on simple unions due to our wrapper type, so that 'if union' was sufficient. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature