From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39637) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZjNj5-0004iI-6j for qemu-devel@nongnu.org; Tue, 06 Oct 2015 04:36:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZjNj1-0006aC-0k for qemu-devel@nongnu.org; Tue, 06 Oct 2015 04:35:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:39984) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZjNj0-0006Zf-NG for qemu-devel@nongnu.org; Tue, 06 Oct 2015 04:35:54 -0400 From: Markus Armbruster References: <1443760312-656-1-git-send-email-eblake@redhat.com> <1443760312-656-4-git-send-email-eblake@redhat.com> <87pp0xg0eg.fsf@blackfin.pond.sub.org> <560E8133.9060805@redhat.com> Date: Tue, 06 Oct 2015 10:35:51 +0200 In-Reply-To: <560E8133.9060805@redhat.com> (Eric Blake's message of "Fri, 2 Oct 2015 07:05:55 -0600") Message-ID: <87oagcs8bc.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH v6 03/12] qapi: Lazy creation of array types List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Michael Roth , marcandre.lureau@redhat.com, qemu-devel@nongnu.org, ehabkost@redhat.com Eric Blake writes: > On 10/02/2015 02:06 AM, Markus Armbruster wrote: >> Woot! >> >> 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 guess our general for info should be: > > s/general/general description/ ? General idea. Looks like my typing engine doesn't run on all cylinders on Fridays... >> For explicitly defined entities, info points to the (explicit) >> definition. For implicitly defined entities, it points to a place that >> triggers implicit definition. For some kinds of entities, multiple >> places may exist, and info points to one of them. >> > > Right now, we don't document any of the fields, but I can certainly add > that comment. > > >> >> Here, you're talking about the effect on generated code. I'd put the >> specific case of introspection last, in a paragraph of its own. >> > > Improving the commit message is easier than redoing bad code :) > >>> +++ b/qapi-schema.json >>> @@ -3396,6 +3396,16 @@ >>> 'features': 'int' } } >>> >>> ## >>> +# @Dummy >>> +# >>> +# Not used by QMP; hack to let us use X86CPUFeatureWordInfoList internally >>> +# >>> +# Since 2.5 >>> +## >>> +{ 'struct': 'Dummy', 'data': { 'unused': ['X86CPUFeatureWordInfo'] } } >> >> DummyToForceArrays? > > Sure. > >> >>> + >>> + >>> +## >>> # @RxState: >>> # >>> # Packets receiving state >>> diff --git a/scripts/qapi.py b/scripts/qapi.py >>> index 8123ab3..15640b6 100644 >>> --- a/scripts/qapi.py >>> +++ b/scripts/qapi.py >>> @@ -1143,7 +1143,7 @@ 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? >>> + self._make_array_type(name, None) >> >> Should we keep a TODO here? > > Sure, with better text explaining the guard issue on types shared in > multiple qapi-types.h files. > > >>> typ = self._make_implicit_object_type(typ, 'wrapper', >>> - [self._make_member('data', typ)]) >>> + [self._make_member('data', typ, >>> + info)]) >> >> Consider a hanging indent here: >> >> typ = self._make_implicit_object_type( >> typ, 'wrapper', [self._make_member('data', typ, info)]) > > > Okay. pep8 and pylint like it (I'm not used to doing it, but it does > look better, and emacs didn't fight me too hard). I'm still getting used to hanging indents myself. When in Rome...