From patchwork Fri Mar 18 10:04:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Markus Armbruster X-Patchwork-Id: 8617871 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id A05429F54C for ; Fri, 18 Mar 2016 10:11:11 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id AC5772034B for ; Fri, 18 Mar 2016 10:11:10 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5D4092024D for ; Fri, 18 Mar 2016 10:11:09 +0000 (UTC) Received: from localhost ([::1]:42662 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agrN6-0003HI-MR for patchwork-qemu-devel@patchwork.kernel.org; Fri, 18 Mar 2016 06:11:08 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53021) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agrGq-0008CX-Rx for qemu-devel@nongnu.org; Fri, 18 Mar 2016 06:04:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agrGk-0001Tp-Pj for qemu-devel@nongnu.org; Fri, 18 Mar 2016 06:04:40 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40982) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agrGk-0001Tc-Hs for qemu-devel@nongnu.org; Fri, 18 Mar 2016 06:04:34 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 387D85BED1 for ; Fri, 18 Mar 2016 10:04:34 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-116-34.ams2.redhat.com [10.36.116.34]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u2IA4W1C031791 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 18 Mar 2016 06:04:33 -0400 Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id 8258C3003844; Fri, 18 Mar 2016 11:04:29 +0100 (CET) From: Markus Armbruster To: qemu-devel@nongnu.org Date: Fri, 18 Mar 2016 11:04:19 +0100 Message-Id: <1458295469-22215-6-git-send-email-armbru@redhat.com> In-Reply-To: <1458295469-22215-1-git-send-email-armbru@redhat.com> References: <1458295469-22215-1-git-send-email-armbru@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Fri, 18 Mar 2016 10:04:34 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 05/15] qapi: Emit implicit structs in generated C X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Eric Blake We already have several places that want to visit all the members of an implicit object within a larger context (simple union variant, event with anonymous data, command with anonymous arguments struct); and will be adding another one soon (the ability to declare an anonymous base for a flat union). Having a C struct declared for these implicit types, along with a visit_type_FOO_members() helper function, will make for fewer special cases in our generator. We do not, however, need qapi_free_FOO() or visit_type_FOO() functions for implicit types, because they should not be used directly outside of the generated code. This is done by adding a conditional in visit_object_type() for both qapi-types.py and qapi-visit.py based on the object name. The comparison of "name.startswith('q_')" is a bit hacky (it's basically duplicating what .is_implicit() already uses), but beats changing the signature of the visit_object_type() callback to pass a new 'implicit' flag. The hack should be temporary: we are considering adding a future patch that consolidates the narrow visit_object_type(..., base, local_members, variants) and visit_object_type_flat(..., all_members, variants) [where different sets of information are already broken out, and the QAPISchemaObjectType is no longer available] into a broader visit_object_type(obj_type) [where the visitor can query the needed fields from obj_type directly]. Also, now that we WANT to output C code for implicits, we no longer need the visit_needed() filter, leaving 'q_empty' as the only object still needing a special case. Remember, 'q_empty' is the only built-in generated object, which means that without a special case it would be emitted in multiple files (the main qapi-types.h and in qga-qapi-types.h) causing compilation failure due to redefinition. But since it has no members, it's easier to just avoid an attempt to visit that particular type; since gen_object() is called recursively, we also prime the objects_seen set to cover any recursion into the empty type. The patch relies on the changed naming of implicit types in the previous patch. It is a bit unfortunate that the generated struct names and visit_type_FOO_members() don't match normal naming conventions, but it's not too bad, since they will only be used in generated code. The generated code grows substantially in size: the implicit '-wrapper' types must be emitted in qapi-types.h before any union can include an unboxed member of that type. Arguably, the '-args' types could be emitted in a private header for just qapi-visit.c and qmp-marshal.c, rather than polluting qapi-types.h; but adding complexity to the generator to split the output location according to role doesn't seem worth the maintenance costs. Signed-off-by: Eric Blake Message-Id: <1458254921-17042-6-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster --- scripts/qapi-types.py | 19 +++++++++++-------- scripts/qapi-visit.py | 23 +++++++++++------------ scripts/qapi.py | 2 -- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index f194bea..79416b2 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -61,8 +61,7 @@ def gen_object(name, base, members, variants): ret = '' if variants: for v in variants.variants: - if (isinstance(v.type, QAPISchemaObjectType) and - not v.type.is_implicit()): + if isinstance(v.type, QAPISchemaObjectType): ret += gen_object(v.type.name, v.type.base, v.type.local_members, v.type.variants) @@ -180,6 +179,8 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor): self._btin = None def visit_begin(self, schema): + # gen_object() is recursive, ensure it doesn't visit the empty type + objects_seen.add(schema.the_empty_object_type.name) self.decl = '' self.defn = '' self._fwdecl = '' @@ -196,11 +197,6 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor): self.decl = self._btin + self.decl self._btin = None - def visit_needed(self, entity): - # Visit everything except implicit objects - return not (entity.is_implicit() and - isinstance(entity, QAPISchemaObjectType)) - def _gen_type_cleanup(self, name): self.decl += gen_type_cleanup_decl(name) self.defn += gen_type_cleanup(name) @@ -229,11 +225,18 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor): self._gen_type_cleanup(name) def visit_object_type(self, name, info, base, members, variants): + # Nothing to do for the special empty builtin + if name == 'q_empty': + return self._fwdecl += gen_fwd_object_or_array(name) self.decl += gen_object(name, base, members, variants) if base: self.decl += gen_upcast(name, base) - self._gen_type_cleanup(name) + # TODO Worth changing the visitor signature, so we could + # directly use rather than repeat type.is_implicit()? + if not name.startswith('q_'): + # implicit types won't be directly allocated/freed + self._gen_type_cleanup(name) def visit_alternate_type(self, name, info, variants): self._fwdecl += gen_fwd_object_or_array(name) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index a712e9a..4923b2e 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -215,13 +215,11 @@ out: def gen_visit_object(name, base, members, variants): - ret = gen_visit_object_members(name, base, members, variants) - # FIXME: if *obj is NULL on entry, and visit_start_struct() assigns to # *obj, but then visit_type_FOO_members() fails, we should clean up *obj # rather than leaving it non-NULL. As currently written, the caller must # call qapi_free_FOO() to avoid a memory leak of the partial FOO. - ret += mcgen(''' + return mcgen(''' void visit_type_%(c_name)s(Visitor *v, const char *name, %(c_name)s **obj, Error **errp) { @@ -245,8 +243,6 @@ out: ''', c_name=c_name(name)) - return ret - class QAPISchemaGenVisitVisitor(QAPISchemaVisitor): def __init__(self): @@ -268,11 +264,6 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor): self.decl = self._btin + self.decl self._btin = None - def visit_needed(self, entity): - # Visit everything except implicit objects - return not (entity.is_implicit() and - isinstance(entity, QAPISchemaObjectType)) - def visit_enum_type(self, name, info, values, prefix): # Special case for our lone builtin enum type # TODO use something cleaner than existence of info @@ -296,9 +287,17 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor): self.defn += defn def visit_object_type(self, name, info, base, members, variants): + # Nothing to do for the special empty builtin + if name == 'q_empty': + return self.decl += gen_visit_members_decl(name) - self.decl += gen_visit_decl(name) - self.defn += gen_visit_object(name, base, members, variants) + self.defn += gen_visit_object_members(name, base, members, variants) + # TODO Worth changing the visitor signature, so we could + # directly use rather than repeat type.is_implicit()? + if not name.startswith('q_'): + # only explicit types need an allocating visit + self.decl += gen_visit_decl(name) + self.defn += gen_visit_object(name, base, members, variants) def visit_alternate_type(self, name, info, variants): self.decl += gen_visit_decl(name) diff --git a/scripts/qapi.py b/scripts/qapi.py index f6701f5..96fb216 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -1000,7 +1000,6 @@ class QAPISchemaObjectType(QAPISchemaType): return self.name.startswith('q_') def c_name(self): - assert not self.is_implicit() return QAPISchemaType.c_name(self) def c_type(self): @@ -1008,7 +1007,6 @@ class QAPISchemaObjectType(QAPISchemaType): return c_name(self.name) + pointer_suffix def c_unboxed_type(self): - assert not self.is_implicit() return c_name(self.name) def json_type(self):