| Message ID | 20210618102507.3761128-6-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: untie 'if' conditions from C preprocessor | expand |
marcandre.lureau@redhat.com writes: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > Instead of building prepocessor conditions from a list of string, use > the result generated from QAPISchemaIfCond.cgen() and hide the > implementation details. > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Please mention that the patch changes generated code. See below for details. > --- > scripts/qapi/common.py | 35 ++++++++++++++++++++++------------- > scripts/qapi/gen.py | 4 ++-- > scripts/qapi/introspect.py | 4 ++-- > scripts/qapi/schema.py | 5 ++++- > scripts/qapi/types.py | 20 ++++++++++---------- > scripts/qapi/visit.py | 12 ++++++------ > 6 files changed, 46 insertions(+), 34 deletions(-) > > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > index 6ad1eeb61d..ba9fe14e4b 100644 > --- a/scripts/qapi/common.py > +++ b/scripts/qapi/common.py > @@ -12,7 +12,12 @@ > # See the COPYING file in the top-level directory. > > import re > -from typing import Match, Optional, Sequence > +from typing import ( > + List, > + Match, > + Optional, > + Union, > +) > > > #: Magic string that gets removed along with all space to its right. > @@ -194,22 +199,26 @@ def guardend(name: str) -> str: > name=c_fname(name).upper()) > > > -def gen_if(ifcond: Sequence[str]) -> str: > - ret = '' > - for ifc in ifcond: > - ret += mcgen(''' > +def cgen_ifcond(ifcond: Union[str, List[str]]) -> str: > + if not ifcond: > + return '' > + return '(' + ') && ('.join(ifcond) + ')' > + > + > +def gen_if(cond: str) -> str: > + if not cond: > + return '' > + return mcgen(''' > #if %(cond)s > -''', cond=ifc) > - return ret > +''', cond=cond) > > > -def gen_endif(ifcond: Sequence[str]) -> str: > - ret = '' > - for ifc in reversed(ifcond): > - ret += mcgen(''' > +def gen_endif(cond: str) -> str: > + if not cond: > + return '' > + return mcgen(''' > #endif /* %(cond)s */ > -''', cond=ifc) > - return ret > +''', cond=cond) > > This patch does three things: (1) Change gen_if(), gen_endif() to always generate a single #if, #endif. This enables: (2) Factor cgen_ifcond() out of gen_if() and gen_endif() (3) Lift the call of cgen_ifcond() into into gen_if()'s, gen_endif()'s callers. I'd split the patch. This is *not* a demand. The motivation for (3) is unclear. Is it so gen_if() doesn't depend on QAPISchemaIfCond? Step (1) affects the generated code. When @ifcond is [COND1, COND2, ...], gen_if()'s value changes from #if COND1 #if COND2 ... to #if (COND1) && (COND2) Example: in tests/test-qapi-introspect.c @@ -259,11 +257,9 @@ const QLitObject test_qmp_schema_qlit = QLIT_QDICT(((QLitDictEntry[]) { { "arg-type", QLIT_QSTR("1"), }, { "features", QLIT_QLIST(((QLitObject[]) { -#if defined(TEST_IF_COND_1) -#if defined(TEST_IF_COND_2) +#if (defined(TEST_IF_COND_1)) && (defined(TEST_IF_COND_2)) QLIT_QSTR("feature1"), -#endif /* defined(TEST_IF_COND_2) */ -#endif /* defined(TEST_IF_COND_1) */ +#endif /* (defined(TEST_IF_COND_1)) && (defined(TEST_IF_COND_2)) */ {} })), }, { "meta-type", QLIT_QSTR("command"), }, The common case: when it's just [COND], the value changes from #if COND to #if (COND) which is a bit ugly. Example: in qapi-types-block-export.c @@ -76,7 +76,7 @@ const QEnumLookup FuseExportAllowOther_l .size = FUSE_EXPORT_ALLOW_OTHER__MAX }; -#if defined(CONFIG_FUSE) +#if (defined(CONFIG_FUSE)) void qapi_free_BlockExportOptionsFuse(BlockExportOptionsFuse *obj) { Visitor *v; @@ -89,7 +89,7 @@ void qapi_free_BlockExportOptionsFuse(Bl visit_type_BlockExportOptionsFuse(v, NULL, &obj, NULL); visit_free(v); } -#endif /* defined(CONFIG_FUSE) */ +#endif /* (defined(CONFIG_FUSE)) */ void qapi_free_NbdServerAddOptions(NbdServerAddOptions *obj) { Avoiding the redundant pair of parenthesis takes another special case. Let's do it. > def must_match(pattern: str, string: str) -> Match[str]: > diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py > index 1c5b190276..51a597a025 100644 > --- a/scripts/qapi/gen.py > +++ b/scripts/qapi/gen.py > @@ -95,9 +95,9 @@ def _wrap_ifcond(ifcond: QAPISchemaIfCond, before: str, after: str) -> str: > if added[0] == '\n': > out += '\n' > added = added[1:] > - out += gen_if(ifcond.ifcond) > + out += gen_if(ifcond.cgen()) > out += added > - out += gen_endif(ifcond.ifcond) > + out += gen_endif(ifcond.cgen()) > return out > > > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > index 77a8c33ad4..474b08fd4d 100644 > --- a/scripts/qapi/introspect.py > +++ b/scripts/qapi/introspect.py > @@ -124,10 +124,10 @@ def indent(level: int) -> str: > if obj.comment: > ret += indent(level) + f"/* {obj.comment} */\n" > if obj.ifcond: > - ret += gen_if(obj.ifcond.ifcond) > + ret += gen_if(obj.ifcond.cgen()) > ret += _tree_to_qlit(obj.value, level) > if obj.ifcond: > - ret += '\n' + gen_endif(obj.ifcond.ifcond) > + ret += '\n' + gen_endif(obj.ifcond.cgen()) > return ret > > ret = '' > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > index c35fa3bf51..70120f0dcc 100644 > --- a/scripts/qapi/schema.py > +++ b/scripts/qapi/schema.py > @@ -19,7 +19,7 @@ > import re > from typing import Optional > > -from .common import POINTER_SUFFIX, c_name > +from .common import POINTER_SUFFIX, c_name, cgen_ifcond > from .error import QAPIError, QAPISemError, QAPISourceError > from .expr import check_exprs > from .parser import QAPISchemaParser > @@ -29,6 +29,9 @@ class QAPISchemaIfCond: > def __init__(self, ifcond=None): > self.ifcond = ifcond or [] > > + def cgen(self): > + return cgen_ifcond(self.ifcond) > + As far as I can tell, this is only ever used like gen_if(obj.ifcond.cgen()) and gen_endif(obj.ifcond.cgen()) Wouldn't obj.ifcond.gen_if() and obj.ifcond.gen_endif() be neater? Not a demand, since we can get there on top if we want to. > def is_present(self): > return bool(self.ifcond) > > diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py > index 3673cf0f49..db9ff95bd1 100644 > --- a/scripts/qapi/types.py > +++ b/scripts/qapi/types.py > @@ -51,13 +51,13 @@ def gen_enum_lookup(name: str, > ''', > c_name=c_name(name)) > for memb in members: > - ret += gen_if(memb.ifcond.ifcond) > + ret += gen_if(memb.ifcond.cgen()) > index = c_enum_const(name, memb.name, prefix) > ret += mcgen(''' > [%(index)s] = "%(name)s", > ''', > index=index, name=memb.name) > - ret += gen_endif(memb.ifcond.ifcond) > + ret += gen_endif(memb.ifcond.cgen()) > > ret += mcgen(''' > }, > @@ -81,12 +81,12 @@ def gen_enum(name: str, > c_name=c_name(name)) > > for memb in enum_members: > - ret += gen_if(memb.ifcond.ifcond) > + ret += gen_if(memb.ifcond.cgen()) > ret += mcgen(''' > %(c_enum)s, > ''', > c_enum=c_enum_const(name, memb.name, prefix)) > - ret += gen_endif(memb.ifcond.ifcond) > + ret += gen_endif(memb.ifcond.cgen()) > > ret += mcgen(''' > } %(c_name)s; > @@ -126,7 +126,7 @@ def gen_array(name: str, element_type: QAPISchemaType) -> str: > def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str: > ret = '' > for memb in members: > - ret += gen_if(memb.ifcond.ifcond) > + ret += gen_if(memb.ifcond.cgen()) > if memb.optional: > ret += mcgen(''' > bool has_%(c_name)s; > @@ -136,7 +136,7 @@ def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str: > %(c_type)s %(c_name)s; > ''', > c_type=memb.type.c_type(), c_name=c_name(memb.name)) > - ret += gen_endif(memb.ifcond.ifcond) > + ret += gen_endif(memb.ifcond.cgen()) > return ret > > > @@ -159,7 +159,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond, > ret += mcgen(''' > > ''') > - ret += gen_if(ifcond.ifcond) > + ret += gen_if(ifcond.cgen()) > ret += mcgen(''' > struct %(c_name)s { > ''', > @@ -193,7 +193,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond, > ret += mcgen(''' > }; > ''') > - ret += gen_endif(ifcond.ifcond) > + ret += gen_endif(ifcond.cgen()) > > return ret > > @@ -220,13 +220,13 @@ def gen_variants(variants: QAPISchemaVariants) -> str: > for var in variants.variants: > if var.type.name == 'q_empty': > continue > - ret += gen_if(var.ifcond.ifcond) > + ret += gen_if(var.ifcond.cgen()) > ret += mcgen(''' > %(c_type)s %(c_name)s; > ''', > c_type=var.type.c_unboxed_type(), > c_name=c_name(var.name)) > - ret += gen_endif(var.ifcond.ifcond) > + ret += gen_endif(var.ifcond.cgen()) > > ret += mcgen(''' > } u; > diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py > index 67721b2470..56ea516399 100644 > --- a/scripts/qapi/visit.py > +++ b/scripts/qapi/visit.py > @@ -79,7 +79,7 @@ def gen_visit_object_members(name: str, > > for memb in members: > deprecated = 'deprecated' in [f.name for f in memb.features] > - ret += gen_if(memb.ifcond.ifcond) > + ret += gen_if(memb.ifcond.cgen()) > if memb.optional: > ret += mcgen(''' > if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) { > @@ -112,7 +112,7 @@ def gen_visit_object_members(name: str, > ret += mcgen(''' > } > ''') > - ret += gen_endif(memb.ifcond.ifcond) > + ret += gen_endif(memb.ifcond.cgen()) > > if variants: > tag_member = variants.tag_member > @@ -126,7 +126,7 @@ def gen_visit_object_members(name: str, > for var in variants.variants: > case_str = c_enum_const(tag_member.type.name, var.name, > tag_member.type.prefix) > - ret += gen_if(var.ifcond.ifcond) > + ret += gen_if(var.ifcond.cgen()) > if var.type.name == 'q_empty': > # valid variant and nothing to do > ret += mcgen(''' > @@ -142,7 +142,7 @@ def gen_visit_object_members(name: str, > case=case_str, > c_type=var.type.c_name(), c_name=c_name(var.name)) > > - ret += gen_endif(var.ifcond.ifcond) > + ret += gen_endif(var.ifcond.cgen()) > ret += mcgen(''' > default: > abort(); > @@ -228,7 +228,7 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str: > c_name=c_name(name)) > > for var in variants.variants: > - ret += gen_if(var.ifcond.ifcond) > + ret += gen_if(var.ifcond.cgen()) > ret += mcgen(''' > case %(case)s: > ''', > @@ -254,7 +254,7 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str: > ret += mcgen(''' > break; > ''') > - ret += gen_endif(var.ifcond.ifcond) > + ret += gen_endif(var.ifcond.cgen()) > > ret += mcgen(''' > case QTYPE_NONE:
Markus Armbruster <armbru@redhat.com> writes: > marcandre.lureau@redhat.com writes: > >> From: Marc-André Lureau <marcandre.lureau@redhat.com> >> >> Instead of building prepocessor conditions from a list of string, use >> the result generated from QAPISchemaIfCond.cgen() and hide the >> implementation details. >> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > Please mention that the patch changes generated code. See below for > details. [...] > This patch does three things: > > (1) Change gen_if(), gen_endif() to always generate a single #if, > #endif. This enables: > > (2) Factor cgen_ifcond() out of gen_if() and gen_endif() > > (3) Lift the call of cgen_ifcond() into into gen_if()'s, gen_endif()'s > callers. > > I'd split the patch. This is *not* a demand. > > The motivation for (3) is unclear. Is it so gen_if() doesn't depend on > QAPISchemaIfCond? > > Step (1) affects the generated code. When @ifcond is [COND1, COND2, ...], > gen_if()'s value changes from > > #if COND1 > #if COND2 > ... > > to > > #if (COND1) && (COND2) [...] > The common case: when it's just [COND], the value changes from > > #if COND > > to > > #if (COND) > > which is a bit ugly. > > Example: in qapi-types-block-export.c [...] > Avoiding the redundant pair of parenthesis takes another special case. > Let's do it. Looks like PATCH 07 does it. Avoiding the temporary change would be nice, but isn't required.
Markus Armbruster <armbru@redhat.com> writes: [...] >> Avoiding the redundant pair of parenthesis takes another special case. >> Let's do it. > > Looks like PATCH 07 does it. Avoiding the temporary change would be > nice, but isn't required. Make that PATCH 08.
Markus Armbruster <armbru@redhat.com> writes: > Markus Armbruster <armbru@redhat.com> writes: > > [...] > >>> Avoiding the redundant pair of parenthesis takes another special case. >>> Let's do it. >> >> Looks like PATCH 07 does it. Avoiding the temporary change would be >> nice, but isn't required. > > Make that PATCH 08. Scratch that, it's 07. Sorry for the noise.
Hi On Mon, Aug 2, 2021 at 6:46 PM Markus Armbruster <armbru@redhat.com> wrote: > marcandre.lureau@redhat.com writes: > > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > > > Instead of building prepocessor conditions from a list of string, use > > the result generated from QAPISchemaIfCond.cgen() and hide the > > implementation details. > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > Please mention that the patch changes generated code. See below for > details. > I'll add Note: this patch introduces a minor regression, generating a redundant pair of parenthesis. This is fixed in a later patch in this series ("qapi: replace if condition list with dict [..]") > > --- > > scripts/qapi/common.py | 35 ++++++++++++++++++++++------------- > > scripts/qapi/gen.py | 4 ++-- > > scripts/qapi/introspect.py | 4 ++-- > > scripts/qapi/schema.py | 5 ++++- > > scripts/qapi/types.py | 20 ++++++++++---------- > > scripts/qapi/visit.py | 12 ++++++------ > > 6 files changed, 46 insertions(+), 34 deletions(-) > > > > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > > index 6ad1eeb61d..ba9fe14e4b 100644 > > --- a/scripts/qapi/common.py > > +++ b/scripts/qapi/common.py > > @@ -12,7 +12,12 @@ > > # See the COPYING file in the top-level directory. > > > > import re > > -from typing import Match, Optional, Sequence > > +from typing import ( > > + List, > > + Match, > > + Optional, > > + Union, > > +) > > > > > > #: Magic string that gets removed along with all space to its right. > > @@ -194,22 +199,26 @@ def guardend(name: str) -> str: > > name=c_fname(name).upper()) > > > > > > -def gen_if(ifcond: Sequence[str]) -> str: > > - ret = '' > > - for ifc in ifcond: > > - ret += mcgen(''' > > +def cgen_ifcond(ifcond: Union[str, List[str]]) -> str: > > + if not ifcond: > > + return '' > > + return '(' + ') && ('.join(ifcond) + ')' > > + > > + > > +def gen_if(cond: str) -> str: > > + if not cond: > > + return '' > > + return mcgen(''' > > #if %(cond)s > > -''', cond=ifc) > > - return ret > > +''', cond=cond) > > > > > > -def gen_endif(ifcond: Sequence[str]) -> str: > > - ret = '' > > - for ifc in reversed(ifcond): > > - ret += mcgen(''' > > +def gen_endif(cond: str) -> str: > > + if not cond: > > + return '' > > + return mcgen(''' > > #endif /* %(cond)s */ > > -''', cond=ifc) > > - return ret > > +''', cond=cond) > > > > > > This patch does three things: > > (1) Change gen_if(), gen_endif() to always generate a single #if, > #endif. This enables: > > (2) Factor cgen_ifcond() out of gen_if() and gen_endif() > > (3) Lift the call of cgen_ifcond() into into gen_if()'s, gen_endif()'s > callers. > > I'd split the patch. This is *not* a demand. > > The motivation for (3) is unclear. Is it so gen_if() doesn't depend on > QAPISchemaIfCond? > > Step (1) affects the generated code. When @ifcond is [COND1, COND2, ...], > gen_if()'s value changes from > > #if COND1 > #if COND2 > ... > > to > > #if (COND1) && (COND2) > > Example: in tests/test-qapi-introspect.c > > @@ -259,11 +257,9 @@ const QLitObject test_qmp_schema_qlit = > QLIT_QDICT(((QLitDictEntry[]) { > { "arg-type", QLIT_QSTR("1"), }, > { "features", QLIT_QLIST(((QLitObject[]) { > -#if defined(TEST_IF_COND_1) > -#if defined(TEST_IF_COND_2) > +#if (defined(TEST_IF_COND_1)) && (defined(TEST_IF_COND_2)) > QLIT_QSTR("feature1"), > -#endif /* defined(TEST_IF_COND_2) */ > -#endif /* defined(TEST_IF_COND_1) */ > +#endif /* (defined(TEST_IF_COND_1)) && (defined(TEST_IF_COND_2)) */ > {} > })), }, > { "meta-type", QLIT_QSTR("command"), }, > > The common case: when it's just [COND], the value changes from > > #if COND > > to > > #if (COND) > > which is a bit ugly. > > Example: in qapi-types-block-export.c > > @@ -76,7 +76,7 @@ const QEnumLookup FuseExportAllowOther_l > .size = FUSE_EXPORT_ALLOW_OTHER__MAX > }; > > -#if defined(CONFIG_FUSE) > +#if (defined(CONFIG_FUSE)) > void qapi_free_BlockExportOptionsFuse(BlockExportOptionsFuse *obj) > { > Visitor *v; > @@ -89,7 +89,7 @@ void qapi_free_BlockExportOptionsFuse(Bl > visit_type_BlockExportOptionsFuse(v, NULL, &obj, NULL); > visit_free(v); > } > -#endif /* defined(CONFIG_FUSE) */ > +#endif /* (defined(CONFIG_FUSE)) */ > > void qapi_free_NbdServerAddOptions(NbdServerAddOptions *obj) > { > > Avoiding the redundant pair of parenthesis takes another special case. > Let's do it. > > > def must_match(pattern: str, string: str) -> Match[str]: > > diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py > > index 1c5b190276..51a597a025 100644 > > --- a/scripts/qapi/gen.py > > +++ b/scripts/qapi/gen.py > > @@ -95,9 +95,9 @@ def _wrap_ifcond(ifcond: QAPISchemaIfCond, before: > str, after: str) -> str: > > if added[0] == '\n': > > out += '\n' > > added = added[1:] > > - out += gen_if(ifcond.ifcond) > > + out += gen_if(ifcond.cgen()) > > out += added > > - out += gen_endif(ifcond.ifcond) > > + out += gen_endif(ifcond.cgen()) > > return out > > > > > > > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > > index 77a8c33ad4..474b08fd4d 100644 > > --- a/scripts/qapi/introspect.py > > +++ b/scripts/qapi/introspect.py > > @@ -124,10 +124,10 @@ def indent(level: int) -> str: > > if obj.comment: > > ret += indent(level) + f"/* {obj.comment} */\n" > > if obj.ifcond: > > - ret += gen_if(obj.ifcond.ifcond) > > + ret += gen_if(obj.ifcond.cgen()) > > ret += _tree_to_qlit(obj.value, level) > > if obj.ifcond: > > - ret += '\n' + gen_endif(obj.ifcond.ifcond) > > + ret += '\n' + gen_endif(obj.ifcond.cgen()) > > return ret > > > > ret = '' > > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > > index c35fa3bf51..70120f0dcc 100644 > > --- a/scripts/qapi/schema.py > > +++ b/scripts/qapi/schema.py > > @@ -19,7 +19,7 @@ > > import re > > from typing import Optional > > > > -from .common import POINTER_SUFFIX, c_name > > +from .common import POINTER_SUFFIX, c_name, cgen_ifcond > > from .error import QAPIError, QAPISemError, QAPISourceError > > from .expr import check_exprs > > from .parser import QAPISchemaParser > > @@ -29,6 +29,9 @@ class QAPISchemaIfCond: > > def __init__(self, ifcond=None): > > self.ifcond = ifcond or [] > > > > + def cgen(self): > > + return cgen_ifcond(self.ifcond) > > + > > As far as I can tell, this is only ever used like > > gen_if(obj.ifcond.cgen()) > > and > > gen_endif(obj.ifcond.cgen()) > > Wouldn't > > obj.ifcond.gen_if() > > and > > obj.ifcond.gen_endif() > > be neater? > Yes > Not a demand, since we can get there on top if we want to. > > > def is_present(self): > > return bool(self.ifcond) > > > > diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py > > index 3673cf0f49..db9ff95bd1 100644 > > --- a/scripts/qapi/types.py > > +++ b/scripts/qapi/types.py > > @@ -51,13 +51,13 @@ def gen_enum_lookup(name: str, > > ''', > > c_name=c_name(name)) > > for memb in members: > > - ret += gen_if(memb.ifcond.ifcond) > > + ret += gen_if(memb.ifcond.cgen()) > > index = c_enum_const(name, memb.name, prefix) > > ret += mcgen(''' > > [%(index)s] = "%(name)s", > > ''', > > index=index, name=memb.name) > > - ret += gen_endif(memb.ifcond.ifcond) > > + ret += gen_endif(memb.ifcond.cgen()) > > > > ret += mcgen(''' > > }, > > @@ -81,12 +81,12 @@ def gen_enum(name: str, > > c_name=c_name(name)) > > > > for memb in enum_members: > > - ret += gen_if(memb.ifcond.ifcond) > > + ret += gen_if(memb.ifcond.cgen()) > > ret += mcgen(''' > > %(c_enum)s, > > ''', > > c_enum=c_enum_const(name, memb.name, prefix)) > > - ret += gen_endif(memb.ifcond.ifcond) > > + ret += gen_endif(memb.ifcond.cgen()) > > > > ret += mcgen(''' > > } %(c_name)s; > > @@ -126,7 +126,7 @@ def gen_array(name: str, element_type: > QAPISchemaType) -> str: > > def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> > str: > > ret = '' > > for memb in members: > > - ret += gen_if(memb.ifcond.ifcond) > > + ret += gen_if(memb.ifcond.cgen()) > > if memb.optional: > > ret += mcgen(''' > > bool has_%(c_name)s; > > @@ -136,7 +136,7 @@ def gen_struct_members(members: > List[QAPISchemaObjectTypeMember]) -> str: > > %(c_type)s %(c_name)s; > > ''', > > c_type=memb.type.c_type(), c_name=c_name(memb.name > )) > > - ret += gen_endif(memb.ifcond.ifcond) > > + ret += gen_endif(memb.ifcond.cgen()) > > return ret > > > > > > @@ -159,7 +159,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond, > > ret += mcgen(''' > > > > ''') > > - ret += gen_if(ifcond.ifcond) > > + ret += gen_if(ifcond.cgen()) > > ret += mcgen(''' > > struct %(c_name)s { > > ''', > > @@ -193,7 +193,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond, > > ret += mcgen(''' > > }; > > ''') > > - ret += gen_endif(ifcond.ifcond) > > + ret += gen_endif(ifcond.cgen()) > > > > return ret > > > > @@ -220,13 +220,13 @@ def gen_variants(variants: QAPISchemaVariants) -> > str: > > for var in variants.variants: > > if var.type.name == 'q_empty': > > continue > > - ret += gen_if(var.ifcond.ifcond) > > + ret += gen_if(var.ifcond.cgen()) > > ret += mcgen(''' > > %(c_type)s %(c_name)s; > > ''', > > c_type=var.type.c_unboxed_type(), > > c_name=c_name(var.name)) > > - ret += gen_endif(var.ifcond.ifcond) > > + ret += gen_endif(var.ifcond.cgen()) > > > > ret += mcgen(''' > > } u; > > diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py > > index 67721b2470..56ea516399 100644 > > --- a/scripts/qapi/visit.py > > +++ b/scripts/qapi/visit.py > > @@ -79,7 +79,7 @@ def gen_visit_object_members(name: str, > > > > for memb in members: > > deprecated = 'deprecated' in [f.name for f in memb.features] > > - ret += gen_if(memb.ifcond.ifcond) > > + ret += gen_if(memb.ifcond.cgen()) > > if memb.optional: > > ret += mcgen(''' > > if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) { > > @@ -112,7 +112,7 @@ def gen_visit_object_members(name: str, > > ret += mcgen(''' > > } > > ''') > > - ret += gen_endif(memb.ifcond.ifcond) > > + ret += gen_endif(memb.ifcond.cgen()) > > > > if variants: > > tag_member = variants.tag_member > > @@ -126,7 +126,7 @@ def gen_visit_object_members(name: str, > > for var in variants.variants: > > case_str = c_enum_const(tag_member.type.name, var.name, > > tag_member.type.prefix) > > - ret += gen_if(var.ifcond.ifcond) > > + ret += gen_if(var.ifcond.cgen()) > > if var.type.name == 'q_empty': > > # valid variant and nothing to do > > ret += mcgen(''' > > @@ -142,7 +142,7 @@ def gen_visit_object_members(name: str, > > case=case_str, > > c_type=var.type.c_name(), c_name=c_name( > var.name)) > > > > - ret += gen_endif(var.ifcond.ifcond) > > + ret += gen_endif(var.ifcond.cgen()) > > ret += mcgen(''' > > default: > > abort(); > > @@ -228,7 +228,7 @@ def gen_visit_alternate(name: str, variants: > QAPISchemaVariants) -> str: > > c_name=c_name(name)) > > > > for var in variants.variants: > > - ret += gen_if(var.ifcond.ifcond) > > + ret += gen_if(var.ifcond.cgen()) > > ret += mcgen(''' > > case %(case)s: > > ''', > > @@ -254,7 +254,7 @@ def gen_visit_alternate(name: str, variants: > QAPISchemaVariants) -> str: > > ret += mcgen(''' > > break; > > ''') > > - ret += gen_endif(var.ifcond.ifcond) > > + ret += gen_endif(var.ifcond.cgen()) > > > > ret += mcgen(''' > > case QTYPE_NONE: > >
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index 6ad1eeb61d..ba9fe14e4b 100644 --- a/scripts/qapi/common.py +++ b/scripts/qapi/common.py @@ -12,7 +12,12 @@ # See the COPYING file in the top-level directory. import re -from typing import Match, Optional, Sequence +from typing import ( + List, + Match, + Optional, + Union, +) #: Magic string that gets removed along with all space to its right. @@ -194,22 +199,26 @@ def guardend(name: str) -> str: name=c_fname(name).upper()) -def gen_if(ifcond: Sequence[str]) -> str: - ret = '' - for ifc in ifcond: - ret += mcgen(''' +def cgen_ifcond(ifcond: Union[str, List[str]]) -> str: + if not ifcond: + return '' + return '(' + ') && ('.join(ifcond) + ')' + + +def gen_if(cond: str) -> str: + if not cond: + return '' + return mcgen(''' #if %(cond)s -''', cond=ifc) - return ret +''', cond=cond) -def gen_endif(ifcond: Sequence[str]) -> str: - ret = '' - for ifc in reversed(ifcond): - ret += mcgen(''' +def gen_endif(cond: str) -> str: + if not cond: + return '' + return mcgen(''' #endif /* %(cond)s */ -''', cond=ifc) - return ret +''', cond=cond) def must_match(pattern: str, string: str) -> Match[str]: diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index 1c5b190276..51a597a025 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -95,9 +95,9 @@ def _wrap_ifcond(ifcond: QAPISchemaIfCond, before: str, after: str) -> str: if added[0] == '\n': out += '\n' added = added[1:] - out += gen_if(ifcond.ifcond) + out += gen_if(ifcond.cgen()) out += added - out += gen_endif(ifcond.ifcond) + out += gen_endif(ifcond.cgen()) return out diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index 77a8c33ad4..474b08fd4d 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -124,10 +124,10 @@ def indent(level: int) -> str: if obj.comment: ret += indent(level) + f"/* {obj.comment} */\n" if obj.ifcond: - ret += gen_if(obj.ifcond.ifcond) + ret += gen_if(obj.ifcond.cgen()) ret += _tree_to_qlit(obj.value, level) if obj.ifcond: - ret += '\n' + gen_endif(obj.ifcond.ifcond) + ret += '\n' + gen_endif(obj.ifcond.cgen()) return ret ret = '' diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index c35fa3bf51..70120f0dcc 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -19,7 +19,7 @@ import re from typing import Optional -from .common import POINTER_SUFFIX, c_name +from .common import POINTER_SUFFIX, c_name, cgen_ifcond from .error import QAPIError, QAPISemError, QAPISourceError from .expr import check_exprs from .parser import QAPISchemaParser @@ -29,6 +29,9 @@ class QAPISchemaIfCond: def __init__(self, ifcond=None): self.ifcond = ifcond or [] + def cgen(self): + return cgen_ifcond(self.ifcond) + def is_present(self): return bool(self.ifcond) diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index 3673cf0f49..db9ff95bd1 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -51,13 +51,13 @@ def gen_enum_lookup(name: str, ''', c_name=c_name(name)) for memb in members: - ret += gen_if(memb.ifcond.ifcond) + ret += gen_if(memb.ifcond.cgen()) index = c_enum_const(name, memb.name, prefix) ret += mcgen(''' [%(index)s] = "%(name)s", ''', index=index, name=memb.name) - ret += gen_endif(memb.ifcond.ifcond) + ret += gen_endif(memb.ifcond.cgen()) ret += mcgen(''' }, @@ -81,12 +81,12 @@ def gen_enum(name: str, c_name=c_name(name)) for memb in enum_members: - ret += gen_if(memb.ifcond.ifcond) + ret += gen_if(memb.ifcond.cgen()) ret += mcgen(''' %(c_enum)s, ''', c_enum=c_enum_const(name, memb.name, prefix)) - ret += gen_endif(memb.ifcond.ifcond) + ret += gen_endif(memb.ifcond.cgen()) ret += mcgen(''' } %(c_name)s; @@ -126,7 +126,7 @@ def gen_array(name: str, element_type: QAPISchemaType) -> str: def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str: ret = '' for memb in members: - ret += gen_if(memb.ifcond.ifcond) + ret += gen_if(memb.ifcond.cgen()) if memb.optional: ret += mcgen(''' bool has_%(c_name)s; @@ -136,7 +136,7 @@ def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str: %(c_type)s %(c_name)s; ''', c_type=memb.type.c_type(), c_name=c_name(memb.name)) - ret += gen_endif(memb.ifcond.ifcond) + ret += gen_endif(memb.ifcond.cgen()) return ret @@ -159,7 +159,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond, ret += mcgen(''' ''') - ret += gen_if(ifcond.ifcond) + ret += gen_if(ifcond.cgen()) ret += mcgen(''' struct %(c_name)s { ''', @@ -193,7 +193,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond, ret += mcgen(''' }; ''') - ret += gen_endif(ifcond.ifcond) + ret += gen_endif(ifcond.cgen()) return ret @@ -220,13 +220,13 @@ def gen_variants(variants: QAPISchemaVariants) -> str: for var in variants.variants: if var.type.name == 'q_empty': continue - ret += gen_if(var.ifcond.ifcond) + ret += gen_if(var.ifcond.cgen()) ret += mcgen(''' %(c_type)s %(c_name)s; ''', c_type=var.type.c_unboxed_type(), c_name=c_name(var.name)) - ret += gen_endif(var.ifcond.ifcond) + ret += gen_endif(var.ifcond.cgen()) ret += mcgen(''' } u; diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index 67721b2470..56ea516399 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -79,7 +79,7 @@ def gen_visit_object_members(name: str, for memb in members: deprecated = 'deprecated' in [f.name for f in memb.features] - ret += gen_if(memb.ifcond.ifcond) + ret += gen_if(memb.ifcond.cgen()) if memb.optional: ret += mcgen(''' if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) { @@ -112,7 +112,7 @@ def gen_visit_object_members(name: str, ret += mcgen(''' } ''') - ret += gen_endif(memb.ifcond.ifcond) + ret += gen_endif(memb.ifcond.cgen()) if variants: tag_member = variants.tag_member @@ -126,7 +126,7 @@ def gen_visit_object_members(name: str, for var in variants.variants: case_str = c_enum_const(tag_member.type.name, var.name, tag_member.type.prefix) - ret += gen_if(var.ifcond.ifcond) + ret += gen_if(var.ifcond.cgen()) if var.type.name == 'q_empty': # valid variant and nothing to do ret += mcgen(''' @@ -142,7 +142,7 @@ def gen_visit_object_members(name: str, case=case_str, c_type=var.type.c_name(), c_name=c_name(var.name)) - ret += gen_endif(var.ifcond.ifcond) + ret += gen_endif(var.ifcond.cgen()) ret += mcgen(''' default: abort(); @@ -228,7 +228,7 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str: c_name=c_name(name)) for var in variants.variants: - ret += gen_if(var.ifcond.ifcond) + ret += gen_if(var.ifcond.cgen()) ret += mcgen(''' case %(case)s: ''', @@ -254,7 +254,7 @@ def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str: ret += mcgen(''' break; ''') - ret += gen_endif(var.ifcond.ifcond) + ret += gen_endif(var.ifcond.cgen()) ret += mcgen(''' case QTYPE_NONE: