Re: [Qemu-devel] [PATCH RFC v3 30/32] qapi: New QMP command query-schema for QMP schema introspection

[Eric Blake]

On 08/24/2015 10:55 AM, Markus Armbruster wrote:

> Our motivation for dropping nested structs was to avoid burning the
> 'name': {} struct member syntax on a trivial and rarely used
> convenience, and instead make it available for a way to specify member
> attributes beyond name and type.
> 
> Is there a chance we want to define simple union cases with attributes
> beyond tag value and type?

You may have a valid point there.  It's hard to predict the future, but
leaving dictionary open for future use is the most extensible approach.

But in the patches I'm currently working on, I had only been adding
support for anonymous types for the branches of flat unions; I
intentionally left simple unions to REQUIRE a type name for the branches
(because of the way we create a wrapper type around the single data
member for introspection purposes).

> 
> I think we have a better chance to answer that question after we clean
> non-simple unions.

Well, my proposed cleanup was figuring out a way to explicitly specify
that for a given enum value, we add no additional members to the wire
struct.  But there is a possible alternative syntax for that:

{ 'union': 'Union', 'base': 'Base', 'discriminator': 'type',
  'data': { 'branch1': 'AdditionalMembers',
            'branch2': null } }

which uses 'null' in place of '{}' for the explicitly empty case, while
still requiring a type name for all other branches.  I still think that
requiring a user to explicitly list all branches of a union is a nice
fail-safe (if the enum is extended, we are then reminded to update the
union to match) that we don't currently have.

> 
>>> Both Abort and ChardevDummy exist only because you need a type to
>>> declare a simple union case.  I'd like to explore cleaning up the
>>> convoluted union syntax first.  If we then still have a need for
>>> empty structs, we can consider optimizing them.
>>
>> And that's where my patches were headed - by allowing a dict instead of
>> a type name for the branches of a flat union, the syntax for flat unions
>> becomes simpler, and allows us to sanely represent a
>> "no-additional-members" variant without needing 'Abort' as an empty type.
> 
> Empty cases in flat unions are not a problem: simply don't mention the
> tag value.

But that's opposite of the direction I want to move, where we require
all branches to be listed, but have a clean way to document the branches
that add no additional members to the variant object.

> 
> I'd like to explore doing unions in schema syntax the way we represent
> them internally and in introspection.  Basically get rid of the "need to
> inherit the common members from a base" nonsense.

I've already posted patches that would allow:

{ 'union': 'Union', 'base': { ... }, 'discriminator': 'type',
  'data': { ... } }

that is, allowing the base fields to be specified inline as an anonymous
struct rather than having to create a one-off named struct for that task.

https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg02346.html

But there's still the question of whether we want to always tie the
union branches to an explicitly named enum, or whether it is nice to
preserve the current simple union semantics that an implicit enum is
created to cover all branches when an explicit enum type is not named.
Conversely, I still want to get to the point that even a simple union
can optionally document that it reuses an existing enum (along with the
corresponding qapi-generator enforced rules that all enum branches are
properly covered), rather than always being forced to use an implicit
enum type (where mismatches between an intended explicit enum are too
likely).

> 
> Once that's done, we can decice whether simple unions are still
> worthwhile syntactical sugar.

Agreed - there's still some room to play with things, and flushing our
existing patch queue to have a stable base to play on will make it a bit
nicer.

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

signature.asc
Description: OpenPGP digital signature