[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions |
Date: |
Thu, 01 Oct 2015 14:57:20 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> 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.
Your choice. The rather minor issues I found in this series could all
be touched up on commit (assuming consensus on what to do).
- [Qemu-devel] [PATCH v7 14/18] qapi: Consistent generated code: minimize push_indent() usage, (continued)
- [Qemu-devel] [PATCH v7 14/18] qapi: Consistent generated code: minimize push_indent() usage, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 15/18] qapi: Share gen_err_check(), Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 02/18] qapi: Improve 'include' error message, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 12/18] qapi: Consistent generated code: prefer common labels, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 04/18] qapi: Clean up qapi.py per pep8, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 07/18] qapi: Add tests for empty unions, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions, Eric Blake, 2015/10/08
- Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions, Eric Blake, 2015/10/08
- Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions,
Markus Armbruster <=
- Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions, Markus Armbruster, 2015/10/08
- Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions, Eric Blake, 2015/10/08
- Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions, Markus Armbruster, 2015/10/08
- Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions, Eric Blake, 2015/10/08
- Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions, Markus Armbruster, 2015/10/08
- Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions, Eric Blake, 2015/10/08
- Re: [Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions, Markus Armbruster, 2015/10/08
[Qemu-devel] [PATCH v7 13/18] qapi: Consistent generated code: prefer common indentation, Eric Blake, 2015/10/08
[Qemu-devel] [PATCH v7 11/18] qapi: Consistent generated code: prefer visitor 'v', Eric Blake, 2015/10/08
[Qemu-devel] [RFC PATCH v7 18/18] qapi: Use gen_err_check() in more places, Eric Blake, 2015/10/08