Re: [Qemu-devel] [PATCH v5 04/46] qapi: Add tests for empty unions

[Eric Blake]

On 09/24/2015 08:16 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> The documentation claims that alternates are useful for
>> allowing two types, although nothing enforces this.  Meanwhile,
>> it is silent on whether empty unions are allowed.  In practice,
>> the generated code will compile, in part because we have a
>> 'void *data' branch; but attempting to visit such a type will
>> cause an abort().  Add some tests to expose the problems, and
> 
> There's nothing really wrong with degenerate alternates or unions, but I
> don't want to spend time on making them work.  Outlawing them will do.
> The commit message could perhaps be rephrased a bit to better convey
> that.

Sure, that sounds like a better wording to weave in.

> 
> What about empty structs and enums?

Empty structs: required support (for example, 'Abort' as a member of
'transaction's union).

Empty enums: Documented as not very useful but currently supported, and
we already have code that tests that we don't choke on them.

> 
>> adjust existing tests that should be failing for other reasons.
> 
> I had to read this a few times until I understood "should be failing for
> other reasons" means something like "are meant to test something else,
> but could fail for the wrong reason if we reject degenerate alternates /
> unions".

I like it.

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

signature.asc
Description: OpenPGP digital signature