Re: [Qemu-devel] [PATCH v10 09/13] qapi: Emit structs used as variants in topological order

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2884 bytes --]

On 02/16/2016 10:03 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Right now, we emit the branches of union types as a boxed pointer,
>> and it suffices to have a forward declaration of the type.  However,
>> a future patch will swap things to directly use the branch type,
>> instead of hiding it behind a pointer.  For this to work, the
>> compiler needs the full definition of the type, not just a forward
>> declaration, prior to the union that is including the branch type.
>> This patch just adds topological sorting to hoist all types
>> mentioned in a branch of a union to be fully declared before the
>> union itself.  The sort is always possible, because we do not
>> allow circular union types that include themselves as a direct
>> branch (it is, however, still possible to include a branch type
>> that itself has a pointer to the union, for a type that can
>> indirectly recursively nest itself - that remains safe, because
>> that the member of the branch type will remain a pointer, and the
>> QMP representation of such a type adds another {} for each recurring
>> layer of the union type).
>>

>> +    ret = ''
>> +    if variants:
>> +        for v in variants.variants:
>> +            if isinstance(v.type, QAPISchemaObjectType) and \
>> +               not v.type.is_implicit():
>> +                ret += gen_object(v.type.name, v.type.base,
>> +                                  v.type.local_members, v.type.variants)
> 
> PEP 8:
> 
>     The preferred way of wrapping long lines is by using Python's
>     implied line continuation inside parentheses, brackets and
>     braces. Long lines can be broken over multiple lines by wrapping
>     expressions in parentheses. These should be used in preference to
>     using a backslash for line continuation.
> 
> In this case:
> 
>                if (isinstance(v.type, QAPISchemaObjectType) and
>                    not v.type.is_implicit()):

pep8 silently accepted my version, but complains about yours:

scripts/qapi-types.py:65:5: E129 visually indented line with same indent
as next logical line

So the compromise for both of us is added indentation:

        if (isinstance(v.type, QAPISchemaObjectType) and
                not v.type.is_implicit()):
            ret += ...

Or, I could revisit my earlier proposal of:

v.type.is_implicit(QAPISchemaObjectType)

of giving .is_implicit() an optional parameter; if absent, all types are
considered, but if present, the predicate is True only if the type of
the object being queried matches the parameter type name.

Here's the last time we discussed the tradeoffs of the shorter form:
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg02272.html

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]