All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, Michael Roth <mdroth@linux.vnet.ibm.com>
Subject: Re: [Qemu-devel] [PATCH v8.5 1/4] qapi: Drop all_members parameter from check()
Date: Tue, 3 Nov 2015 10:34:07 -0700	[thread overview]
Message-ID: <5638F00F.9050805@redhat.com> (raw)
In-Reply-To: <8737wn5a40.fsf@blackfin.pond.sub.org>

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

On 11/03/2015 09:19 AM, Markus Armbruster wrote:

>>> I'm assuming this patch is based on
>>> [PATCH v8 06/17] qapi-types: Consolidate gen_struct() and gen_union()
>>> which has
>>>
>>>     def check(self, schema, members, seen):
>>>         if self.tag_name:    # flat union
>>>             self.tag_member = seen[self.tag_name]
>>>         elif seen:           # simple union
>>>             assert self.tag_member in seen.itervalues()
>>>         else:                # alternate
>>> --->        self.tag_member.check(schema, members, seen)
>>>
>>> in QAPISchemaObjectTypeVariants.
>>
>> That was true in v8, but not in my pending v9 which did essentially what
>> your 7/7 did (move the tag_member.check() into Alternate.check() back in
>> patch 5).

Caused me some churn in incorporating your patches, but not too bad.

>>
>> At any rate, my first glance of your series shows that it is reasonable,
>> so my task today is to spit out a v9 of my series, but using your seven
>> patches in place of my four.
> 
> Buyer beware: I'm not sure my seven do everything your four do.

Yours do the same as my patch 1. You also made a compelling argument for
eliminating my patch 3 (the enum values are already collision-proof,
therefore if we check that a variant name is in the enum, we don't need
to check for collisions) - except that in my 10/17, when I get rid of
the generated enum for alternates, I have to reinstate that check
somewhere; but adding it directly to QAPISchemaAlternateType.check() at
that point feels better.

My patch 2 is still needed, but looks a bit nicer on top of some of your
refactoring.  And my patch 4 is still useful, as additional refactoring
to share the code used by my patch 2 and your code for ObjectType.check().

We'll see how it all turns out in v9.

-- 
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-11-03 17:34 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-28 17:14 [Qemu-devel] [PATCH v8 00/17] alternate layout (post-introspection cleanups, subset C) Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 01/17] qapi: Use generated TestStruct machinery in tests Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 02/17] qapi: Strengthen test of TestStructList Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 03/17] qapi: Provide nicer array names in introspection Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 04/17] qapi-introspect: Guarantee particular sorting Eric Blake
2015-10-30 11:20   ` Markus Armbruster
2015-10-30 15:41     ` Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 05/17] qapi: Track simple union tag in object.local_members Eric Blake
2015-10-30 12:54   ` Markus Armbruster
2015-10-30 16:32     ` Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 06/17] qapi-types: Consolidate gen_struct() and gen_union() Eric Blake
2015-10-30 13:01   ` Markus Armbruster
2015-10-30 16:36     ` Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 07/17] qapi: Rework collision assertions Eric Blake
2015-11-02 15:37   ` Markus Armbruster
2015-11-02 17:20     ` Eric Blake
2015-11-02 21:24     ` Eric Blake
2015-11-03  7:56       ` Markus Armbruster
2015-11-03 13:30         ` Eric Blake
2015-11-02 22:41     ` [Qemu-devel] [PATCH v8.5 0/4] rework of 7/17 Eric Blake
2015-11-02 22:41     ` [Qemu-devel] [PATCH v8.5 1/4] qapi: Drop all_members parameter from check() Eric Blake
2015-11-03 11:06       ` Markus Armbruster
2015-11-03 13:26         ` Eric Blake
2015-11-03 14:02           ` Markus Armbruster
2015-11-03 14:12             ` Eric Blake
2015-11-03 16:19               ` Markus Armbruster
2015-11-03 17:34                 ` Eric Blake [this message]
2015-11-03 14:04         ` [Qemu-devel] [PATCH 1/7] qapi: Drop obsolete tag value collision assertions Markus Armbruster
2015-11-03 14:04           ` [Qemu-devel] [PATCH 2/7] qapi: Simplify QAPISchemaObjectTypeMember.check() Markus Armbruster
2015-11-03 14:04           ` [Qemu-devel] [PATCH 3/7] qapi: Clean up after previous commit Markus Armbruster
2015-11-03 14:04           ` [Qemu-devel] [PATCH 4/7] qapi: Fix up commit 7618b91's clash sanity checking change Markus Armbruster
2015-11-03 14:04           ` [Qemu-devel] [PATCH 5/7] qapi: Eliminate QAPISchemaObjectType.check() variable members Markus Armbruster
2015-11-03 14:04           ` [Qemu-devel] [PATCH 6/7] qapi: Factor out QAPISchemaObjectTypeMember.check_clash() Markus Armbruster
2015-11-03 14:04           ` [Qemu-devel] [PATCH 7/7] qapi: QAPISchemaObjectTypeVariants.check() Markus Armbruster
2015-11-02 22:41     ` [Qemu-devel] [PATCH v8.5 2/4] qapi: Check for QMP collisions of flat union branches Eric Blake
2015-11-02 22:41     ` [Qemu-devel] [PATCH v8.5 3/4] qapi: Fix check for variant tag values collision Eric Blake
2015-11-02 22:41     ` [Qemu-devel] [PATCH v8.5 4/4] qapi: Consolidate collision detection code Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 08/17] qapi: Remove outdated tests related to QMP/branch collisions Eric Blake
2015-11-03 16:32   ` Markus Armbruster
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 09/17] qapi: Add positive tests to qapi-schema-test Eric Blake
2015-11-03 16:43   ` Markus Armbruster
2015-11-03 16:56     ` Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 10/17] qapi: Simplify visiting of alternate types Eric Blake
2015-11-03 18:30   ` Markus Armbruster
2015-11-03 18:59     ` Eric Blake
2015-11-04  7:30       ` Markus Armbruster
2015-11-04 16:03       ` Markus Armbruster
2015-11-04 20:52         ` Eric Blake
2015-11-05  7:17           ` Markus Armbruster
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 11/17] qapi: Fix alternates that accept 'number' but not 'int' Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 12/17] qapi: Remove dead visitor code Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 13/17] qapi: Plug leaks in test-qmp-* Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 14/17] qapi: Simplify error testing " Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 15/17] qapi: Test failure in middle of array parse Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 16/17] qapi: More tests of input arrays Eric Blake
2015-10-28 17:14 ` [Qemu-devel] [PATCH v8 17/17] qapi: Simplify visits of optional fields 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=5638F00F.9050805@redhat.com \
    --to=eblake@redhat.com \
    --cc=armbru@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.