On 10/13/2015 12:43 PM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Rename alternate-clash to alternate-clash-members, and add a
>> new test alternate-clash-type.  While similar to the earlier
>> addition of union-clash-type, we have one major difference: a
>> future patch will be simplifying alternates to not need an
>> implict AlternateKind enum, but we still need to detect the
>> collision with the resulting C 'qtype_code type;' tag.
> 
> You're alluding to a future change of the generated code from
> 
>     struct BlockdevRef {
>         BlockdevRefKind kind;
>         union { /* union tag is @kind */
>             void *data;
>             BlockdevOptions *definition;
>             char *reference;
>         };
>     };
> 
> to
> 
>     struct BlockdevRef {
>         qtype_code type;
>         union { /* union tag is @type */
>             void *data;
>             BlockdevOptions *definition; /* QTYPE_QDICT */
>             char *reference;             /* QTYPE_QSTRING */
>         };
>     };
> 
> right?

Yes.

> 
> I don't think that affects collision checking at all.  Both before and
> after, we have a tag member, and we need to check for collisions with
> its name.

I guess I wrote that at one point where I was using
alternate.variants.tag_member = None for alternates; but in the
meantime, I've reworked things to just use a special subclass of
QAPISchemaObjectTypeMember instead, at which point normal collision
checking still works.  So yeah, I can probably simplify or drop wording
here.

>> +++ b/tests/qapi-schema/alternate-clash-type.json
>> @@ -0,0 +1,10 @@
>> +# Alternate branch 'type'
>> +# Reject this, because we would have a clash in generated C, between the
>> +# alternate's implicit tag member 'kind' and the branch name 'kind'
>> +# within the alternate.
>> +# TODO: Even if alternates are simplified in the future to use a simpler
>> +# 'qtype_code type' tag, rather than a full QAPISchemaObjectTypeMember,
>> +# we must still flag the collision, or else munge the generated C branch
>> +# names to allow compilation.
> 
> I don't think there's a TODO here.

Again, probably leftover text from my first implementation.

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