Re: [Qemu-devel] [PATCH v7 03/15] qapi: Require all branches of flat union enum to be covered

[Eric Blake <eblake@redhat.com>]

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

On 06/14/2016 07:46 AM, Eric Blake wrote:

>> I'd let the cases not mentioned default to the empty type (ample
>> precedence in other languages), but I can live with making the user ask
>> for the empty type explicitly.  But we should then make that less
>> cumbersome than now: you have to define an empty struct type, and use
>> that.  Examples of such hoop-jumping: CpuInfoOther, Abort,
>> NetdevNoneOptions.
> 
> Later in the series, I do just that, so that we can write 'other':{}
> instead of 'other':'CpuInfoOther'.
> 
> What I did not do (but maybe should) is make that short syntax possible
> on simple unions (so that we could do 'abort':{} rather than
> 'abort':'Abort') - and merely distinguish that simple unions cannot
> stick anything within the {}, thus leaving non-empty anonymous branches
> only for discriminated unions.  Can do that as a followup or if this
> series needs a respin.

In fact, it turns out that allowing 'abort':{} for 'TransactionAction'
is harder than I thought - we still have to create a wrapper type, and
the wrapper type must still be used to parse:

'action': { 'type':'abort', 'data': {} }

and qapi-types.h would look roughly like:

+struct q_obj_q_empty_wrapper {
+    q_empty *data;
+};
...
 struct TransactionAction {
     TransactionActionKind type;
     union { /* union tag is @type */
         q_obj_BlockdevSnapshot_wrapper blockdev_snapshot;
         q_obj_BlockdevSnapshotSync_wrapper blockdev_snapshot_sync;
         q_obj_DriveBackup_wrapper drive_backup;
         q_obj_BlockdevBackup_wrapper blockdev_backup;
-        q_obj_Abort_wrapper abort;
+        q_obj_q_empty_wrapper abort;

but as I said elsewhere in this thread, we then have to figure out how
to emit 'q_empty' and 'visit_type_q_empty_members()' without running
into duplicate definitions.

For that matter, it might be nice if the parser would allow:

'action': { 'type':'abort' }

that is, treat 'data' as optional if the only thing data would contain
is an empty dictionary - but that would be even trickier to code.

So for v8, I'm not getting rid of any empty types for simple unions.

-- 
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 --]