[revisiting this series, finally!] On 09/25/2014 10:19 AM, Markus Armbruster wrote:
>> 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.
In trying to code up what it would take to easily reject anonymous
unions from a return type, I'm realizing that it would be smarter to
quit mixing anonymous unions in with other unions.
Refresher course: on the wire, both a regular union:
QAPI:
{ 'type': 'Type1', 'data': { 'value': 'int' } }
{ 'union': 'Foo', 'data': { 'a': 'Type1', 'b': 'Type2' } }
{ 'command': 'bar', 'data': 'Foo' }
Wire:
{ "execute": "bar", "arguments": { "type": "a",
"data": { "value": 1 } } }
and a flat union:
QAPI:
{ 'type': 'Type1', 'data': { 'value': 'int' } }
{ 'enum': 'Enum', 'data': [ 'a', 'b' ] }
{ 'type': 'Base', { 'switch': 'Enum' } }
{ 'union': 'Foo', 'base': 'Base', 'discriminator': 'switch',
'data': { 'a': 'Type1', 'b': 'Type2' } }
{ 'command': 'bar', 'data': 'Foo' }
Wire:
{ "execute": "bar", "arguments": { "switch": "a",
"value": 1 } }
happen to guarantee a top-level dictionary (plain unions always have a
two-element dictionary, flat unions are required to have a base type
which is itself a dictionary). But an anonymous union is explicitly
allowing a multi-type variable, where the determination of which branch
of the union is made by the type of the variable. Furthermore, we do
not allow two branches to have the same type, so at least one branch
must be a non-dictionary; but as _all_ QMP commands currently take a
dictionary for the "arguments" key, we do not want to allow:
QAPI:
{ 'type': 'Type1', 'data': { 'value': 'int' } }
{ 'union': 'Foo', 'discriminator': {},
'data': { 'a': 'Type1', 'b': 'int' } }
{ 'command': 'bar', 'data': 'Foo' }
Wire:
{ "execute": "bar", "arguments": 1 }
Tracking all three qapi expressions as union types is making the
generator code a bit verbose, especially since the code generation for
all three is so distinct.
Proposal: I am proposing that we convert to an alternate syntax for what
we now term as anonymous unions. It will not have any impact to the
wire representation of QMP, only to the qapi code generators. The
proposal is simple: instead of using "'union':'Name',
'discriminator':{}", we instead use "'alternate': 'Foo'" when declaring
a type as an anonymous union (which, for obvious reasons, I would then
update the documentation to call an "alternate" instead of an "anonymous
union").
I'll go ahead and propose the patches (I've already done the bulk of the
conversion work, to prove that not many files were affected).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
