qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>,
	marcandre.lureau@redhat.com, qemu-devel@nongnu.org,
	ehabkost@redhat.com
Subject: Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check()
Date: Tue, 13 Oct 2015 06:46:46 -0600	[thread overview]
Message-ID: <561CFD36.5020106@redhat.com> (raw)
In-Reply-To: <87fv1fgs9g.fsf@blackfin.pond.sub.org>

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

On 10/13/2015 01:08 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 10/12/2015 09:53 AM, Markus Armbruster wrote:
> [...]
>>> It would be simpler if we could make C clash only when QMP clashes.
>>
>> If a QMP clash always caused a C clash, life would be a bit simpler. We
>> aren't going to get there (because in a flat union, hiding the members
>> of the branch type behind a single pointer means those members don't
>> clash with any C names of the overall union), but I can certainly try to
>> make the comments explain what is going on.
> 
> (1) If QMP clashed exactly when C clashed, we'd have to think only about
> one of them, and the C compiler would check for clashes statically.

Although deferring the error about the clash until compile time is a bit
long, compared to reporting the clash at generator time.

> 
> (2) If a QMP clash implied a C clash, we'd only have to think about C,
> and the C compiler would check statically.
> 
> (3) If a C clash implied a QMP clash, we'd only have to think about QMP.
> 
> (4) If they can clash independently, we need to think about both
> independently.
> 
> We currently have (4), and having to juggle both QMP and C in my mind
> has been taxing.
> 
> My remark was about (3): C clashes only when QMP clashes <=> C clash
> implies QMP clash.
> 
> Your remark was about (2).  More useful than (3), but as you say not
> feasible.
> 
> Do you think we can get to (3)?

C clash that does not imply a QMP clash:

{ 'union':'U', 'data':{ 'a':'int', 'A':'str' } }

C clash (the implicit enum has two U_KIND_A values) but not a QMP clash
('a' and 'A' don't even appear in QMP; and even if they did, they are
distinct in C as branch names).  Might be possible to avoid the C clash
by munging case labels of the generated enum differently, but I'm not
sure it is worth the effort.

QMP clash that does not (currently) imply a C clash (using abbreviated
notation):

{ 'union':'U', 'base':{ 'type':'Enum', 'member':'int' },
  'discriminator':'type',
  'data':{ 'branch':{ 'member':'str' } } }

QMP clash (the field 'member' is part of the base type, and also part of
the 'branch' variant type), but not a C clash (because the C layout
accesses the branch through a box named 'branch').

But we cannot just remove the boxing, either, because then we'd have a
clash in C that is NOT a clash in QMP:

{ 'union':'U', 'base':{ 'type':'Enum' }, 'discriminator':'type',
'data':{ 'branch1':{ 'member':'str' }, 'branch2': { 'member':'int' } } }

If the two 'member' fields were at the same C level as 'type', then we
could use C collisions to detect a QMP clash between a variant's members
and the non-variant fields - but it also has the drawback of declaring a
C collision between the two branches.

So no, I don't think we can get to (3).

-- 
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-10-13 12:47 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-04  3:40 [Qemu-devel] [PATCH v7 00/14] post-introspection cleanups, subset B Eric Blake
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 01/14] qapi: Use predicate callback to determine visit filtering Eric Blake
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 02/14] qapi: Prepare for errors during check() Eric Blake
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 03/14] qapi: Drop redundant alternate-good test Eric Blake
2015-10-07 16:15   ` Markus Armbruster
2015-10-07 16:33     ` Eric Blake
2015-10-13  8:12       ` Markus Armbruster
2015-10-13 12:31         ` Eric Blake
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 04/14] qapi: Don't use info as witness of implicit object type Eric Blake
2015-10-07 16:27   ` Markus Armbruster
2015-10-09 22:41     ` Eric Blake
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 05/14] qapi: Lazy creation of array types Eric Blake
2015-10-07 16:38   ` Markus Armbruster
2015-10-10 20:16     ` Eric Blake
2015-10-12  8:24       ` Markus Armbruster
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 06/14] qapi: Create simple union type member earlier Eric Blake
2015-10-07 16:44   ` Markus Armbruster
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 07/14] qapi: Move union tag quirks into subclass Eric Blake
2015-10-07 16:11   ` [Qemu-devel] [PATCH] fixup to " Eric Blake
2015-10-08 12:25   ` [Qemu-devel] [PATCH v7 07/14] " Markus Armbruster
2015-10-08 15:02     ` Eric Blake
2015-10-08 12:26   ` [Qemu-devel] [RFC PATCH] qapi: Rename simple union's generated tag member to type Markus Armbruster
2015-10-08 14:56     ` Eric Blake
2015-10-14 13:16     ` Eric Blake
2015-10-14 16:04       ` Markus Armbruster
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 08/14] qapi: Track location that created an implicit type Eric Blake
2015-10-08 14:19   ` Markus Armbruster
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 09/14] qapi: Track owner of each object member Eric Blake
2015-10-09 13:17   ` Markus Armbruster
2015-10-09 14:30     ` Eric Blake
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 10/14] qapi: Detect collisions in C member names Eric Blake
2015-10-09 14:11   ` Markus Armbruster
2015-10-09 14:33     ` Eric Blake
2015-10-12  8:34       ` Markus Armbruster
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check() Eric Blake
2015-10-12 15:53   ` Markus Armbruster
2015-10-12 16:22     ` Eric Blake
2015-10-13  4:10       ` Eric Blake
2015-10-13  7:08       ` Markus Armbruster
2015-10-13 12:46         ` Eric Blake [this message]
2015-10-13 15:39           ` Markus Armbruster
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 12/14] qapi: Move duplicate enum value " Eric Blake
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 13/14] qapi: Add test for alternate branch 'kind' clash Eric Blake
2015-10-04  3:41 ` [Qemu-devel] [PATCH v7 14/14] qapi: Detect base class loops Eric Blake

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=561CFD36.5020106@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=marcandre.lureau@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).