Re: [Qemu-devel] [PATCH 2/4] string-input-visitor: Support alternate types

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2767 bytes --]

On 05/04/2017 02:42 PM, Eduardo Habkost wrote:

>>>> As in: we forbid the combination of a scalar (whether 'int', 'number',
>>>> 'bool', and perhaps 'null') with a plain 'str' (since there's no way to
>>>> tell whether '1' should parse as an integer or the string "1"); and
>>>> combining a scalar with an 'enum' requires that all enum members be
>>>> distinct from what could otherwise be parsed as a scalar?
>>>
>>> Exactly.
>>>
>>>>                                                            I can live
>>>> with such a restriction.
>>>
>>> Then let's do it.
>>>
>>> Eduardo, are you comfortable implementing this, or would you like me to
>>> do it?
>>
>> I will give it a try and include it in the next version. Thanks!
> 
> So, I made qapi.py detect ambiguous alternates[1]. The bad news
> is that lots of the alternates in qapi-schema-test.json already
> break those rules.

I'm not surprised, but if test code gets in the way of real life, I'm in
favor of simplifying test code (change what is currently positive tests
of "does this corner case work even though no one uses it" to instead be
negative tests "do we properly reject this alternate as ambiguous").

> This will require changing the schema and
> rewriting tests in test-clone-visitor and qobject-input-visitor.

Yes, and Markus or I could help do some of that work, if you don't feel
up to it.

> 
> I think there's a small risk we will want to support some of
> those forbidden alternate combinations again in the future.

Maybe, but 'git revert' is powerfully easy, and whoever adds the
(now-ambiguous) use case should be able to justify their need at the
time they (re-)add support.

> If
> that happens, detecting them at runtime in string-input-visitor
> will keep us safe.

I still much prefer compile-time rejections ("we don't support this, and
we're telling you up front") over runtime rejections ("it compiled, and
only if you get lucky will you discover that you had a problem").

> 
> I plan to submit v2 with code to detect ambiguous alternates at
> runtime, only, because it seems simpler than rewriting the test
> code.

Simpler, maybe. But harder to maintain, so it may STILL be worth going
the more complex way and updating the testsuite to comply with the new
rules.  Perhaps it can be done as followups (again, where Markus or I
may step in and do the rest on top of your initial work).

> After that, we can still make QAPI reject them at compile
> time too, if we really want to.
> 
> [1] https://github.com/ehabkost/qemu-hacks/commit/cda70a2e1c30c8dadb36fd46095a4f6ee3d89737
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]