[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 03/17] tests: remove alt num-int cases
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 03/17] tests: remove alt num-int cases |
Date: |
Thu, 11 May 2017 14:34:37 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 05/09/2017 12:35 PM, Marc-André Lureau wrote:
>> There are no real users of this case, and it's going to be invalid after
>> merging of QFloat and QInt use the same QNum type in the following patch.
>>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>> ---
>> tests/test-keyval.c | 3 ---
>> tests/test-qobject-input-visitor.c | 26 --------------------------
>> tests/qapi-schema/qapi-schema-test.json | 2 --
>> tests/qapi-schema/qapi-schema-test.out | 8 --------
>> 4 files changed, 39 deletions(-)
>
> Is it worth adding tests/qapi-schema/alternate-num-int.json that
> attempts to create an alternate on 'int' and 'number' to (eventually)
> show that we give a decent error message at QAPI generation time that
> such a type is invalid? Such a test would (eventually be) a negative
> replacement test for the (currently positive) test that you are deleting
> here, and make it easier to see the point in the series where we flip
> the concept from supported to rejected.
>
> That said, it doesn't have to necessarily be done at this point in the
> series (since I haven't even yet seen how QNum will look), so it doesn't
> hold up review of this particular patch.
>
> Also, this may interact with Eduardo's current attempt (or at least
> start of an attempt) to tighten the QAPI parser to reject alternates
> that would otherwise be ambiguous when passed through the string input
> visitor (for example, forbidding the combination of an enum starting
> with a digit in an alternate that also accepts integers). So be aware
> that we may have some integration things to think about down the road.
>
> Reviewed-by: Eric Blake <address@hidden>
Eduardo's work made me think about alternates, and I've come to the
conclusion that restricting alternates now would be prudent. I'll work
on a patch. It'll probably replace this one.
[Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Marc-André Lureau, 2017/05/09
- Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Markus Armbruster, 2017/05/11
- Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Eric Blake, 2017/05/11
- Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Marc-André Lureau, 2017/05/30
- Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Eric Blake, 2017/05/30
- Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Markus Armbruster, 2017/05/30
- Re: [Qemu-devel] [PATCH 04/17] qapi: merge QInt and QFloat in QNum, Marc-André Lureau, 2017/05/30