From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44734) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZmGHi-0007ZG-KE for qemu-devel@nongnu.org; Wed, 14 Oct 2015 03:15:39 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZmGHc-0003p6-RE for qemu-devel@nongnu.org; Wed, 14 Oct 2015 03:15:37 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38264) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZmGHc-0003p2-FO for qemu-devel@nongnu.org; Wed, 14 Oct 2015 03:15:32 -0400 From: Markus Armbruster References: <1444710158-8723-1-git-send-email-eblake@redhat.com> <1444710158-8723-9-git-send-email-eblake@redhat.com> Date: Wed, 14 Oct 2015 09:15:28 +0200 In-Reply-To: <1444710158-8723-9-git-send-email-eblake@redhat.com> (Eric Blake's message of "Mon, 12 Oct 2015 22:22:28 -0600") Message-ID: <87y4f59azz.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v8 08/18] qapi: Lazy creation of array types 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: > Commit ac88219a had several TODO markers about whether we needed > to automatically create the corresponding array type alongside > any other type. It turns out that most of the time, we don't! > > As part of lazy creation of array types, this patch now assigns > an 'info' to array types at their point of first instantiation, > rather than leaving it None. I'm afraid this flips the value of .is_implicit() to False. Currently harmless, but let's keep it correct anyway. The obvious fix is to define the trivial override method: def is_implicit(self): return True But I'd rather do *all* the "give implicit types info" work in "qapi: Track location that created an implicit type", i.e. move the plumbing of info there, add the override method there, drop the "As part of" paragraph from the commit message here. I append what's left of this patch then. I like it, because the patch that actually changes generated code (this one) becomes really simple, and the lengthened patch remains mere info-plumbing that doesn't affect the generated code. diff --git a/scripts/qapi.py b/scripts/qapi.py index d7cf0f3..9e01705 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -1143,7 +1143,12 @@ class QAPISchema(object): def _def_builtin_type(self, name, json_type, c_type, c_null): self._def_entity(QAPISchemaBuiltinType(name, json_type, c_type, c_null)) - self._make_array_type(name) # TODO really needed? + # TODO As long as we have QAPI_TYPES_BUILTIN to share multiple + # qapi-types.h from a single .c, all arrays of builtins must be + # declared in the first file whether or not they are used. Nicer + # would be to use lazy instantiation, while figuring out how to + # avoid compilation issues with multiple qapi-types.h. + self._make_array_type(name) def _def_predefineds(self): for t in [('str', 'string', 'char' + pointer_suffix, 'NULL'), @@ -1192,7 +1197,6 @@ class QAPISchema(object): data = expr['data'] prefix = expr.get('prefix') self._def_entity(QAPISchemaEnumType(name, info, data, prefix)) - self._make_array_type(name) # TODO really needed? def _make_member(self, name, typ): optional = False @@ -1215,7 +1219,6 @@ class QAPISchema(object): self._def_entity(QAPISchemaObjectType(name, info, base, self._make_members(data), None)) - self._make_array_type(name) # TODO really needed? def _make_variant(self, case, typ): return QAPISchemaObjectTypeVariant(case, typ) @@ -1251,7 +1254,6 @@ class QAPISchema(object): QAPISchemaObjectTypeVariants(tag_name, tag_enum, variants))) - self._make_array_type(name) # TODO really needed? def _def_alternate_type(self, expr, info): name = expr['alternate'] @@ -1264,7 +1266,6 @@ class QAPISchema(object): QAPISchemaObjectTypeVariants(None, tag_enum, variants))) - self._make_array_type(name) # TODO really needed? def _def_command(self, expr, info): name = expr['command']