qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: kwolf@redhat.com, berto@igalia.com, qemu-devel@nongnu.org,
	mdroth@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [PATCH RFC v3 30/32] qapi: New QMP command query-schema for QMP schema introspection
Date: Mon, 24 Aug 2015 11:14:31 -0600	[thread overview]
Message-ID: <55DB50F7.8040909@redhat.com> (raw)
In-Reply-To: <87twror5ha.fsf@blackfin.pond.sub.org>

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

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  reply	other threads:[~2015-08-24 17:14 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-04 15:57 [Qemu-devel] [PATCH RFC v3 00/32] qapi: QMP introspection Markus Armbruster
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 01/32] qapi: Rename class QAPISchema to QAPISchemaParser Markus Armbruster
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 02/32] qapi: New QAPISchema intermediate reperesentation Markus Armbruster
2015-08-04 21:53   ` Eric Blake
2015-08-05  6:23     ` Markus Armbruster
2015-08-05 14:27       ` Eric Blake
2015-08-06  5:46         ` Markus Armbruster
2015-08-31 18:09           ` Markus Armbruster
2015-09-03 14:52             ` Eric Blake
2015-09-03 16:13               ` Markus Armbruster
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 03/32] qapi: QAPISchema code generation helper methods Markus Armbruster
2015-08-04 22:18   ` Eric Blake
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 04/32] qapi: New QAPISchemaVisitor Markus Armbruster
2015-08-04 22:26   ` Eric Blake
2015-08-05  6:24     ` Markus Armbruster
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 05/32] tests/qapi-schema: Convert test harness to QAPISchemaVisitor Markus Armbruster
2015-08-04 22:35   ` Eric Blake
2015-08-05  6:26     ` Markus Armbruster
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 06/32] qapi: Split up some typedefs to ease review Markus Armbruster
2015-08-04 22:37   ` Eric Blake
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 07/32] qapi: Generate comments to simplify splitting for review Markus Armbruster
2015-08-04 22:54   ` Eric Blake
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 08/32] Revert "qapi: Generate comments to simplify splitting for review" Markus Armbruster
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 09/32] Revert "qapi: Split up some typedefs to ease review" Markus Armbruster
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 10/32] qapi-types: Convert to QAPISchemaVisitor, fixing flat unions Markus Armbruster
2015-08-05 15:15   ` Eric Blake
2015-08-06  5:50     ` Markus Armbruster
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 11/32] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs Markus Armbruster
2015-08-05 16:03   ` Eric Blake
2015-08-06 22:53   ` Eric Blake
2015-08-08  6:07     ` Markus Armbruster
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 12/32] qapi-commands: Convert to QAPISchemaVisitor Markus Armbruster
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 13/32] qapi: De-duplicate enum code generation Markus Armbruster
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 14/32] qapi-event: Eliminate global variable event_enum_value Markus Armbruster
2015-08-04 15:57 ` [Qemu-devel] [PATCH RFC v3 15/32] qapi-event: Convert to QAPISchemaVisitor, fixing data with base Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 16/32] qapi: Generate comments to simplify splitting for review Markus Armbruster
2015-08-04 23:02   ` Eric Blake
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 17/32] Revert "qapi: Generate comments to simplify splitting for review" Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 18/32] qapi: Replace dirty is_c_ptr() by method c_null() Markus Armbruster
2015-08-05 16:13   ` Eric Blake
2015-08-06  5:52     ` Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 19/32] qapi: Clean up after recent conversions to QAPISchemaVisitor Markus Armbruster
2015-08-05 16:29   ` Eric Blake
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 20/32] qapi-visit: Rearrange code a bit Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 21/32] qapi-commands: Rearrange code Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 22/32] qapi: Rename qmp_marshal_input_FOO() to qmp_marshal_FOO() Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 23/32] qapi: De-duplicate parameter list generation Markus Armbruster
2015-08-05 17:00   ` Eric Blake
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 24/32] qapi-commands: De-duplicate output marshaling functions Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 25/32] qapi: Improve built-in type documentation Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 26/32] qapi: Introduce a first class 'any' type Markus Armbruster
2015-08-07 19:30   ` Eric Blake
2015-08-08  6:24     ` Markus Armbruster
2015-08-31  9:07       ` Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 27/32] qom: Don't use 'gen': false for qom-get, qom-set, object-add Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 28/32] qapi-schema: Fix up misleading specification of netdev_add Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 29/32] qapi: Pseudo-type '**' is now unused, drop it Markus Armbruster
2015-08-05 17:13   ` Eric Blake
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 30/32] qapi: New QMP command query-schema for QMP schema introspection Markus Armbruster
2015-08-05 20:20   ` Eric Blake
2015-08-06  6:23     ` Markus Armbruster
2015-09-01 13:09       ` Markus Armbruster
2015-09-01 15:48         ` Eric Blake
2015-08-05 20:54   ` Eric Blake
2015-08-06  6:47     ` Markus Armbruster
2015-08-23  4:17   ` Eric Blake
2015-08-24 11:30     ` Markus Armbruster
2015-08-24 13:07       ` Eric Blake
2015-08-24 16:51         ` Eric Blake
2015-08-24 16:55         ` Markus Armbruster
2015-08-24 17:14           ` Eric Blake [this message]
2015-08-25  8:33             ` Markus Armbruster
2015-09-01 18:40   ` Michael Roth
2015-09-01 19:12     ` Eric Blake
2015-09-01 20:23       ` Michael Roth
2015-09-02  8:56         ` Markus Armbruster
2015-09-02 16:21           ` Michael Roth
2015-09-03  9:26             ` Markus Armbruster
2015-09-03 14:31               ` Eric Blake
2015-09-03 15:59                 ` Michael Roth
2015-09-03 17:01                   ` Markus Armbruster
2015-09-03 19:59                     ` Michael Roth
2015-09-04  6:39                       ` Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 31/32] qapi-introspect: Map all integer types to 'int' Markus Armbruster
2015-08-04 15:58 ` [Qemu-devel] [PATCH RFC v3 32/32] qapi-introspect: Hide type names Markus Armbruster
2015-08-05 21:06   ` Eric Blake
2015-08-05 21:50     ` Eric Blake
2015-08-06  6:49     ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55DB50F7.8040909@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berto@igalia.com \
    --cc=kwolf@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).