Eric Blake <ebl...@redhat.com> writes:
> On 02/16/2016 10:03 AM, Markus Armbruster wrote:
>> Eric Blake <ebl...@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 += ...
Sold.
>
> 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
