[{"id":1817840,"web_url":"http://patchwork.ozlabs.org/comment/1817840/","msgid":"<87h8t2o7i3.fsf@dusky.pond.sub.org>","list_archive_url":null,"date":"2017-12-07T15:57:56","subject":"Re: [Qemu-devel] [PATCH v3 15/50] qapi-types: refactor variants\n\thandling","submitter":{"id":2645,"url":"http://patchwork.ozlabs.org/api/people/2645/","name":"Markus Armbruster","email":"armbru@redhat.com"},"content":"Marc-André Lureau writes:\n\n> Generate variants objects outside gen_object(). This will allow to\n> easily wrap gen_object() with ifcond_decorator in the following patch.\n>\n> Signed-off-by: Marc-André Lureau \n> ---\n> scripts/qapi-types.py | 37 +++++++++++++++++++++++--------------\n> 1 file changed, 23 insertions(+), 14 deletions(-)\n>\n> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py\n> index 915786c463..2b3588267b 100644\n> --- a/scripts/qapi-types.py\n> +++ b/scripts/qapi-types.py\n> @@ -53,23 +53,27 @@ def gen_struct_members(members):\n> return ret\n> \n> \n> -def gen_object(name, base, members, variants):\n> - if name in objects_seen:\n> - return ''\n> - objects_seen.add(name)\n> -\n> +def gen_variants_objects(variants):\n> ret = ''\n> if variants:\n> for v in variants.variants:\n> if isinstance(v.type, QAPISchemaObjectType):\n> + ret += gen_variants_objects(v.type.variants)\n> ret += gen_object(v.type.name, v.type.base,\n> v.type.local_members, v.type.variants)\n> + return ret\n> \n> - ret += mcgen('''\n> +\n> +def gen_object(name, base, members, variants):\n> + if name in objects_seen:\n> + return ''\n> + objects_seen.add(name)\n> +\n> + ret = mcgen('''\n> \n> struct %(c_name)s {\n> ''',\n> - c_name=c_name(name))\n> + c_name=c_name(name))\n> \n> if base:\n> if not base.is_implicit():\n> @@ -218,11 +222,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):\n> self.decl += gen_array(name, element_type)\n> self._gen_type_cleanup(name)\n> \n> - def visit_object_type(self, name, info, ifcond, base, members, variants):\n> - # Nothing to do for the special empty builtin\n> - if name == 'q_empty':\n> - return\n> - self._fwdecl += gen_fwd_object_or_array(name)\n> + def _gen_object(self, name, info, ifcond, base, members, variants):\n> self.decl += gen_object(name, base, members, variants)\n> if base and not base.is_implicit():\n> self.decl += gen_upcast(name, base)\n> @@ -232,10 +232,19 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):\n> # implicit types won't be directly allocated/freed\n> self._gen_type_cleanup(name)\n> \n> + def visit_object_type(self, name, info, ifcond, base, members, variants):\n> + # Nothing to do for the special empty builtin\n> + if name == 'q_empty':\n> + return\n> + self._fwdecl += gen_fwd_object_or_array(name)\n> + self.decl += gen_variants_objects(variants)\n> + self._gen_object(name, info, None, base, members, variants)\n> +\n> def visit_alternate_type(self, name, info, ifcond, variants):\n> self._fwdecl += gen_fwd_object_or_array(name)\n> - self.decl += gen_object(name, None, [variants.tag_member], variants)\n> - self._gen_type_cleanup(name)\n> + self.decl += gen_variants_objects(variants)\n> + self._gen_object(name, info, None, None,\n> + [variants.tag_member], variants)\n\nWhere did self._gen_type_cleanup(name) go? Hmm, I guess it's now in\n_gen_object(). Confusing.\n\n> \n> # If you link code generated from multiple schemata, you want only one\n> # instance of the code for built-in types. Generate it only when\n\nIf I read this patch correctly, you move code from the beginning of\ngen_object() to new gen_variants_objects(), then arrange to have\ngen_variants_objects() called right before gen_object(). Correct?\n\nIn old gen_object(), the code is guarded by\n\n if name in objects_seen:\n return ''\n objects_seen.add(name)\n\nIn new gen_variants_objects(), it isn't. Why?","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":"ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3yt0XX4j64z9s03\n\tfor ;\n\tFri, 8 Dec 2017 02:59:08 +1100 (AEDT)","from localhost ([::1]:33010 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1eMyZm-0003SE-Qg\n\tfor incoming@patchwork.ozlabs.org; Thu, 07 Dec 2017 10:59:06 -0500","from eggs.gnu.org ([2001:4830:134:3::10]:49968)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1eMyYk-0002ww-Uu\n\tfor qemu-devel@nongnu.org; Thu, 07 Dec 2017 10:58:04 -0500","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1eMyYk-000509-0F\n\tfor qemu-devel@nongnu.org; Thu, 07 Dec 2017 10:58:03 -0500","from mx1.redhat.com ([209.132.183.28]:9782)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from ) id 1eMyYj-0004zs-NT\n\tfor qemu-devel@nongnu.org; Thu, 07 Dec 2017 10:58:01 -0500","from smtp.corp.redhat.com\n\t(int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id C353D356D3;\n\tThu, 7 Dec 2017 15:58:00 +0000 (UTC)","from blackfin.pond.sub.org (ovpn-116-74.ams2.redhat.com\n\t[10.36.116.74])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id 4ECC2703A9;\n\tThu, 7 Dec 2017 15:57:58 +0000 (UTC)","by blackfin.pond.sub.org (Postfix, from userid 1000)\n\tid D32B21138658; Thu, 7 Dec 2017 16:57:56 +0100 (CET)"],"From":"Markus Armbruster ","To":"=?utf-8?q?Marc-Andr=C3=A9?= Lureau ","References":"<20170911110623.24981-1-marcandre.lureau@redhat.com>\n\t<20170911110623.24981-16-marcandre.lureau@redhat.com>","Date":"Thu, 07 Dec 2017 16:57:56 +0100","In-Reply-To":"<20170911110623.24981-16-marcandre.lureau@redhat.com> (\n\t=?utf-8?b?Ik1hcmMtQW5kcsOp?= Lureau\"'s message of \"Mon,\n\t11 Sep 2017 \t13:05:48 +0200\")","Message-ID":"<87h8t2o7i3.fsf@dusky.pond.sub.org>","User-Agent":"Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.15","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tThu, 07 Dec 2017 15:58:00 +0000 (UTC)","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 15/50] qapi-types: refactor variants\n\thandling","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"qemu-devel@nongnu.org, Michael Roth ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1837331,"web_url":"http://patchwork.ozlabs.org/comment/1837331/","msgid":"","list_archive_url":null,"date":"2018-01-11T21:22:02","subject":"Re: [Qemu-devel] [PATCH v3 15/50] qapi-types: refactor variants\n\thandling","submitter":{"id":6442,"url":"http://patchwork.ozlabs.org/api/people/6442/","name":"Marc-André Lureau","email":"marcandre.lureau@gmail.com"},"content":"Hi\n\nOn Thu, Dec 7, 2017 at 4:57 PM, Markus Armbruster wrote:\n> Marc-André Lureau writes:\n>\n>> Generate variants objects outside gen_object(). This will allow to\n>> easily wrap gen_object() with ifcond_decorator in the following patch.\n>>\n>> Signed-off-by: Marc-André Lureau \n>> ---\n>> scripts/qapi-types.py | 37 +++++++++++++++++++++++--------------\n>> 1 file changed, 23 insertions(+), 14 deletions(-)\n>>\n>> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py\n>> index 915786c463..2b3588267b 100644\n>> --- a/scripts/qapi-types.py\n>> +++ b/scripts/qapi-types.py\n>> @@ -53,23 +53,27 @@ def gen_struct_members(members):\n>> return ret\n>>\n>>\n>> -def gen_object(name, base, members, variants):\n>> - if name in objects_seen:\n>> - return ''\n>> - objects_seen.add(name)\n>> -\n>> +def gen_variants_objects(variants):\n>> ret = ''\n>> if variants:\n>> for v in variants.variants:\n>> if isinstance(v.type, QAPISchemaObjectType):\n>> + ret += gen_variants_objects(v.type.variants)\n>> ret += gen_object(v.type.name, v.type.base,\n>> v.type.local_members, v.type.variants)\n>> + return ret\n>>\n>> - ret += mcgen('''\n>> +\n>> +def gen_object(name, base, members, variants):\n>> + if name in objects_seen:\n>> + return ''\n>> + objects_seen.add(name)\n>> +\n>> + ret = mcgen('''\n>>\n>> struct %(c_name)s {\n>> ''',\n>> - c_name=c_name(name))\n>> + c_name=c_name(name))\n>>\n>> if base:\n>> if not base.is_implicit():\n>> @@ -218,11 +222,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):\n>> self.decl += gen_array(name, element_type)\n>> self._gen_type_cleanup(name)\n>>\n>> - def visit_object_type(self, name, info, ifcond, base, members, variants):\n>> - # Nothing to do for the special empty builtin\n>> - if name == 'q_empty':\n>> - return\n>> - self._fwdecl += gen_fwd_object_or_array(name)\n>> + def _gen_object(self, name, info, ifcond, base, members, variants):\n>> self.decl += gen_object(name, base, members, variants)\n>> if base and not base.is_implicit():\n>> self.decl += gen_upcast(name, base)\n>> @@ -232,10 +232,19 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):\n>> # implicit types won't be directly allocated/freed\n>> self._gen_type_cleanup(name)\n>>\n>> + def visit_object_type(self, name, info, ifcond, base, members, variants):\n>> + # Nothing to do for the special empty builtin\n>> + if name == 'q_empty':\n>> + return\n>> + self._fwdecl += gen_fwd_object_or_array(name)\n>> + self.decl += gen_variants_objects(variants)\n>> + self._gen_object(name, info, None, base, members, variants)\n>> +\n>> def visit_alternate_type(self, name, info, ifcond, variants):\n>> self._fwdecl += gen_fwd_object_or_array(name)\n>> - self.decl += gen_object(name, None, [variants.tag_member], variants)\n>> - self._gen_type_cleanup(name)\n>> + self.decl += gen_variants_objects(variants)\n>> + self._gen_object(name, info, None, None,\n>> + [variants.tag_member], variants)\n>\n> Where did self._gen_type_cleanup(name) go? Hmm, I guess it's now in\n> _gen_object(). Confusing.\n\nThis factors out common code that must be wrap by the same ifcond, in\nthe following patch.\n\n>>\n>> # If you link code generated from multiple schemata, you want only one\n>> # instance of the code for built-in types. Generate it only when\n>\n> If I read this patch correctly, you move code from the beginning of\n> gen_object() to new gen_variants_objects(), then arrange to have\n> gen_variants_objects() called right before gen_object(). Correct?\n>\n> In old gen_object(), the code is guarded by\n>\n> if name in objects_seen:\n> return ''\n> objects_seen.add(name)\n>\n> In new gen_variants_objects(), it isn't. Why?\n\ngen_variants_objects() calls gen_object() for each variants, so it\nremains guarded.","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ozlabs.org; dkim=pass (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"Dg1XXLmR\"; dkim-atps=neutral"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3zHf8R4L4bz9sNr\n\tfor ;\n\tFri, 12 Jan 2018 08:26:47 +1100 (AEDT)","from localhost ([::1]:43597 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1eZkN3-0006AC-LC\n\tfor incoming@patchwork.ozlabs.org; Thu, 11 Jan 2018 16:26:45 -0500","from eggs.gnu.org ([2001:4830:134:3::10]:43873)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1eZkIX-0002ez-Rw\n\tfor qemu-devel@nongnu.org; Thu, 11 Jan 2018 16:22:06 -0500","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1eZkIW-0008Cp-Oy\n\tfor qemu-devel@nongnu.org; Thu, 11 Jan 2018 16:22:05 -0500","from mail-wr0-x232.google.com ([2a00:1450:400c:c0c::232]:43632)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from )\n\tid 1eZkIW-0008CL-Ei\n\tfor qemu-devel@nongnu.org; Thu, 11 Jan 2018 16:22:04 -0500","by mail-wr0-x232.google.com with SMTP id s13so3519894wra.10\n\tfor ; Thu, 11 Jan 2018 13:22:04 -0800 (PST)","by 10.223.199.143 with HTTP; Thu, 11 Jan 2018 13:22:02 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=PB40Ou7/QdN8lDtrOWXtP+fVmOO0On3qekkoOxk6Lq0=;\n\tb=Dg1XXLmR8MBXy9BmCW16wjuND1xQRfAnAU8GhthPoE3LUr0eHrvXgkqQjMGaCnubwm\n\tnArGRXjS1MeY/xl+xir9YpUma9DOiLWaRd6JGuXqd7b+THN0h/m4F6diJOhAukPxDl6e\n\tBAS2dBcWgSkUviOnwA8cYZjaijBMG7ljb/m1hbO6jdDyZGQ5TQBNEfwCY6v5QxDijS9p\n\tWBPb5iAn+CHB5eNfhD9mvXkwa65bG6xpvVkoiX/DtY8ltj43N20aLyypDJUv56vJw/gn\n\t6DdTQIggumes9RPNYcKNY01AHKPq0MiVGHLpee6dV5AuJ49IYU4aPB4GxsAD4YgcNkAH\n\tOmdQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=PB40Ou7/QdN8lDtrOWXtP+fVmOO0On3qekkoOxk6Lq0=;\n\tb=hNeSkFPvmzKdblvJiBLmWM1ryyK0+hf32RuWlcfShHs7LRySQmFZ1jMIjcbWP5PG6X\n\thYexGuAqqZGWaT/MhoAmn6X5Qd544fHUuGaCFqlFph7YvoLhHkGZrM3iv8p/hO5cKugo\n\tkWJwTxnWAjG+OPH4u5rVFURFhNav4nelNeNp/dXN8M1bvtNhCtlw4jRzEAQ0sj8BW0L8\n\tEYXQbWsyhHvLAWS9J+D8pl7yya0rSwYtzNDahSBeZIpQ74zN4odYmse4koc5pKNQhtHU\n\th/TwqmJzbt1+dw1i8Vrnr92hyf9twk8i47fhvfl7ZavarWqx7HPtpMtSIGc3T2zVdC/S\n\t3lcA==","X-Gm-Message-State":"AKGB3mJfkYBM1Fwq9RbJkAL+XkJjwqctgiuPyn39ZjqwLl/P+SBB5/Em\n\t/XET1QXLu2aAO78/+DZ3BHNLKfqk8IOzd08y0t8=","X-Google-Smtp-Source":"ACJfBosZPnkoURSOuN8Ivs3Hk8saV0zdsQFcK2H0Ol2HPqRYnH1VB9yREpuU2SJmjR4bxuqOggVufOQyZhe6XJwhqbU=","X-Received":"by 10.223.197.69 with SMTP id s5mr22650353wrf.96.1515705723453; \n\tThu, 11 Jan 2018 13:22:03 -0800 (PST)","MIME-Version":"1.0","In-Reply-To":"<87h8t2o7i3.fsf@dusky.pond.sub.org>","References":"<20170911110623.24981-1-marcandre.lureau@redhat.com>\n\t<20170911110623.24981-16-marcandre.lureau@redhat.com>\n\t<87h8t2o7i3.fsf@dusky.pond.sub.org>","From":"=?utf-8?q?Marc-Andr=C3=A9_Lureau?= ","Date":"Thu, 11 Jan 2018 22:22:02 +0100","Message-ID":"","To":"Markus Armbruster ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2a00:1450:400c:c0c::232","Subject":"Re: [Qemu-devel] [PATCH v3 15/50] qapi-types: refactor variants\n\thandling","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"QEMU , Michael Roth ","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]