Eric Blake <ebl...@redhat.com> 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 <marcandre.lur...@redhat.com> >> --- >> 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 <ebl...@redhat.com>
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.
