Re: [Qemu-devel] [PATCH v8 09/17] qapi: Add positive tests to qapi-schema-test

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3254 bytes --]

On 11/03/2015 09:43 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Add positive tests to qapi-schema-test for things that were
>> made possible by recent patches but which caused compile errors
>> due to collisions prior to that point.
>>
>> This includes:
>> Use of a member 'base' in a struct with a base class
>> Use of a member name ending in 'Kind' or 'List'
>> Use of a type name starting with 'has_'
>> Use of a type named 'u'
>> Use of a union branch name of 'u'
>> Use of a union branch name starting with 'has_'
>> Use of a union branch name of 'type'
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>
>> ---
>> v8: new, but collects portions of subset B v10 patches 2, 3, and
>> 16 and subset C v7 patch 6 that were deferred to later.
>>
>> It might be worth dropping or simplifying this patch, depending
>> on how many corner cases we actually want to test.
> 
> Let's go through your list:
> 
> * Use of a member 'base' in a struct with a base class
> 
>   'base' is no longer special, and testing it is as useful as testing
>   any other non-special member name.  I'd drop it.  Or am I missing
>   something?

Broken prior to commit ddf2190, and no reservation exists. I don't mind
dropping it, especially since it quite distinct from the other grouping
of collision testing.

> 
> * Use of a member name ending in 'Kind' or 'List'
> 
>   These aren't special as member names, but they are reserved type
>   names.  The test ensures we don't accidentally reserve them as member
>   names.  Low probability * low damage = very low risk.  But since you
>   wrote the test already, we can just as well keep it.
> 
> * Use of a type name starting with 'has_'
> * Use of a type named 'u'

Never have been broken, but this adds insurance that we don't break it
(and that our reservations added earlier aren't too greedy).

> * Use of a union branch name of 'u'

Broken prior to commit e4ba22b3; also proves that the reservation in
5359baf is not too greedy.

> * Use of a union branch name starting with 'has_'

Broken prior to commit e4ba22b3; also proves that the reservation in
9fb081e is not too greedy.

> 
>   Similarly: these are only reserved as member names, very low risk, but
>   why not keep the test.
> 
> * Use of a union branch name of 'type'

Broken prior to commit e4ba22b3. Doesn't have a reservation, so...

> 
>   As far as I know, 'type' isn't reserved anywhere, it's just the name
>   of a simple union's implicit tag member.  Testing a tag value doesn't
>   clash with 'type' is as useful as testing any other non-variant member
>   name.  I'd drop it.  Or am I missing something?

...I could indeed drop this part of the test, on the same grounds as
dropping the 'base' non-collision tests.

> 
> Anything else being tested here?
> 

I think that covers it.  There's also one more positive test added in
10/17, when I rework the layout of alternate types (at that point, it is
possible to have an alternate branch named 'max' because it no longer
collides with a generated enum _MAX value).

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]