[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaV
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs |
Date: |
Wed, 29 Jul 2015 10:00:54 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> 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.
Good. Thank you!
- [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, (continued)
- [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, Markus Armbruster, 2015/07/01
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, Eric Blake, 2015/07/22
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, Markus Armbruster, 2015/07/27
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, Eric Blake, 2015/07/27
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, Markus Armbruster, 2015/07/28
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, Eric Blake, 2015/07/28
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, Markus Armbruster, 2015/07/29
Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, Eric Blake, 2015/07/27
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, Markus Armbruster, 2015/07/28
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, Eric Blake, 2015/07/28
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH RFC v2 27/47] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs, Eric Blake, 2015/07/29
[Qemu-devel] [PATCH RFC v2 24/47] tests/qapi-schema: Convert test harness to QAPISchemaVisitor, Markus Armbruster, 2015/07/01
[Qemu-devel] [PATCH RFC v2 41/47] qom: Don't use 'gen': false for qom-get, qom-set, object-add, Markus Armbruster, 2015/07/01
[Qemu-devel] [PATCH RFC v2 33/47] qapi: Clean up after recent conversions to QAPISchemaVisitor, Markus Armbruster, 2015/07/01