Re: [Qemu-devel] [PATCH v4 12/19] qapi: Add some type check tests

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 09/25/2014 02:06 AM, Markus Armbruster wrote:
>
>>>
>>> The QAPI schema's 'returns' becomes "return" on the wire.  We suck.
>>>
>>> qmp-spec.txt is *wrong*!  We actually use json-array in addition to
>>> json-object.
>> 
>> Actually, we use json-int and json-str as well:
>> query-migrate-cache-size, ringbuf-read, human-monitor-command.
>> 
>>> Similar argument on types wanted as for 'data' / "arguments" above.  I
>>> think we should permit exactly the same QAPI types, plus lists.
>> 
>> The similarity to 'data' just isn't there.  Separate analysis needed.
>
> Correct.  'data' and 'returns' are different beasts when it comes to
> acceptable types.  And different still from the acceptable type of each
> member of a dictionary.  But my check_type function in 13/19 is flexible
> enough to cover all the cases.
>
>> 
>> Any QAPI types that don't make sense, other than list with length != 1?
>
> Return of an anon union isn't used yet, but _might_ make sense (as the
> only feasible way of changing existing commands that return an array or
> primitive extensible to instead return a dict) - 

Good point.

>                                                  except that back-compat
> demands that we can't return a dict in place of a primitive unless the
> arguments of the command are also enhanced (that is, older callers are
> not expecting a dict, so we can't return a dict unless the caller
> witnesses they are new enough by explicitly asking for a dict return).

I think we can keep things simple for now and reject anonymous unions.
We can always relax the check when we run into a use.

You're giving the generator a good shove from "god knows what it
accepts, but as long as you stick to stuff that is being used already,
probably generates something that works" towards "if it accepts it, it
works".  I like it.