Re: qapi-schema esotera

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On 8/3/20 1:25 PM, Eric Blake wrote:
>> On 8/3/20 11:49 AM, John Snow wrote:
>>> UNION is split into two primary forms:
>>>
>>> 1. Simple (No discriminator nor base)
>>> 2. Flat (Discriminator and base)
>>>
>>> In expr.py, I notice that we modify the perceived type of the
>>> 'type' expression based on the two union forms.
>>>
>>> 1a. Simple unions allow Array[T]
>>> 1b. Flat unions disallow Array[T]
>>
>> Rather, branches in a simple unions are syntactic sugar for a
>> wrapper struct that contains a single member 'data'; because of that
>> extra nesting, the type of that single member is unconstrained.  In
>> flat unionw, the type MUST be a QAPI struct, because its members
>> will be used inline; as currently coded, this prevents the use of an
>> intrinsic type ('int', 'str') or an array type.
>>
>
> I meant syntactically here, to be clear. I'm looking at expr.py -- if
> there are deeper constraints on the semantics of the information
> provided, that happens later.
>
> Specifically, check_union's use of check_type() changes depending on
> the form of the union. One allows a string, the other allows a List of
> strings, provided the list is precisely one element long.
>
>> If you need to use an array type in a flat union, you can't do:
>>
>> { 'union' ...
>>    'data': { 'foo': [ 'MyBranch' ] } }
>>
>> but you can provide a wrapper type yourself:
>>
>> { 'struct': 'MyBranch', 'data': { 'array': [ 'MyType' ] } }
>> { 'union' ...
>>    'data': { 'foo': 'MyBranch' } }
>>
>>>
>>>  From the docs:
>>>
>>> Syntax:
>>>      UNION = { 'union': STRING,
>>>                'data': BRANCHES,
>>>                '*if': COND,
>>>                '*features': FEATURES }
>>>            | { 'union': STRING,
>>>                'data': BRANCHES,
>>>                'base': ( MEMBERS | STRING ),
>>>                'discriminator': STRING,
>>>                '*if': COND,
>>>                '*features': FEATURES }
>>>      BRANCHES = { BRANCH, ... }
>>>      BRANCH = STRING : TYPE-REF
>>>             | STRING : { 'type': TYPE-REF, '*if': COND }
>>>
>>> Both arms use the same "BRANCHES" grammar production, which both
>>> use TYPE-REF.
>>>
>>>      TYPE-REF = STRING | ARRAY-TYPE
>>>      ARRAY-TYPE = [ STRING ]
>>>
>>> Implying that List[T] should be allowed for both productions.
>>> Can I ask for a ruling from the judges?
>>
>> As you found, the docs are a bit misleading; the semantic constraint
>> on flat union branches being a struct (because they will be inlined)
>> prevents the use of type-refs that are valid in simple unions (where
>> those simple types will be wrapped in an implicit struct).  A patch
>> to improve the docs would be a reasonable idea.
>>
>
> Yes. I was working on a YAML prototype and I am trying to follow the
> existing parser as closely as possible. In some cases, this highlights
> differences between the grammar as advertised and what the parser
> actually does.

Please report all such differences, so we can fix them.

> If we are to keep the current state of things, splitting UNION into
> two separate productions might be nice.

It *is* two productions, joined with |.

The work unions really, really need is:

* Eliminate the simple union sugar.

* Make flat unions less cumbersome to write.  I'd like to fuse struct
  and union into a single object type, like introspect.json already
  does.

The former is a matter of massaging the schema and simplifying code.
The latter requires actual thought.  No big deal, just takes time, and
time is always in short supply.