[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check() |
Date: |
Tue, 13 Oct 2015 17:39:05 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 10/13/2015 01:08 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> On 10/12/2015 09:53 AM, Markus Armbruster wrote:
>> [...]
>>>> It would be simpler if we could make C clash only when QMP clashes.
>>>
>>> If a QMP clash always caused a C clash, life would be a bit simpler. We
>>> aren't going to get there (because in a flat union, hiding the members
>>> of the branch type behind a single pointer means those members don't
>>> clash with any C names of the overall union), but I can certainly try to
>>> make the comments explain what is going on.
>>
>> (1) If QMP clashed exactly when C clashed, we'd have to think only about
>> one of them, and the C compiler would check for clashes statically.
>
> Although deferring the error about the clash until compile time is a bit
> long, compared to reporting the clash at generator time.
I'd view the C compiler as insurance. The generators should still
check.
>> (2) If a QMP clash implied a C clash, we'd only have to think about C,
>> and the C compiler would check statically.
>>
>> (3) If a C clash implied a QMP clash, we'd only have to think about QMP.
>>
>> (4) If they can clash independently, we need to think about both
>> independently.
>>
>> We currently have (4), and having to juggle both QMP and C in my mind
>> has been taxing.
>>
>> My remark was about (3): C clashes only when QMP clashes <=> C clash
>> implies QMP clash.
>>
>> Your remark was about (2). More useful than (3), but as you say not
>> feasible.
>>
>> Do you think we can get to (3)?
>
> C clash that does not imply a QMP clash:
>
> { 'union':'U', 'data':{ 'a':'int', 'A':'str' } }
>
> C clash (the implicit enum has two U_KIND_A values) but not a QMP clash
> ('a' and 'A' don't even appear in QMP; and even if they did, they are
> distinct in C as branch names). Might be possible to avoid the C clash
> by munging case labels of the generated enum differently, but I'm not
> sure it is worth the effort.
>
> QMP clash that does not (currently) imply a C clash (using abbreviated
> notation):
>
> { 'union':'U', 'base':{ 'type':'Enum', 'member':'int' },
> 'discriminator':'type',
> 'data':{ 'branch':{ 'member':'str' } } }
>
> QMP clash (the field 'member' is part of the base type, and also part of
> the 'branch' variant type), but not a C clash (because the C layout
> accesses the branch through a box named 'branch').
>
> But we cannot just remove the boxing, either, because then we'd have a
> clash in C that is NOT a clash in QMP:
>
> { 'union':'U', 'base':{ 'type':'Enum' }, 'discriminator':'type',
> 'data':{ 'branch1':{ 'member':'str' }, 'branch2': { 'member':'int' } } }
>
> If the two 'member' fields were at the same C level as 'type', then we
> could use C collisions to detect a QMP clash between a variant's members
> and the non-variant fields - but it also has the drawback of declaring a
> C collision between the two branches.
>
> So no, I don't think we can get to (3).
Scratch that then. My head hurts.
- Re: [Qemu-devel] [PATCH v7 08/14] qapi: Track location that created an implicit type, (continued)
[Qemu-devel] [PATCH v7 10/14] qapi: Detect collisions in C member names, Eric Blake, 2015/10/08
[Qemu-devel] [PATCH v7 14/14] qapi: Detect base class loops, Eric Blake, 2015/10/08