Re: [Qemu-devel] [PATCH v8.5 1/4] qapi: Drop all_members parameter from check()

[Eric Blake]

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