Re: [Qemu-devel] [PATCH v6 04/12] qapi: Create simple union type member earlier

[Eric Blake]

On 10/02/2015 02:34 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> For simple unions, we were creating the implicit 'type' tag
>> member during the QAPISchemaObjectTypeVariants constructor.
>> This is different from every other implicit QAPISchemaEntity
>> object, which get created by QAPISchema methods.  Hoist the
>> creation to the caller, and pass the entity rather than the
>> string name, so that we have the nice property that no
>> entities are created as a side effect within a different
>> entity.  A later patch will then have an easier time of
>> associating location info with each entity creation.
>>
>> No change to generated code.
>>
>> Signed-off-by: Eric Blake <address@hidden>
>>

>>  class QAPISchemaObjectTypeVariants(object):
>> -    def __init__(self, tag_name, tag_enum, variants):
>> +    def __init__(self, tag_name, tag_member, variants):
>>          assert tag_name is None or isinstance(tag_name, str)
>> -        assert tag_enum is None or isinstance(tag_enum, str)
>> +        assert (tag_member is None or
>> +                isinstance(tag_member, QAPISchemaObjectTypeMember))
>>          for v in variants:
>>              assert isinstance(v, QAPISchemaObjectTypeVariant)
>>          self.tag_name = tag_name
>>          if tag_name:
>> -            assert not tag_enum
>> -            self.tag_member = None
>> -        else:
>> -            self.tag_member = QAPISchemaObjectTypeMember('type', tag_enum,
>> -                                                         False)
>> +            assert tag_member is None
> 
> Since the conditional degenerates to just checking the "either tag_name
> of tag_member" precondition, let's move it to right after the checking
> of tag_name and tag_member, i.e. before the loop.
> 
> I'd also simplify to
> 
>         assert not tag_name != not tag_member
> 
> or
> 
>         bool(tag_name) != bool(tag_member)

Sure, that works.  I'll also add comments on the preconditions.

>> @@ -1226,8 +1224,9 @@ class QAPISchema(object):
>>          return QAPISchemaObjectTypeVariant(case, typ)
>>
>>      def _make_tag_enum(self, type_name, variants):
>> -        return self._make_implicit_enum_type(type_name,
>> -                                             [v.name for v in variants])
>> +        typ = self._make_implicit_enum_type(type_name,
>> +                                            [v.name for v in variants])
>> +        return QAPISchemaObjectTypeMember('type', typ, False)
> 
> I think this function should now be called _make_tag_member(), or
> _make_implicit_tag_member(), or _make_implicit_tag().
> 

Makes sense.  I'll probably use _make_implicit_tag().

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

signature.asc
Description: OpenPGP digital signature