Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs

[Eric Blake]

On 07/28/2015 12:44 AM, Markus Armbruster wrote:

>>
>>> +def gen_visit_union(name, base, variants):
>>> +    ret = ''
>>>  
>>>      if base:
>>> -        assert discriminator
>>> -        base_fields = find_struct(base)['data'].copy()
>>> -        del base_fields[discriminator]
>>> -        ret += generate_visit_struct_fields(name, base_fields)
>>> +        members = [m for m in base.members if m != variants.tag_member]
>>> +        ret += generate_visit_struct_fields(name, members)
>>
>> Ouch. This hurts.  If the same class is used as both the base class of a
>> flat union, and the base class of an ordinary struct, then the struct
>> tries to visit the base class, but no longer parses the field that the
>> union was using as its discriminator.
>>
>> We don't have any code that demonstrates this, but probably should.  I
>> ran into it while working up my POC of what it would take to unbox
>> inherited structs (http://thread.gmane.org/gmane.comp.emulators.qemu/353204)
> 
> Is this broken in master, or do my patches break it?
> 
> Got a reproducer?

Turns out I'm mistaken; we got lucky.  The call to
generate_visit_struct_fields() creates a function for 'name' (aka the
union name), and not for 'base' (aka the class name that owns the
fields).  So even if we have Base as a common struct between Child and
Union, the code emitted for Child generates visit_Base_fields(), while
the code emitted for Union generates visit_Union_fields().

So there is no reproducer, but _only_ as long as we reject unions as a
base class for any other object.  And there is redundancy: we could
reuse visit_Base_fields() for the sake of the union, then avoid
(re-)visiting the discriminator, and we would no longer need to emit
visit_Union_fields().  But I can do that as part of the followup
cleanups; since I don't see anything broken with your patch, we don't
have to worry about it during this series.

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

signature.asc
Description: OpenPGP digital signature