qemu-devel
[Top][All Lists]
Advanced

[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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]