[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v11 27/28] qapi: Move duplicate enum value check
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v11 27/28] qapi: Move duplicate enum value checks to schema check() |
Date: |
Tue, 17 Nov 2015 23:48:06 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 11/12/2015 08:46 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> Similar to the previous commit, move the detection of a collision
>> in enum values from parse time to QAPISchemaEnumType.check().
>> This happens to also detect collisions in union branch names
>> mapping to the same enum value, even when the names do not
>> collide case-wise. So for a decent error message, we have to
>> determine if the enum is implicit (and if so where the real
>> collision lies).
>>
>> + def _describe(self, schema):
>> + # If the enum is implicit, report the error on behalf of
>> + # the union or alternate that triggered the enum
>> + if self.is_implicit():
>> + owner = schema.lookup_type(self.name[:-4])
>> + assert owner
>> + if isinstance(owner, QAPISchemaAlternateType):
>> + return "Alternate '%s' branch" % owner.name
>
> Didn't we just drop this kind of implicit enum?
>
>> + else:
>> + return "Union '%s' branch" % owner.name
>> + else:
>> + return "Enum '%s' value" % self.name
>
> I like to call it "member" rather than value, because it avoids
> confusion with the numeric value of the C enumeration constant generated
> for it.
>
> The conditional isn't exactly elegant, but it would do. I'm not 100%
> convinced we need it, though. self.info already points to whatever
> defined this enum, either an explicit enum definition, or a simple union
> definition. How do the error messages come out if we dumb down to
> "Member '%s'"?
>
> A method with a similar purpose exists in QAPISchemaObjectTypeMember,
> but it's spelled describe(). It's used only from within the class.
> Rename it to match this one?
[as much notes to myself as anything else...]
We chatted some on IRC, and had some more ideas, thus I will delay
patches 26-28 to a later date while posting the rest of v12. Among
those ideas
- getting rid of the enum _MAX clash will simplify things (to be done
in v12)
- create a new QAPISchemaMember class as the superclass of
QAPISchemaObjectTypeMember, and have enum members be instances of this
type rather than a straight string (at which point the describe() method
and collision checking lives in the superclass, while the associated
type only lives in the subclass). Less code duplication that way
- the idea of multiline error messages, only when info !=
member.owner.info; as in:
foo.json:4: Member 'one' clashes with 'One'
foo.json:2: Member 'one' defined here
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v11 22/28] qapi: Simplify visiting of alternate types, (continued)
- [Qemu-devel] [PATCH v11 28/28] qapi: Detect base class loops, Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 23/28] qapi: Fix alternates that accept 'number' but not 'int', Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 27/28] qapi: Move duplicate enum value checks to schema check(), Eric Blake, 2015/11/11
- [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage, Eric Blake, 2015/11/11
- Re: [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage, Andreas Färber, 2015/11/11
- Re: [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage, Markus Armbruster, 2015/11/11
- Re: [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage, Eric Blake, 2015/11/11
- Re: [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage, Markus Armbruster, 2015/11/11
- Re: [Qemu-devel] [PATCH v11 18/28] qerror: more error_setg() usage, Eric Blake, 2015/11/11
[Qemu-devel] [PATCH v11 21/28] qapi: Convert qtype_code into qapi enum type, Eric Blake, 2015/11/11