From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AA632C48BDF for ; Fri, 18 Jun 2021 10:34:37 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2256A613D1 for ; Fri, 18 Jun 2021 10:34:37 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2256A613D1 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:59366 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1luBpb-0002Er-CU for qemu-devel@archiver.kernel.org; Fri, 18 Jun 2021 06:34:36 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:60762) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1luBn8-0005n7-Ea for qemu-devel@nongnu.org; Fri, 18 Jun 2021 06:32:03 -0400 Received: from mail-ed1-x534.google.com ([2a00:1450:4864:20::534]:38448) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1luBn4-0004DR-2D for qemu-devel@nongnu.org; Fri, 18 Jun 2021 06:32:02 -0400 Received: by mail-ed1-x534.google.com with SMTP id t7so7964210edd.5 for ; Fri, 18 Jun 2021 03:31:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=PL/3AmmeSNvZdaEfSMyGTUHPcwo/XdHO6ZWRAFtzCUE=; b=BZsZyiPw3UYsOFsjTCt7V75K72KG6pA8p9R0eEmkYU+sp3jhHRMzGJ5Dv/Uw2I4tLm m59ld6yvrVkBHzhgqGjphQakwfkw9h6jGCMfwYQlRsgEqUQNIxtCfXdlP4oicS0kGOgP E/zw+6JrK+rFXjum3M7Jipar7YTHCR/BeUvpYsUaJrG/elAkFjJlNwejntSFHgokuk1p JEF3CF4r+yhIJaPBxF+H3wZHhsBldYxI0a33bGrKx6DmjmfQuM93PNl7HYfafcMeXz3g aEX0ZcukbPs3509CeHeMvk4czUbdXFHEQKJcpl0K6a0PKtLKE/QwOnIFVOmaLDV3LhjS yYlw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=PL/3AmmeSNvZdaEfSMyGTUHPcwo/XdHO6ZWRAFtzCUE=; b=NpbIL+DAl9J0VkMx67dBoP0Dk/lmLBjRKskIkUoOZOI+ITa3BAw4dqXPvnkqFDZhSp BK3MVZ91JRkzBS3zxM6m24Dawo9W4v4V0CAjoOEJ8l+amFUiwTV7f9xH3ZNrN2+SSPRi /iAdvlse6jtl7JDSb2aYYAM+mXMKc6QOKnQKB+Ti7K++iyNW4kxNRBnemkVn+f8n4kEy AgHMMcML5V8D7diICLGxaHErkoCu3Q+WUKWJOA5c82EkuV5dektTG+c50Rn/8mWNP83S JekI3ePBrppFYxbhKuJMks9gbiMdVHh8/bzPa9Clp18pRWMNtnG/aIJNknstXDH5ffW3 Is0w== X-Gm-Message-State: AOAM53044W9RpHvavY1KHFJELDW29jUzlyJLyG67NWlttPlhRERMZtaS oVwHYxIAqHhjmUSGA5B8M6kcOEa/ovayJ9mQDco= X-Google-Smtp-Source: ABdhPJycLKERjawGnH5m1SAQg79pMDEcukTUMtYBfB0LLMVYluDhqycWaTGpLt0O2zHjOqcln6t2xUkMpKCuplBQzUM= X-Received: by 2002:a05:6402:18f6:: with SMTP id x54mr4127519edy.53.1624012314048; Fri, 18 Jun 2021 03:31:54 -0700 (PDT) MIME-Version: 1.0 References: <20210608120723.2268181-1-marcandre.lureau@redhat.com> <20210608120723.2268181-5-marcandre.lureau@redhat.com> <87bl88y0is.fsf@dusky.pond.sub.org> In-Reply-To: <87bl88y0is.fsf@dusky.pond.sub.org> From: =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= Date: Fri, 18 Jun 2021 14:31:41 +0400 Message-ID: Subject: Re: [PATCH v5 4/9] qapi: start building an 'if' predicate tree To: Markus Armbruster Content-Type: multipart/alternative; boundary="00000000000016fa8205c507d564" Received-SPF: pass client-ip=2a00:1450:4864:20::534; envelope-from=marcandre.lureau@gmail.com; helo=mail-ed1-x534.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Eric Blake , John Snow , QEMU Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --00000000000016fa8205c507d564 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi On Mon, Jun 14, 2021 at 6:39 PM Markus Armbruster wrote= : > marcandre.lureau@redhat.com writes: > > > From: Marc-Andr=C3=A9 Lureau > > > > The following patches are going to express schema 'if' conditions in a > > target language agnostic way. For that, let's start building a predicat= e > > tree of the configuration options. > > > > This intermediary steps still uses C-preprocessor expressions as > > the predicates: > > > > "if: [STR, ..]" is translated to a "IfCond -> IfAll -> > > [IfOption, ..]" tree, which will generate "#if STR && .." C code. > > > > Once the boolean operation tree nodes are introduced, the 'if' > > expressions will be converted to replace the C syntax (no more > > !defined(), &&, ...) and based only on option identifiers. > > > > For now, the condition tree will be less expressive than a full C macro > > expression as it will only support these operations: 'all', 'any' and > > 'not', the only ones needed so far. > > > > Signed-off-by: Marc-Andr=C3=A9 Lureau > > Reviewed-by: Stefan Hajnoczi > > Tested-by: John Snow > > --- > > docs/sphinx/qapidoc.py | 6 +-- > > scripts/qapi/common.py | 53 ++++++++++++++++++++++- > > scripts/qapi/schema.py | 17 ++++++-- > > tests/qapi-schema/doc-good.out | 12 +++--- > > tests/qapi-schema/qapi-schema-test.out | 58 +++++++++++++------------- > > tests/qapi-schema/test-qapi.py | 2 +- > > 6 files changed, 103 insertions(+), 45 deletions(-) > > > > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py > > index b737949007..0f87fb16ce 100644 > > --- a/docs/sphinx/qapidoc.py > > +++ b/docs/sphinx/qapidoc.py > > @@ -112,12 +112,10 @@ def _make_section(self, title): > > def _nodes_for_ifcond(self, ifcond, with_if=3DTrue): > > """Return list of Text, literal nodes for the ifcond > > > > - Return a list which gives text like ' (If: cond1, cond2, > cond3)', where > > - the conditions are in literal-text and the commas are not. > > + Return a list which gives text like ' (If: condition)'. > > If with_if is False, we don't return the "(If: " and ")". > > """ > > - condlist =3D intersperse([nodes.literal('', c) for c in > ifcond.ifcond], > > - nodes.Text(', ')) > > + condlist =3D [nodes.literal('', ifcond.docgen())] > > if not with_if: > > return condlist > > > > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > > index c305aaf2f1..01e3203545 100644 > > --- a/scripts/qapi/common.py > > +++ b/scripts/qapi/common.py > > @@ -12,7 +12,7 @@ > > # See the COPYING file in the top-level directory. > > > > import re > > -from typing import Match, Optional > > +from typing import Match, Optional, Sequence > > > > > > #: Magic string that gets removed along with all space to its right. > > @@ -214,3 +214,54 @@ def must_match(pattern: str, string: str) -> > Match[str]: > > match =3D re.match(pattern, string) > > assert match is not None > > return match > > + > > + > > +class IfPredicate: > > This is the abstract base class of the two (soon four) forms 'if'. > qapi-code-gen.txt calls them "conditionals", and the code so far uses > names like @ifcond. I'd prefer not to throw "predicate" into the > cauldron. IfCond? IfConditional? > > ok > > + """An 'if' condition predicate""" > > + > > + def cgen(self) -> str: > > + raise NotImplementedError() > > + > > + def docgen(self) -> str: > > + raise NotImplementedError() > > + > > + > > +class IfOption(IfPredicate): > > The name IfOption tells me nothing. > > At this point in the series, the IfOption are arbitrary C preprocessor > expressions. > > At the end of the series, they are operands of the C preprocessor's > unary operator defined, i.e. a C macro name. > > Once I know that, IfOption kind of makes sense. Hmm. IfConfigOption? > IfIdentifier? IfLeaf? I'm not quite sure which one I dislike the least > :) > Ok IfConfigOption. > > > + def __init__(self, option: str): > > + self.option =3D option > > + > > + def cgen(self) -> str: > > + return self.option > > + > > + def docgen(self) -> str: > > + return self.option > > + > > + def __repr__(self) -> str: > > + return f"{type(self).__name__}({self.option!r})" > > Intended use? > %s in test-qapi > > + > > + def __eq__(self, other: object) -> bool: > > + if not isinstance(other, IfOption): > > + return NotImplemented > > + return self.option =3D=3D other.option > > Why raise on type mismatch? Feels rather un-pythonic to me. > removed > > + > > + > > +class IfAll(IfPredicate): > > + def __init__(self, pred_list: Sequence[IfPredicate]): > > + self.pred_list =3D pred_list > > + > > + def cgen(self) -> str: > > + return " && ".join([p.cgen() for p in self.pred_list]) > > Fragile. See my review of PATCH 3. > > ok > + > > + def docgen(self) -> str: > > + return " and ".join([p.docgen() for p in self.pred_list]) > > + > > + def __bool__(self) -> bool: > > + return bool(self.pred_list) > > Not as confusing as QAPISchemaIfCond.__bool__() as long as it's kept > well away from None. Still, I'm not sure I like it. > > removed > + > > + def __repr__(self) -> str: > > + return f"{type(self).__name__}({self.pred_list!r})" > > + > > + def __eq__(self, other: object) -> bool: > > + if not isinstance(other, IfAll): > > + return NotImplemented > > + return self.pred_list =3D=3D other.pred_list > > Same as above. > > Why are these classes in common.py? > moved to schema.py > > > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > > index aa4715c519..f52caaeecc 100644 > > --- a/scripts/qapi/schema.py > > +++ b/scripts/qapi/schema.py > > @@ -19,7 +19,12 @@ > > import re > > from typing import Optional > > > > -from .common import POINTER_SUFFIX, c_name > > +from .common import ( > > + POINTER_SUFFIX, > > + IfAll, > > + IfOption, > > + c_name, > > +) > > from .error import QAPIError, QAPISemError, QAPISourceError > > from .expr import check_exprs > > from .parser import QAPISchemaParser > > @@ -28,18 +33,22 @@ > > class QAPISchemaIfCond: > > def __init__(self, ifcond=3DNone): > > self.ifcond =3D ifcond or [] > > + self.pred =3D IfAll([IfOption(opt) for opt in self.ifcond]) > > In the common case of just one element, we get a no-op IfAll wrapped > around it. Okay. > > self.ifcond goes away in PATCH 7. Okay. > > > + > > + def docgen(self): > > + return self.pred.docgen() > > > > def cgen(self): > > - return ' && '.join(self.ifcond) > > + return self.pred.cgen() > > > > # Returns true if the condition is not void > > def __bool__(self): > > - return bool(self.ifcond) > > + return bool(self.pred) > > > > def __eq__(self, other): > > if not isinstance(other, QAPISchemaIfCond): > > return NotImplemented > > - return self.ifcond =3D=3D other.ifcond > > + return self.pred =3D=3D other.pred > > Not much left in this class, and it's going to get even thinner. > Yes, see v7. > > > > > > class QAPISchemaEntity: > > diff --git a/tests/qapi-schema/doc-good.out > b/tests/qapi-schema/doc-good.out > > index 8f54ceff2e..db1d23c6bf 100644 > > --- a/tests/qapi-schema/doc-good.out > > +++ b/tests/qapi-schema/doc-good.out > > @@ -12,15 +12,15 @@ enum QType > > module doc-good.json > > enum Enum > > member one > > - if ['defined(IFONE)'] > > + if IfAll([IfOption('defined(IFONE)')]) > > member two > > - if ['defined(IFCOND)'] > > + if IfAll([IfOption('defined(IFCOND)')]) > > feature enum-feat > > object Base > > member base1: Enum optional=3DFalse > > object Variant1 > > member var1: str optional=3DFalse > > - if ['defined(IFSTR)'] > > + if IfAll([IfOption('defined(IFSTR)')]) > > feature member-feat > > feature variant1-feat > > object Variant2 > > @@ -29,7 +29,7 @@ object Object > > tag base1 > > case one: Variant1 > > case two: Variant2 > > - if ['IFTWO'] > > + if IfAll([IfOption('IFTWO')]) > > feature union-feat1 > > object q_obj_Variant1-wrapper > > member data: Variant1 optional=3DFalse > > @@ -38,13 +38,13 @@ object q_obj_Variant2-wrapper > > enum SugaredUnionKind > > member one > > member two > > - if ['IFTWO'] > > + if IfAll([IfOption('IFTWO')]) > > object SugaredUnion > > member type: SugaredUnionKind optional=3DFalse > > tag type > > case one: q_obj_Variant1-wrapper > > case two: q_obj_Variant2-wrapper > > - if ['IFTWO'] > > + if IfAll([IfOption('IFTWO')]) > > feature union-feat2 > > alternate Alternate > > tag type > > diff --git a/tests/qapi-schema/qapi-schema-test.out > b/tests/qapi-schema/qapi-schema-test.out > > index e0b8a5f0b6..e4e0fb173a 100644 > > --- a/tests/qapi-schema/qapi-schema-test.out > > +++ b/tests/qapi-schema/qapi-schema-test.out > > @@ -298,65 +298,65 @@ command __org.qemu_x-command > q_obj___org.qemu_x-command-arg -> __org.qemu_x-Unio > > object TestIfStruct > > member foo: int optional=3DFalse > > member bar: int optional=3DFalse > > - if ['defined(TEST_IF_STRUCT_BAR)'] > > - if ['defined(TEST_IF_STRUCT)'] > > + if IfAll([IfOption('defined(TEST_IF_STRUCT_BAR)')]) > > + if IfAll([IfOption('defined(TEST_IF_STRUCT)')]) > > enum TestIfEnum > > member foo > > member bar > > - if ['defined(TEST_IF_ENUM_BAR)'] > > - if ['defined(TEST_IF_ENUM)'] > > + if IfAll([IfOption('defined(TEST_IF_ENUM_BAR)')]) > > + if IfAll([IfOption('defined(TEST_IF_ENUM)')]) > > object q_obj_TestStruct-wrapper > > member data: TestStruct optional=3DFalse > > enum TestIfUnionKind > > member foo > > member bar > > - if ['defined(TEST_IF_UNION_BAR)'] > > - if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)'] > > + if IfAll([IfOption('defined(TEST_IF_UNION_BAR)')]) > > + if IfAll([IfOption('defined(TEST_IF_UNION) && > defined(TEST_IF_STRUCT)')]) > > object TestIfUnion > > member type: TestIfUnionKind optional=3DFalse > > tag type > > case foo: q_obj_TestStruct-wrapper > > case bar: q_obj_str-wrapper > > - if ['defined(TEST_IF_UNION_BAR)'] > > - if ['defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)'] > > + if IfAll([IfOption('defined(TEST_IF_UNION_BAR)')]) > > + if IfAll([IfOption('defined(TEST_IF_UNION) && > defined(TEST_IF_STRUCT)')]) > > object q_obj_test-if-union-cmd-arg > > member union-cmd-arg: TestIfUnion optional=3DFalse > > - if ['defined(TEST_IF_UNION)'] > > + if IfAll([IfOption('defined(TEST_IF_UNION)')]) > > command test-if-union-cmd q_obj_test-if-union-cmd-arg -> None > > gen=3DTrue success_response=3DTrue boxed=3DFalse oob=3DFalse preco= nfig=3DFalse > > - if ['defined(TEST_IF_UNION)'] > > + if IfAll([IfOption('defined(TEST_IF_UNION)')]) > > alternate TestIfAlternate > > tag type > > case foo: int > > case bar: TestStruct > > - if ['defined(TEST_IF_ALT_BAR)'] > > - if ['defined(TEST_IF_ALT) && defined(TEST_IF_STRUCT)'] > > + if IfAll([IfOption('defined(TEST_IF_ALT_BAR)')]) > > + if IfAll([IfOption('defined(TEST_IF_ALT) && > defined(TEST_IF_STRUCT)')]) > > object q_obj_test-if-alternate-cmd-arg > > member alt-cmd-arg: TestIfAlternate optional=3DFalse > > - if ['defined(TEST_IF_ALT)'] > > + if IfAll([IfOption('defined(TEST_IF_ALT)')]) > > command test-if-alternate-cmd q_obj_test-if-alternate-cmd-arg -> None > > gen=3DTrue success_response=3DTrue boxed=3DFalse oob=3DFalse preco= nfig=3DFalse > > - if ['defined(TEST_IF_ALT)'] > > + if IfAll([IfOption('defined(TEST_IF_ALT)')]) > > object q_obj_test-if-cmd-arg > > member foo: TestIfStruct optional=3DFalse > > member bar: TestIfEnum optional=3DFalse > > - if ['defined(TEST_IF_CMD_BAR)'] > > - if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] > > + if IfAll([IfOption('defined(TEST_IF_CMD_BAR)')]) > > + if IfAll([IfOption('defined(TEST_IF_CMD)'), > IfOption('defined(TEST_IF_STRUCT)')]) > > command test-if-cmd q_obj_test-if-cmd-arg -> UserDefThree > > gen=3DTrue success_response=3DTrue boxed=3DFalse oob=3DFalse preco= nfig=3DFalse > > - if ['defined(TEST_IF_CMD)', 'defined(TEST_IF_STRUCT)'] > > + if IfAll([IfOption('defined(TEST_IF_CMD)'), > IfOption('defined(TEST_IF_STRUCT)')]) > > command test-cmd-return-def-three None -> UserDefThree > > gen=3DTrue success_response=3DTrue boxed=3DFalse oob=3DFalse preco= nfig=3DFalse > > array TestIfEnumList TestIfEnum > > - if ['defined(TEST_IF_ENUM)'] > > + if IfAll([IfOption('defined(TEST_IF_ENUM)')]) > > object q_obj_TEST_IF_EVENT-arg > > member foo: TestIfStruct optional=3DFalse > > member bar: TestIfEnumList optional=3DFalse > > - if ['defined(TEST_IF_EVT_BAR)'] > > - if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)'] > > + if IfAll([IfOption('defined(TEST_IF_EVT_BAR)')]) > > + if IfAll([IfOption('defined(TEST_IF_EVT) && > defined(TEST_IF_STRUCT)')]) > > event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg > > boxed=3DFalse > > - if ['defined(TEST_IF_EVT) && defined(TEST_IF_STRUCT)'] > > + if IfAll([IfOption('defined(TEST_IF_EVT) && > defined(TEST_IF_STRUCT)')]) > > object FeatureStruct0 > > member foo: int optional=3DFalse > > object FeatureStruct1 > > @@ -379,17 +379,17 @@ object FeatureStruct4 > > object CondFeatureStruct1 > > member foo: int optional=3DFalse > > feature feature1 > > - if ['defined(TEST_IF_FEATURE_1)'] > > + if IfAll([IfOption('defined(TEST_IF_FEATURE_1)')]) > > object CondFeatureStruct2 > > member foo: int optional=3DFalse > > feature feature1 > > - if ['defined(TEST_IF_FEATURE_1)'] > > + if IfAll([IfOption('defined(TEST_IF_FEATURE_1)')]) > > feature feature2 > > - if ['defined(TEST_IF_FEATURE_2)'] > > + if IfAll([IfOption('defined(TEST_IF_FEATURE_2)')]) > > object CondFeatureStruct3 > > member foo: int optional=3DFalse > > feature feature1 > > - if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'] > > + if IfAll([IfOption('defined(TEST_IF_COND_1)'), > IfOption('defined(TEST_IF_COND_2)')]) > > enum FeatureEnum1 > > member eins > > member zwei > > @@ -429,17 +429,17 @@ command test-command-features3 None -> None > > command test-command-cond-features1 None -> None > > gen=3DTrue success_response=3DTrue boxed=3DFalse oob=3DFalse preco= nfig=3DFalse > > feature feature1 > > - if ['defined(TEST_IF_FEATURE_1)'] > > + if IfAll([IfOption('defined(TEST_IF_FEATURE_1)')]) > > command test-command-cond-features2 None -> None > > gen=3DTrue success_response=3DTrue boxed=3DFalse oob=3DFalse preco= nfig=3DFalse > > feature feature1 > > - if ['defined(TEST_IF_FEATURE_1)'] > > + if IfAll([IfOption('defined(TEST_IF_FEATURE_1)')]) > > feature feature2 > > - if ['defined(TEST_IF_FEATURE_2)'] > > + if IfAll([IfOption('defined(TEST_IF_FEATURE_2)')]) > > command test-command-cond-features3 None -> None > > gen=3DTrue success_response=3DTrue boxed=3DFalse oob=3DFalse preco= nfig=3DFalse > > feature feature1 > > - if ['defined(TEST_IF_COND_1)', 'defined(TEST_IF_COND_2)'] > > + if IfAll([IfOption('defined(TEST_IF_COND_1)'), > IfOption('defined(TEST_IF_COND_2)')]) > > event TEST_EVENT_FEATURES0 FeatureStruct1 > > boxed=3DFalse > > event TEST_EVENT_FEATURES1 None > > diff --git a/tests/qapi-schema/test-qapi.py > b/tests/qapi-schema/test-qapi.py > > index 2ec328b22e..631e255fba 100755 > > --- a/tests/qapi-schema/test-qapi.py > > +++ b/tests/qapi-schema/test-qapi.py > > @@ -95,7 +95,7 @@ def _print_variants(variants): > > @staticmethod > > def _print_if(ifcond, indent=3D4): > > if ifcond: > > - print('%sif %s' % (' ' * indent, ifcond.ifcond)) > > + print('%sif %s' % (' ' * indent, ifcond.pred)) > > > > @classmethod > > def _print_features(cls, features, indent=3D4): > > > --=20 Marc-Andr=C3=A9 Lureau --00000000000016fa8205c507d564 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

On Mon, Jun 14, 2021 at 6:39 PM Mar= kus Armbruster <a= rmbru@redhat.com> wrote:
marcandre.lureau@redhat.com writes:

> From: Marc-Andr=C3=A9 Lureau <marcandre.lureau@redhat.com>
>
> The following patches are going to express schema 'if' conditi= ons in a
> target language agnostic way. For that, let's start building a pre= dicate
> tree of the configuration options.
>
> This intermediary steps still uses C-preprocessor expressions as
> the predicates:
>
> "if: [STR, ..]" is translated to a "IfCond -> IfAll = ->
> [IfOption, ..]" tree, which will generate "#if STR &&= ; .." C code.
>
> Once the boolean operation tree nodes are introduced, the 'if'=
> expressions will be converted to replace the C syntax (no more
> !defined(), &&, ...) and based only on option identifiers.
>
> For now, the condition tree will be less expressive than a full C macr= o
> expression as it will only support these operations: 'all', &#= 39;any' and
> 'not', the only ones needed so far.
>
> Signed-off-by: Marc-Andr=C3=A9 Lureau <marcandre.lureau@redhat.com> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Tested-by: John Snow <jsnow@redhat.com>
> ---
>=C2=A0 docs/sphinx/qapidoc.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0|=C2=A0 6 +--
>=C2=A0 scripts/qapi/common.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0| 53 ++++++++++++++++++++++-
>=C2=A0 scripts/qapi/schema.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0| 17 ++++++--
>=C2=A0 tests/qapi-schema/doc-good.out=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= | 12 +++---
>=C2=A0 tests/qapi-schema/qapi-schema-test.out | 58 +++++++++++++-------= ------
>=C2=A0 tests/qapi-schema/test-qapi.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= |=C2=A0 2 +-
>=C2=A0 6 files changed, 103 insertions(+), 45 deletions(-)
>
> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> index b737949007..0f87fb16ce 100644
> --- a/docs/sphinx/qapidoc.py
> +++ b/docs/sphinx/qapidoc.py
> @@ -112,12 +112,10 @@ def _make_section(self, title):
>=C2=A0 =C2=A0 =C2=A0 def _nodes_for_ifcond(self, ifcond, with_if=3DTrue= ):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """Return list of Tex= t, literal nodes for the ifcond
>=C2=A0
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 Return a list which gives text like '= (If: cond1, cond2, cond3)', where
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 the conditions are in literal-text and th= e commas are not.
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 Return a list which gives text like '= (If: condition)'.
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 If with_if is False, we don't re= turn the "(If: " and ")".
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 """
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 condlist =3D intersperse([nodes.literal(&= #39;', c) for c in ifcond.ifcond],
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0nodes.Text(', '))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 condlist =3D [nodes.literal('', i= fcond.docgen())]
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if not with_if:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return condlist
>=C2=A0
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index c305aaf2f1..01e3203545 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -12,7 +12,7 @@
>=C2=A0 # See the COPYING file in the top-level directory.
>=C2=A0
>=C2=A0 import re
> -from typing import Match, Optional
> +from typing import Match, Optional, Sequence
>=C2=A0
>=C2=A0
>=C2=A0 #: Magic string that gets removed along with all space to its ri= ght.
> @@ -214,3 +214,54 @@ def must_match(pattern: str, string: str) -> M= atch[str]:
>=C2=A0 =C2=A0 =C2=A0 match =3D re.match(pattern, string)
>=C2=A0 =C2=A0 =C2=A0 assert match is not None
>=C2=A0 =C2=A0 =C2=A0 return match
> +
> +
> +class IfPredicate:

This is the abstract base class of the two (soon four) forms 'if'.<= br> qapi-code-gen.txt calls them "conditionals", and the code so far = uses
names like @ifcond.=C2=A0 I'd prefer not to throw "predicate"= into the
cauldron.=C2=A0 IfCond?=C2=A0 IfConditional?


ok
=C2=A0
> +=C2=A0 =C2=A0 """An 'if' condition predicate&q= uot;""
> +
> +=C2=A0 =C2=A0 def cgen(self) -> str:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 raise NotImplementedError()
> +
> +=C2=A0 =C2=A0 def docgen(self) -> str:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 raise NotImplementedError()
> +
> +
> +class IfOption(IfPredicate):

The name IfOption tells me nothing.

At this point in the series, the IfOption are arbitrary C preprocessor
expressions.

At the end of the series, they are operands of the C preprocessor's
unary operator defined, i.e. a C macro name.

Once I know that, IfOption kind of makes sense.=C2=A0 Hmm.=C2=A0 IfConfigOp= tion?
IfIdentifier?=C2=A0 IfLeaf?=C2=A0 I'm not quite sure which one I dislik= e the least
:)

Ok IfConfigOption.

> +=C2=A0 =C2=A0 def __init__(self, option: str):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.option =3D option
> +
> +=C2=A0 =C2=A0 def cgen(self) -> str:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return self.option
> +
> +=C2=A0 =C2=A0 def docgen(self) -> str:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return self.option
> +
> +=C2=A0 =C2=A0 def __repr__(self) -> str:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return f"{type(self).__name__}({self= .option!r})"

Intended use?

%s in test-qapi


> +
> +=C2=A0 =C2=A0 def __eq__(self, other: object) -> bool:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if not isinstance(other, IfOption):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return NotImplemented
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return self.option =3D=3D other.option
Why raise on type mismatch?=C2=A0 Feels rather un-pythonic to me.

removed


> +
> +
> +class IfAll(IfPredicate):
> +=C2=A0 =C2=A0 def __init__(self, pred_list: Sequence[IfPredicate]): > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.pred_list =3D pred_list
> +
> +=C2=A0 =C2=A0 def cgen(self) -> str:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return " && ".join([p.c= gen() for p in self.pred_list])

Fragile.=C2=A0 See my review of PATCH 3.


ok

> +
> +=C2=A0 =C2=A0 def docgen(self) -> str:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return " and ".join([p.docgen()= for p in self.pred_list])
> +
> +=C2=A0 =C2=A0 def __bool__(self) -> bool:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return bool(self.pred_list)

Not as confusing as QAPISchemaIfCond.__bool__() as long as it's kept well away from None.=C2=A0 Still, I'm not sure I like it.


removed

> +
> +=C2=A0 =C2=A0 def __repr__(self) -> str:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return f"{type(self).__name__}({self= .pred_list!r})"
> +
> +=C2=A0 =C2=A0 def __eq__(self, other: object) -> bool:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if not isinstance(other, IfAll):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return NotImplemented
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return self.pred_list =3D=3D other.pred_l= ist

Same as above.

Why are these classes in common.py?

mov= ed to schema.py
=C2=A0

> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index aa4715c519..f52caaeecc 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -19,7 +19,12 @@
>=C2=A0 import re
>=C2=A0 from typing import Optional
>=C2=A0
> -from .common import POINTER_SUFFIX, c_name
> +from .common import (
> +=C2=A0 =C2=A0 POINTER_SUFFIX,
> +=C2=A0 =C2=A0 IfAll,
> +=C2=A0 =C2=A0 IfOption,
> +=C2=A0 =C2=A0 c_name,
> +)
>=C2=A0 from .error import QAPIError, QAPISemError, QAPISourceError
>=C2=A0 from .expr import check_exprs
>=C2=A0 from .parser import QAPISchemaParser
> @@ -28,18 +33,22 @@
>=C2=A0 class QAPISchemaIfCond:
>=C2=A0 =C2=A0 =C2=A0 def __init__(self, ifcond=3DNone):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 self.ifcond =3D ifcond or []
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 self.pred =3D IfAll([IfOption(opt) for op= t in self.ifcond])

In the common case of just one element, we get a no-op IfAll wrapped
around it.=C2=A0 Okay.

self.ifcond goes away in PATCH 7.=C2=A0 Okay.

> +
> +=C2=A0 =C2=A0 def docgen(self):
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return self.pred.docgen()
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 def cgen(self):
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 return ' && '.join(self.i= fcond)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return self.pred.cgen()
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 # Returns true if the condition is not void
>=C2=A0 =C2=A0 =C2=A0 def __bool__(self):
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 return bool(self.ifcond)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return bool(self.pred)
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 def __eq__(self, other):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if not isinstance(other, QAPISchemaI= fCond):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return NotImplemented<= br> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 return self.ifcond =3D=3D other.ifcond > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 return self.pred =3D=3D other.pred

Not much left in this class, and it's going to get even thinner.

Yes, see v7.


>=C2=A0
>=C2=A0
>=C2=A0 class QAPISchemaEntity:
> diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-go= od.out
> index 8f54ceff2e..db1d23c6bf 100644
> --- a/tests/qapi-schema/doc-good.out
> +++ b/tests/qapi-schema/doc-good.out
> @@ -12,15 +12,15 @@ enum QType
>=C2=A0 module doc-good.json
>=C2=A0 enum Enum
>=C2=A0 =C2=A0 =C2=A0 member one
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(IFONE)']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(IFONE)= 9;)])
>=C2=A0 =C2=A0 =C2=A0 member two
> -=C2=A0 =C2=A0 if ['defined(IFCOND)']
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(IFCOND)')])
>=C2=A0 =C2=A0 =C2=A0 feature enum-feat
>=C2=A0 object Base
>=C2=A0 =C2=A0 =C2=A0 member base1: Enum optional=3DFalse
>=C2=A0 object Variant1
>=C2=A0 =C2=A0 =C2=A0 member var1: str optional=3DFalse
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(IFSTR)']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(IFSTR)= 9;)])
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 feature member-feat
>=C2=A0 =C2=A0 =C2=A0 feature variant1-feat
>=C2=A0 object Variant2
> @@ -29,7 +29,7 @@ object Object
>=C2=A0 =C2=A0 =C2=A0 tag base1
>=C2=A0 =C2=A0 =C2=A0 case one: Variant1
>=C2=A0 =C2=A0 =C2=A0 case two: Variant2
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['IFTWO']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('IFTWO')])
>=C2=A0 =C2=A0 =C2=A0 feature union-feat1
>=C2=A0 object q_obj_Variant1-wrapper
>=C2=A0 =C2=A0 =C2=A0 member data: Variant1 optional=3DFalse
> @@ -38,13 +38,13 @@ object q_obj_Variant2-wrapper
>=C2=A0 enum SugaredUnionKind
>=C2=A0 =C2=A0 =C2=A0 member one
>=C2=A0 =C2=A0 =C2=A0 member two
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['IFTWO']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('IFTWO')])
>=C2=A0 object SugaredUnion
>=C2=A0 =C2=A0 =C2=A0 member type: SugaredUnionKind optional=3DFalse
>=C2=A0 =C2=A0 =C2=A0 tag type
>=C2=A0 =C2=A0 =C2=A0 case one: q_obj_Variant1-wrapper
>=C2=A0 =C2=A0 =C2=A0 case two: q_obj_Variant2-wrapper
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['IFTWO']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('IFTWO')])
>=C2=A0 =C2=A0 =C2=A0 feature union-feat2
>=C2=A0 alternate Alternate
>=C2=A0 =C2=A0 =C2=A0 tag type
> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schem= a/qapi-schema-test.out
> index e0b8a5f0b6..e4e0fb173a 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -298,65 +298,65 @@ command __org.qemu_x-command q_obj___org.qemu_x-= command-arg -> __org.qemu_x-Unio
>=C2=A0 object TestIfStruct
>=C2=A0 =C2=A0 =C2=A0 member foo: int optional=3DFalse
>=C2=A0 =C2=A0 =C2=A0 member bar: int optional=3DFalse
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_STRUCT_BAR)'= ]
> -=C2=A0 =C2=A0 if ['defined(TEST_IF_STRUCT)']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_S= TRUCT_BAR)')])
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_STRUCT)')])=
>=C2=A0 enum TestIfEnum
>=C2=A0 =C2=A0 =C2=A0 member foo
>=C2=A0 =C2=A0 =C2=A0 member bar
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_ENUM_BAR)']<= br> > -=C2=A0 =C2=A0 if ['defined(TEST_IF_ENUM)']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_E= NUM_BAR)')])
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_ENUM)')]) >=C2=A0 object q_obj_TestStruct-wrapper
>=C2=A0 =C2=A0 =C2=A0 member data: TestStruct optional=3DFalse
>=C2=A0 enum TestIfUnionKind
>=C2=A0 =C2=A0 =C2=A0 member foo
>=C2=A0 =C2=A0 =C2=A0 member bar
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_UNION_BAR)']=
> -=C2=A0 =C2=A0 if ['defined(TEST_IF_UNION) && defined(TEST= _IF_STRUCT)']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_U= NION_BAR)')])
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_UNION) &&am= p; defined(TEST_IF_STRUCT)')])
>=C2=A0 object TestIfUnion
>=C2=A0 =C2=A0 =C2=A0 member type: TestIfUnionKind optional=3DFalse
>=C2=A0 =C2=A0 =C2=A0 tag type
>=C2=A0 =C2=A0 =C2=A0 case foo: q_obj_TestStruct-wrapper
>=C2=A0 =C2=A0 =C2=A0 case bar: q_obj_str-wrapper
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_UNION_BAR)']=
> -=C2=A0 =C2=A0 if ['defined(TEST_IF_UNION) && defined(TEST= _IF_STRUCT)']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_U= NION_BAR)')])
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_UNION) &&am= p; defined(TEST_IF_STRUCT)')])
>=C2=A0 object q_obj_test-if-union-cmd-arg
>=C2=A0 =C2=A0 =C2=A0 member union-cmd-arg: TestIfUnion optional=3DFalse=
> -=C2=A0 =C2=A0 if ['defined(TEST_IF_UNION)']
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_UNION)')])<= br> >=C2=A0 command test-if-union-cmd q_obj_test-if-union-cmd-arg -> None=
>=C2=A0 =C2=A0 =C2=A0 gen=3DTrue success_response=3DTrue boxed=3DFalse o= ob=3DFalse preconfig=3DFalse
> -=C2=A0 =C2=A0 if ['defined(TEST_IF_UNION)']
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_UNION)')])<= br> >=C2=A0 alternate TestIfAlternate
>=C2=A0 =C2=A0 =C2=A0 tag type
>=C2=A0 =C2=A0 =C2=A0 case foo: int
>=C2=A0 =C2=A0 =C2=A0 case bar: TestStruct
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_ALT_BAR)'] > -=C2=A0 =C2=A0 if ['defined(TEST_IF_ALT) && defined(TEST_I= F_STRUCT)']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_A= LT_BAR)')])
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_ALT) &&= defined(TEST_IF_STRUCT)')])
>=C2=A0 object q_obj_test-if-alternate-cmd-arg
>=C2=A0 =C2=A0 =C2=A0 member alt-cmd-arg: TestIfAlternate optional=3DFal= se
> -=C2=A0 =C2=A0 if ['defined(TEST_IF_ALT)']
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_ALT)')]) >=C2=A0 command test-if-alternate-cmd q_obj_test-if-alternate-cmd-arg -&= gt; None
>=C2=A0 =C2=A0 =C2=A0 gen=3DTrue success_response=3DTrue boxed=3DFalse o= ob=3DFalse preconfig=3DFalse
> -=C2=A0 =C2=A0 if ['defined(TEST_IF_ALT)']
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_ALT)')]) >=C2=A0 object q_obj_test-if-cmd-arg
>=C2=A0 =C2=A0 =C2=A0 member foo: TestIfStruct optional=3DFalse
>=C2=A0 =C2=A0 =C2=A0 member bar: TestIfEnum optional=3DFalse
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_CMD_BAR)'] > -=C2=A0 =C2=A0 if ['defined(TEST_IF_CMD)', 'defined(TEST_I= F_STRUCT)']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_C= MD_BAR)')])
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_CMD)'), IfO= ption('defined(TEST_IF_STRUCT)')])
>=C2=A0 command test-if-cmd q_obj_test-if-cmd-arg -> UserDefThree
>=C2=A0 =C2=A0 =C2=A0 gen=3DTrue success_response=3DTrue boxed=3DFalse o= ob=3DFalse preconfig=3DFalse
> -=C2=A0 =C2=A0 if ['defined(TEST_IF_CMD)', 'defined(TEST_I= F_STRUCT)']
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_CMD)'), IfO= ption('defined(TEST_IF_STRUCT)')])
>=C2=A0 command test-cmd-return-def-three None -> UserDefThree
>=C2=A0 =C2=A0 =C2=A0 gen=3DTrue success_response=3DTrue boxed=3DFalse o= ob=3DFalse preconfig=3DFalse
>=C2=A0 array TestIfEnumList TestIfEnum
> -=C2=A0 =C2=A0 if ['defined(TEST_IF_ENUM)']
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_ENUM)')]) >=C2=A0 object q_obj_TEST_IF_EVENT-arg
>=C2=A0 =C2=A0 =C2=A0 member foo: TestIfStruct optional=3DFalse
>=C2=A0 =C2=A0 =C2=A0 member bar: TestIfEnumList optional=3DFalse
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_EVT_BAR)'] > -=C2=A0 =C2=A0 if ['defined(TEST_IF_EVT) && defined(TEST_I= F_STRUCT)']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_E= VT_BAR)')])
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_EVT) &&= defined(TEST_IF_STRUCT)')])
>=C2=A0 event TEST_IF_EVENT q_obj_TEST_IF_EVENT-arg
>=C2=A0 =C2=A0 =C2=A0 boxed=3DFalse
> -=C2=A0 =C2=A0 if ['defined(TEST_IF_EVT) && defined(TEST_I= F_STRUCT)']
> +=C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_EVT) &&= defined(TEST_IF_STRUCT)')])
>=C2=A0 object FeatureStruct0
>=C2=A0 =C2=A0 =C2=A0 member foo: int optional=3DFalse
>=C2=A0 object FeatureStruct1
> @@ -379,17 +379,17 @@ object FeatureStruct4
>=C2=A0 object CondFeatureStruct1
>=C2=A0 =C2=A0 =C2=A0 member foo: int optional=3DFalse
>=C2=A0 =C2=A0 =C2=A0 feature feature1
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_FEATURE_1)']=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_F= EATURE_1)')])
>=C2=A0 object CondFeatureStruct2
>=C2=A0 =C2=A0 =C2=A0 member foo: int optional=3DFalse
>=C2=A0 =C2=A0 =C2=A0 feature feature1
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_FEATURE_1)']=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_F= EATURE_1)')])
>=C2=A0 =C2=A0 =C2=A0 feature feature2
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_FEATURE_2)']=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_F= EATURE_2)')])
>=C2=A0 object CondFeatureStruct3
>=C2=A0 =C2=A0 =C2=A0 member foo: int optional=3DFalse
>=C2=A0 =C2=A0 =C2=A0 feature feature1
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_COND_1)', &#= 39;defined(TEST_IF_COND_2)']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_C= OND_1)'), IfOption('defined(TEST_IF_COND_2)')])
>=C2=A0 enum FeatureEnum1
>=C2=A0 =C2=A0 =C2=A0 member eins
>=C2=A0 =C2=A0 =C2=A0 member zwei
> @@ -429,17 +429,17 @@ command test-command-features3 None -> None >=C2=A0 command test-command-cond-features1 None -> None
>=C2=A0 =C2=A0 =C2=A0 gen=3DTrue success_response=3DTrue boxed=3DFalse o= ob=3DFalse preconfig=3DFalse
>=C2=A0 =C2=A0 =C2=A0 feature feature1
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_FEATURE_1)']=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_F= EATURE_1)')])
>=C2=A0 command test-command-cond-features2 None -> None
>=C2=A0 =C2=A0 =C2=A0 gen=3DTrue success_response=3DTrue boxed=3DFalse o= ob=3DFalse preconfig=3DFalse
>=C2=A0 =C2=A0 =C2=A0 feature feature1
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_FEATURE_1)']=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_F= EATURE_1)')])
>=C2=A0 =C2=A0 =C2=A0 feature feature2
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_FEATURE_2)']=
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_F= EATURE_2)')])
>=C2=A0 command test-command-cond-features3 None -> None
>=C2=A0 =C2=A0 =C2=A0 gen=3DTrue success_response=3DTrue boxed=3DFalse o= ob=3DFalse preconfig=3DFalse
>=C2=A0 =C2=A0 =C2=A0 feature feature1
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 if ['defined(TEST_IF_COND_1)', &#= 39;defined(TEST_IF_COND_2)']
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 if IfAll([IfOption('defined(TEST_IF_C= OND_1)'), IfOption('defined(TEST_IF_COND_2)')])
>=C2=A0 event TEST_EVENT_FEATURES0 FeatureStruct1
>=C2=A0 =C2=A0 =C2=A0 boxed=3DFalse
>=C2=A0 event TEST_EVENT_FEATURES1 None
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-q= api.py
> index 2ec328b22e..631e255fba 100755
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -95,7 +95,7 @@ def _print_variants(variants):
>=C2=A0 =C2=A0 =C2=A0 @staticmethod
>=C2=A0 =C2=A0 =C2=A0 def _print_if(ifcond, indent=3D4):
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if ifcond:
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 print('%sif %s' % (= ' ' * indent, ifcond.ifcond))
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 print('%sif %s' % (= ' ' * indent, ifcond.pred))
>=C2=A0
>=C2=A0 =C2=A0 =C2=A0 @classmethod
>=C2=A0 =C2=A0 =C2=A0 def _print_features(cls, features, indent=3D4):



--
Marc-Andr= =C3=A9 Lureau
--00000000000016fa8205c507d564--