Re: [Qemu-devel] [PATCH v10 02/13] qapi: Forbid empty unions and useless alternates

[Eric Blake <eblake@redhat.com>]

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

On 02/16/2016 09:08 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Empty unions serve no purpose, and while we compile with gcc
>> which permits them, strict C99 forbids them.  We could inject
>> a dummy member (and in fact, we do for empty structs), but while
> 
> gen_variants() injects void *data. 
> 
>> empty structs make sense in qapi,
> 
> Suggest to cut the paragaph until here.

Side effect of rebasing - I originally had this patch after one that
deletes the 'data' member, but that requires a few other patches.  I'll
reword it to mention that we want to delete 'data', at which point we
would be left with an empty union if we didn't prohibit it at parse time.

>> @@ -613,7 +616,11 @@ def check_alternate(expr, expr_info):
>>      members = expr['data']
>>      types_seen = {}
>>
>> -    # Check every branch
>> +    # Check every branch; require at least two branches
>> +    if len(members) < 2:
>> +        raise QAPIExprError(expr_info,
>> +                            "Alternate '%s' should have at least two branches "
>> +                            "in 'data'" % name)
> 
> This is stricter than the commit message announced.  You can either
> relax to at least one branch, or amend the commit message.

Will amend; a later patch actually relies on the 2-or-more promise for
alternates, so it is worth documenting as intentional.

>>  A simple union type defines a mapping from automatic discriminator
>>  values to data types like in this example:
> 
> Missing: update of section "Alternate types".

Already done there (commit 7b1b98c4):

"An alternate type is one that allows a choice between two or more JSON
data types (string, integer, number, or object, but currently not..."

I didn't see anything worth rewording.

> 
> [tests/ diff looks good]
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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