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