Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions

[Eric Blake]

On 09/29/2015 04:21 PM, Eric Blake wrote:
> Expose some weaknesses in the generator: we don't always forbid
> the generation of structs that contain multiple members that map
> to the same C or QMP name.  This has already been marked FIXME in
> qapi.py in commit d90675f, but having more tests will make sure
> future patches produce desired behavior; and updating existing
> patches to better document things doesn't hurt, either.  Some of
> these collisions are already caught in the old-style parser
> checks, but ultimately we want all collisions to be caught in the
> new-style QAPISchema*.check() methods.
> 

> diff --git a/tests/qapi-schema/union-clash-branches.err 
> b/tests/qapi-schema/union-clash-branches.err
> new file mode 100644
> index 0000000..005c48d
> --- /dev/null
> +++ b/tests/qapi-schema/union-clash-branches.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/union-clash-branches.json:4: Union 'TestUnion' member 
> 'a_b' clashes with 'a-b'
> diff --git a/tests/qapi-schema/union-clash-branches.exit 
> b/tests/qapi-schema/union-clash-branches.exit
> new file mode 100644
> index 0000000..d00491f
> --- /dev/null
> +++ b/tests/qapi-schema/union-clash-branches.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/union-clash-branches.json 
> b/tests/qapi-schema/union-clash-branches.json
> new file mode 100644
> index 0000000..31d135f
> --- /dev/null
> +++ b/tests/qapi-schema/union-clash-branches.json
> @@ -0,0 +1,5 @@
> +# Union branch name collision
> +# Reject a union that would result in a collision in generated C names (this
> +# would try to generate two enum values 'TEST_UNION_KIND_A_B').
> +{ 'union': 'TestUnion',
> +  'data': { 'a-b': 'int', 'a_b': 'str' } }

Hmm. This test is very similar to the existing union-bad-branch (I guess
it's poor name is why I didn't notice it before). But that test only
covered 'one' vs. 'ONE' (no clash in the C struct, just in the generated
MyUnionKind enum type); while this test also clashes in the C struct.
Don't know if it is worth a v8 to clean up the duplication, or if we
just save it for a followup patch (namely, where I try to move errors
into the QAPISchema.check() methods); I found the issue while playing
with v5 15/46.

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

signature.asc
Description: OpenPGP digital signature