[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v10 24/30] qapi: Factor out QAPISchemaObjectType
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v10 24/30] qapi: Factor out QAPISchemaObjectType.check_clash() |
Date: |
Mon, 9 Nov 2015 22:32:08 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 11/09/2015 07:49 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> 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 <address@hidden>
>>
>> @@ -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)
>> - 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)
>> for m in self.local_members:
>> m.check(schema)
>> m.check_clash(seen)
>> @@ -994,6 +990,12 @@ class QAPISchemaObjectType(QAPISchemaType):
>> assert self.variants.tag_member in self.members
>> self.variants.check_clash(schema, seen)
>>
>> + def check_clash(self, schema, seen):
>> + self.check(schema)
>
> Do we want to hide this .check() inside .check_clash()?
>
> QAPISchemaObjectTypeMember.check() doesn't. I think the two better
> behave the same.
>
>> + assert not self.variants # not implemented
>> + for m in self.members:
>> + m.check_clash(seen)
The self.check(schema) call is necessary to get self.members populated.
We cannot iterate over self.members if the type has not had check()
called; this is true for both callers of type.check_clash()
(ObjectType.check(), and Variants.check_clash()).
You are correct that neither Member.check() nor Member.check_clash()
call a form of type.check() - but that's because at that level, there is
no need to populate a type.members list.
On the other hand, we've been arguing that check() should populate
everything after construction prior to anything else being run; and not
running Variant.type.check() during Variants.check() of flat unions
feels like we may have a hole (a flat union will have to inline its
types to the overall JSON object, and inlining types requires access to
type.members - but as written, we aren't populating them until
Variants.check_clash()). I can play with hoisting the type.check() out
of type.check_clash() and instead keep base.check() in type.check(), and
add variant.type.check() in Variants.check() (but only for unions, not
for alternates), if you are interested.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v10 19/30] qapi: Fix up commit 7618b91's clash sanity checking change, Eric Blake, 2015/11/06
[Qemu-devel] [PATCH v10 18/30] qapi: Clean up after previous commit, Eric Blake, 2015/11/06
[Qemu-devel] [PATCH v10 23/30] qapi: Check for qapi collisions of flat union branches, Eric Blake, 2015/11/06