Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check()

[Markus Armbruster]

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.