On 11/03/2015 09:19 AM, Markus Armbruster wrote: >>> I'm assuming this patch is based on >>> [PATCH v8 06/17] qapi-types: Consolidate gen_struct() and gen_union() >>> which has >>> >>> def check(self, schema, members, seen): >>> if self.tag_name: # flat union >>> self.tag_member = seen[self.tag_name] >>> elif seen: # simple union >>> assert self.tag_member in seen.itervalues() >>> else: # alternate >>> ---> self.tag_member.check(schema, members, seen) >>> >>> in QAPISchemaObjectTypeVariants. >> >> That was true in v8, but not in my pending v9 which did essentially what >> your 7/7 did (move the tag_member.check() into Alternate.check() back in >> patch 5).
Caused me some churn in incorporating your patches, but not too bad. >> >> At any rate, my first glance of your series shows that it is reasonable, >> so my task today is to spit out a v9 of my series, but using your seven >> patches in place of my four. > > Buyer beware: I'm not sure my seven do everything your four do. Yours do the same as my patch 1. You also made a compelling argument for eliminating my patch 3 (the enum values are already collision-proof, therefore if we check that a variant name is in the enum, we don't need to check for collisions) - except that in my 10/17, when I get rid of the generated enum for alternates, I have to reinstate that check somewhere; but adding it directly to QAPISchemaAlternateType.check() at that point feels better. My patch 2 is still needed, but looks a bit nicer on top of some of your refactoring. And my patch 4 is still useful, as additional refactoring to share the code used by my patch 2 and your code for ObjectType.check(). We'll see how it all turns out in v9. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
