[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of imp
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type |
Date: |
Fri, 2 Oct 2015 06:54:12 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 |
On 10/02/2015 01:02 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
>
>> A future patch will enable error reporting from the various
>> QAPISchema*.check() methods. But to report an error related
>> to an implicit type, we'll need to associate a location with
>> the type (the same location as the top-level entity that is
>> causing the creation of the implicit type), and once we do
>> that, keying off of whether foo.info exists is no longer a
>> viable way to determine if foo is an implicit type.
>>
>> Instead, add an is_implicit() method to QAPISchemaObjectType,
>> and use that function where needed. (Done at the ObjectType
>> level, since we already know all builtins and arrays are
>> implicit, no commands or events are implicit, and we don't
>> have any differences in generated code for regular vs.
>> implicit enums.)
>> +++ b/scripts/qapi-types.py
>> @@ -234,7 +234,8 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>> self._btin = None
>>
>> def visit_predicate(self, entity):
>> - return not isinstance(entity, QAPISchemaObjectType) or entity.info
>> + return not (isinstance(entity, QAPISchemaObjectType) and
>> + entity.is_implicit())
>
> Aha, now the left hand side becomes necessary to guard the
> .is_implicit(). It stays superfluous if you make .is_implicit() a
> QAPISchemaEntity method.
See discussion on 1/12 - actually, it _becomes_ superfluous in this
patch once we make it a QAPISchemaEntity method.
>> +++ b/scripts/qapi.py
>> @@ -970,12 +970,15 @@ class QAPISchemaObjectType(QAPISchemaType):
>> self.variants.check(schema, members, seen)
>> self.members = members
>>
>> + def is_implicit(self):
>> + return self.name[0] == ':'
Actually, this only works for implicit objects. Implicit enums instead
have self.name[-4:] == 'Kind'. But qapi-types cares about implicit
objects only. So if I hoist this, I may need something like:
def is_implicit(self, type=None):
if type and not isinstance(self, type):
return Fals
if isinstance(self, QAPISchemaObjectType):
return self.name[0] == ':'
if isinstance(self, QAPISchemaEnumType):
return self.name[-4:] == 'Kind'
return False
where qapi-types would call entity.is_implicit(QAPISchemaObjectType).
>> +
>
> If this test is here to stay, perhaps add a comment pointing to
> _make_implicit_object_type().
Indeed.
>> @@ -1043,7 +1046,8 @@ class
>> QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
>> # This function exists to support ugly simple union special cases
>> # TODO get rid of them, and drop the function
>> def simple_union_type(self):
>> - if isinstance(self.type, QAPISchemaObjectType) and not
>> self.type.info:
>> + if isinstance(self.type,
>> + QAPISchemaObjectType) and self.type.is_implicit():
>
> I figure you break this line in the argument list to avoid a backslash
> for line continuation. I know PEP8 doesn't like them, but I like line
> breaks away from top level operators even less. I feel it should be
> broken after the and operator, even though that'll require wrapping the
> condition in parenthesis.
Sadly, this form causes pep8 to complain about continuation at the same
indentation as the body:
if (cond1 and
cond2):
body
But it seems that pep8 and pylint don't mind the backslash in:
if cond1 and \
cond2:
body
where the indentation is okay. I guess trying to avoid the \ is not
worth it, if the tools don't complain about it, and that this was a case
of me prematurely guessing (incorrectly) about what the tools don't like.
--
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 v6 07/12] qapi: Detect collisions in C member names, (continued)
[Qemu-devel] [PATCH v6 06/12] qapi: Track owner of each object member, Eric Blake, 2015/10/08
[Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type, Eric Blake, 2015/10/08
- Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type, Markus Armbruster, 2015/10/08
- Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type,
Eric Blake <=
- Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type, Markus Armbruster, 2015/10/08
- Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type, Eric Blake, 2015/10/08
- Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type, Markus Armbruster, 2015/10/08
- Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type, Eric Blake, 2015/10/08
- Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type, Markus Armbruster, 2015/10/08
- Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type, Eric Blake, 2015/10/08
- Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type, Markus Armbruster, 2015/10/08
Re: [Qemu-devel] [PATCH v6 02/12] qapi: Don't use info as witness of implicit object type, Eric Blake, 2015/10/08
[Qemu-devel] [PATCH v6 05/12] qapi: Track location that created an implicit type, Eric Blake, 2015/10/08