From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50197) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ztvpx-0007u8-4M for qemu-devel@nongnu.org; Wed, 04 Nov 2015 06:02:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ztvpt-0003mz-V6 for qemu-devel@nongnu.org; Wed, 04 Nov 2015 06:02:41 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40142) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ztvpt-0003mu-QV for qemu-devel@nongnu.org; Wed, 04 Nov 2015 06:02:37 -0500 From: Markus Armbruster References: <1446618049-13596-1-git-send-email-eblake@redhat.com> <1446618049-13596-11-git-send-email-eblake@redhat.com> Date: Wed, 04 Nov 2015 12:02:34 +0100 In-Reply-To: <1446618049-13596-11-git-send-email-eblake@redhat.com> (Eric Blake's message of "Tue, 3 Nov 2015 23:20:32 -0700") Message-ID: <87fv0mowmt.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v9 10/27] qapi: Track simple union tag in object.local_members List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, Michael Roth Eric Blake writes: > We were previously creating all unions with an empty list for > local_members. However, it will make it easier to unify struct > and union generation if we include the generated tag member in > local_members. That way, we can have a common code pattern: > visit the base (if any), visit the local members (if any), visit > the variants (if any). The local_members of a flat union > remains empty (because the discriminator is already visited as > part of the base). Then, by visiting tag_member.check() during > AlternateType.check(), we no longer need to call it during > Variants.check(). Moving tag_member.check() could perhaps be a separate patch. But it's okay as is. > The various front end entities now exist as follows: > struct: optional base, optional local_members, no variants > simple union: no base, one-element local_members, variants with tag_member > from local_members > flat union: base, no local_members, variants with tag_member from base > alternate: no base, no local_members, variants Ultimately, I'd like a single object type. Then this becomes: object: optional base, optional local_members, optional variants with tag_member from base or local_members alternate: only variants In both cases, I view the tag member as belonging to the outer container, not variants: object stores it like any other member, and variants.tag_member merely identifies the member that serves as tag alternate doesn't store it member in self, but simply uses self.variants.tag_member instead. Perhaps QAPISchemaObjectTypeVariants should be inlined. Just rambling, the commit message is fine as is. > With the new local members, we require a bit of finesse to > avoid assertions in the clients. > > No change to generated code. > > Signed-off-by: Eric Blake Patch looks good.