[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v11 13/28] qapi: Hoist tag collision check to Va
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v11 13/28] qapi: Hoist tag collision check to Variants.check() |
Date: |
Wed, 11 Nov 2015 09:11:36 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 11/11/2015 06:56 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> Checking that a given QAPISchemaObjectTypeVariant.name is a
>> member of the corresponding QAPISchemaEnumType of the owning
>> QAPISchemaObjectTypeVariants.tag_member ensures that there are
>> no collisions in the generated C union for those tag values
>> (since the enum itself should have no collisions).
>>
>> However, this check was the only thing that Variant.check() was
>> doing beyond the work of the superclass ObjectTypeMember.check(),
>
> Since PATCH 05, actually. Suggest to mention that.
I debated about munging patch 5 or 6 to actually make this change there;
but decided that separate is just fine. But yes, mentioning the earlier
commit title will help.
>> @@ -1075,10 +1076,6 @@ class
>> QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
>> def __init__(self, name, typ):
>> QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
>>
>> - def check(self, schema, tag_type):
>> - QAPISchemaObjectTypeMember.check(self, schema)
>> - assert self.name in tag_type.values
>> -
>> # This function exists to support ugly simple union special cases
>> # TODO get rid of them, and drop the function
>> def simple_union_type(self):
>
> QAPISchemaObjectTypeVariant is now an almost trivial variation of
> QAPISchemaObjectTypeMember. Differences:
>
> * __init__() has no parameter optional
>
> * Method simple_union_type(), which exists only to support pointless
> differences in code generation for simple unions, all marked TODO.
>
> There's hope we can get rid of the class after the TODOs are resolved.
Nope. Because in 15/28 I add back in a non-trivial difference of "role =
'branch'" compared to the superclass "role = 'member'", which affects
the quality of error messages using .describe().
That, and I still have no idea how to get rid of the TODO (we know how
to convert simple unions to flat unions, but the conversion is an
either-or choice: either we keep the C layout the same but change {} in
the JSON representation, or we keep QMP the same but affect the C layout
- so fixing the TODO has to resolve that discrepancy, and may end up
touching lots of code).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v11 05/28] qapi: Simplify QAPISchemaObjectTypeMember.check(), (continued)
- [Qemu-devel] [PATCH v11 05/28] qapi: Simplify QAPISchemaObjectTypeMember.check(), Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 06/28] qapi: Clean up after previous commit, Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 02/28] qapi-types: Consolidate gen_struct() and gen_union(), Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 08/28] qapi: Eliminate QAPISchemaObjectType.check() variable members, Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 09/28] qapi: Factor out QAPISchemaObjectTypeMember.check_clash(), Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 12/28] qapi: Factor out QAPISchemaObjectType.check_clash(), Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 14/28] qapi: Remove outdated tests related to QMP/branch collisions, Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 10/28] qapi: Simplify QAPISchemaObjectTypeVariants.check(), Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 13/28] qapi: Hoist tag collision check to Variants.check(), Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 11/28] qapi: Check for qapi collisions of flat union branches, Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 15/28] qapi: Track owner of each object member, Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 16/28] qapi: Detect collisions in C member names, Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 20/28] qapi: Forbid case-insensitive clashes, Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 17/28] cpu: Convert CpuInfo into flat union, Eric Blake, 2015/11/11