Re: QAPI unions as branches / unifying struct and union types

[Het Gala <het.gala@nutanix.com>]

On 14/02/23 3:46 pm, Markus Armbruster wrote:
> Het Gala <het.gala@nutanix.com> writes:
>
>> On 10/02/23 12:54 pm, Markus Armbruster wrote:
>>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>>
>>> [...]
>>>
>>>>> +##
>>>>> +# @MigrateAddress:
>>>>> +#
>>>>> +# The options available for communication transport mechanisms for migration
>>>>> +#
>>>>> +# Since 8.0
>>>>> +##
>>>>> +{ 'union' : 'MigrateAddress',
>>>>> +  'base' : { 'transport' : 'MigrateTransport'},
>>>>> +  'discriminator' : 'transport',
>>>>> +  'data' : {
>>>>> +    'socket' : 'MigrateSocketAddr',
>>>>> +    'exec' : 'MigrateExecAddr',
>>>>> +    'rdma': 'MigrateRdmaAddr' } }
>>>> Ideally this would be
>>>>
>>>>      'data' : {
>>>>        'socket' : 'SocketAddress',
>>>>        'exec' : 'MigrateCommand',
>>>>        'rdma': 'InetSocketAddress' } }
>>>>
>>>> though the first SocketAddress isn't possible unless it is easy to
>>>> lift the QAPI limitation.
>>> Context: SocketAddress is a QAPI union, and "the QAPI limitation" is
>>>
>>>       scripts/qapi-gen.py: In file included from ../qapi/qapi-schema.json:79:
>>>       ../qapi/migration.json: In union 'MigrateAddress':
>>>       ../qapi/migration.json:1505: branch 'socket' cannot use union type 'SocketAddress'
>>>
>>> Emitted by schema.py like this:
>>>
>>>                   if (not isinstance(v.type, QAPISchemaObjectType)
>>>                           or v.type.variants):
>>>                       raise QAPISemError(
>>>                           self.info,
>>>                           "%s cannot use %s"
>>>                           % (v.describe(self.info), v.type.describe()))
>>>
>>> This enforces docs/devel/qapi-code-gen.rst's clause
>>>
>>>       The BRANCH's value defines the branch's properties, in particular its
>>>       type.  The type must a struct type.  [...]
>>>
>>> Next paragraph:
>>>
>>>       In the Client JSON Protocol, a union is represented by an object with
>>>       the common members (from the base type) and the selected branch's
>>>       members.  The two sets of member names must be disjoint.
>>>
>>> So, we're splicing in the members of the branch's JSON object.  For that
>>> to even make sense, the branch type needs to map to a JSON object.  This
>>> is fundamental.  It's the first part of the condition in the code
>>> snippet above.
>>>
>>> We have two kinds of QAPI types that map to a JSON object: struct and
>>> union.  The second part of the condition restricts to struct.  Unless
>>> I'm missing something (imperfect memory...), this is *not* fundamental,
>>> just a matter of implementing it.  But I'd have to try to be sure.
>>>
>>>
>>> Instead of simply allowing unions in addition to structs here, I'd like
>>> to go one step further, and fuse the two into "objects".  Let me
>>> explain.
>>>
>>> If we abstract from syntax, structs have become almost a special kind of
>>> union.  Unions have a set of common members and sets of variant members,
>>> and a special common member (the tag) selects the set of variant
>>> members.  Structs are unions with zero variants and no tag.
>>>
>>> The generator code actually represents both structs and unions as a
>>> common QAPISchemaObjectType already.  QAPI/QMP introspection does the
>>> same: it uses a single meta type 'object' for both.
>>>
>>>
>>> There is another spot where only structs are allowed: a struct or
>>> union's base type.  That restriction will be awkward to lift, as I made
>>> the mistake of baking the assumption "object type has at most one tag
>>> member" into QAPI/QMP introspection .
>> Hi Markus, thankyou for explaning in such detail. I tried to understand of what you explained.
>>
>> So IIUC, you mentioned the QAPI generator treats both structs and unions same, but basically in the schema.py checks is where it tries to distinguish between the two ? and because of the fact that docs/devel/qapi-code-gen.rst states that for a union, it's branches must be 'struct', and that's the reason it gives an error ?
> Permit me a brief digression into history.
>
> The initial QAPI design language provided product types (structs) and
> sum types (unions containing exactly one of several types, and a tag
> member that tells which one).  The two are orthogonal.
>
> These unions turned out rather awkward.
>
> The unions we have today are more general.  They have common members,
> and one of them is the tag member, of enumeration type.  For each tag
> value, they have variant members.  Both the common members and each tag
> value's variant members are given as struct types.
>
> What if the tag's enumeration type is empty, i.e. has no values?  We get
> a union with no variant members, only common ones.  Isn't that a struct?
>
> Not quite.  To get a struct, we also have to drop the tag member.  It
> has no possible values anyway.
>
> You see, struct types are almost a special case of today's union types.
> To overcome "almost", we can introduce the notion of "object type":
>
> * An object type has common members, one of them can be a tag member, of
>    enumeration type, not empty.  For each tag value, it additionally has
>    variant members.
>
> * A union type is an object type with a tag member and variant members.
>
> * A struct type is an object type without tag member and variant
>    members.
>
> The QAPI generator code already made the jump to this object type
> notion.  It transform the special cases into the general case at first
> opportunity, in QAPISchema._def_struct_type() and ._def_union_type().
>
> *Except* we haven't implemented support for variant members in a few
> places where they cannot occur now, e.g. as a tag value's variant.  This
> is the restriction you ran into.
>
> I'd like to make the jump to object type in the QAPI schema language,
> too.  But that's not a prerequisite to lifting the restriction.
>
>> If that's the case, can we improve on our checks and allow union as a part of branch of a union ? or something else ?
> I believe we can implement the missing parts and relax the checks.  But
> to be sure, we need to try.
>
>> or I may have completely misunderstood most of the part 😅. Please let me know
> More questions?

Completely understood everything. Thankyou for the wonderful 
explanation. Looking forward to implement the missing parts in QAPI 
schema language.

Regards,
Het Gala