On 10/02/2015 01:02 AM, Markus Armbruster wrote: > Eric Blake <[email protected]> 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
