Hi On Thu, May 13, 2021 at 12:53 AM John Snow wrote: > On 4/29/21 9:40 AM, marcandre.lureau@redhat.com wrote: > > From: Marc-André Lureau > > > > Wrap the 'if' condition in a higher-level object. Not only this allows > > more type safety but also further refactoring without too much churn. > > > > Would have done it myself if I had gotten to it first. I like having a > named type for this, it matches the other properties we have. > > > The following patches will change the syntax of the schema 'if' > > conditions to be predicate expressions, and will generate code for > > different target languages (C, and Rust in another series). > > > > Signed-off-by: Marc-André Lureau > > --- > > docs/sphinx/qapidoc.py | 2 +- > > scripts/qapi/commands.py | 4 +- > > scripts/qapi/events.py | 5 ++- > > scripts/qapi/gen.py | 14 +++---- > > scripts/qapi/introspect.py | 26 ++++++------- > > scripts/qapi/schema.py | 78 +++++++++++++++++++++++++++----------- > > scripts/qapi/types.py | 33 ++++++++-------- > > scripts/qapi/visit.py | 23 +++++------ > > 8 files changed, 110 insertions(+), 75 deletions(-) > > > > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py > > index 87c67ab23f..b737949007 100644 > > --- a/docs/sphinx/qapidoc.py > > +++ b/docs/sphinx/qapidoc.py > > @@ -116,7 +116,7 @@ def _nodes_for_ifcond(self, ifcond, with_if=True): > > the conditions are in literal-text and the commas are not. > > If with_if is False, we don't return the "(If: " and ")". > > """ > > - condlist = intersperse([nodes.literal('', c) for c in ifcond], > > + condlist = intersperse([nodes.literal('', c) for c in > ifcond.ifcond], > > nodes.Text(', ')) > > if not with_if: > > return condlist > > diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py > > index 0e13d51054..3654825968 100644 > > --- a/scripts/qapi/commands.py > > +++ b/scripts/qapi/commands.py > > @@ -17,7 +17,6 @@ > > Dict, > > List, > > Optional, > > - Sequence, > > Set, > > ) > > > > @@ -31,6 +30,7 @@ > > from .schema import ( > > QAPISchema, > > QAPISchemaFeature, > > + QAPISchemaIfCond, > > QAPISchemaObjectType, > > QAPISchemaType, > > ) > > @@ -301,7 +301,7 @@ def visit_end(self) -> None: > > def visit_command(self, > > name: str, > > info: Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > features: List[QAPISchemaFeature], > > arg_type: Optional[QAPISchemaObjectType], > > ret_type: Optional[QAPISchemaType], > > diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py > > index fee8c671e7..82475e84ec 100644 > > --- a/scripts/qapi/events.py > > +++ b/scripts/qapi/events.py > > @@ -12,7 +12,7 @@ > > See the COPYING file in the top-level directory. > > """ > > > > -from typing import List, Optional, Sequence > > +from typing import List, Optional > > > > from .common import c_enum_const, c_name, mcgen > > from .gen import QAPISchemaModularCVisitor, build_params, ifcontext > > @@ -20,6 +20,7 @@ > > QAPISchema, > > QAPISchemaEnumMember, > > QAPISchemaFeature, > > + QAPISchemaIfCond, > > QAPISchemaObjectType, > > ) > > from .source import QAPISourceInfo > > @@ -227,7 +228,7 @@ def visit_end(self) -> None: > > def visit_event(self, > > name: str, > > info: Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > features: List[QAPISchemaFeature], > > arg_type: Optional[QAPISchemaObjectType], > > boxed: bool) -> None: > > diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py > > index 1fa503bdbd..1c5b190276 100644 > > --- a/scripts/qapi/gen.py > > +++ b/scripts/qapi/gen.py > > @@ -18,7 +18,6 @@ > > Dict, > > Iterator, > > Optional, > > - Sequence, > > Tuple, > > ) > > > > @@ -32,6 +31,7 @@ > > mcgen, > > ) > > from .schema import ( > > + QAPISchemaIfCond, > > QAPISchemaModule, > > QAPISchemaObjectType, > > QAPISchemaVisitor, > > @@ -85,7 +85,7 @@ def write(self, output_dir: str) -> None: > > fp.write(text) > > > > > > -def _wrap_ifcond(ifcond: Sequence[str], before: str, after: str) -> str: > > +def _wrap_ifcond(ifcond: QAPISchemaIfCond, before: str, after: str) -> > str: > > if before == after: > > return after # suppress empty #if ... #endif > > > > @@ -95,9 +95,9 @@ def _wrap_ifcond(ifcond: Sequence[str], before: str, > after: str) -> str: > > if added[0] == '\n': > > out += '\n' > > added = added[1:] > > - out += gen_if(ifcond) > > + out += gen_if(ifcond.ifcond) > > out += added > > - out += gen_endif(ifcond) > > + out += gen_endif(ifcond.ifcond) > > return out > > > > > > @@ -127,9 +127,9 @@ def build_params(arg_type: > Optional[QAPISchemaObjectType], > > class QAPIGenCCode(QAPIGen): > > def __init__(self, fname: str): > > super().__init__(fname) > > - self._start_if: Optional[Tuple[Sequence[str], str, str]] = None > > + self._start_if: Optional[Tuple[QAPISchemaIfCond, str, str]] = > None > > > > - def start_if(self, ifcond: Sequence[str]) -> None: > > + def start_if(self, ifcond: QAPISchemaIfCond) -> None: > > assert self._start_if is None > > self._start_if = (ifcond, self._body, self._preamble) > > > > @@ -187,7 +187,7 @@ def _bottom(self) -> str: > > > > > > @contextmanager > > -def ifcontext(ifcond: Sequence[str], *args: QAPIGenCCode) -> > Iterator[None]: > > +def ifcontext(ifcond: QAPISchemaIfCond, *args: QAPIGenCCode) -> > Iterator[None]: > > """ > > A with-statement context manager that wraps with `start_if()` / > `end_if()`. > > > > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > > index 9a348ca2e5..77a8c33ad4 100644 > > --- a/scripts/qapi/introspect.py > > +++ b/scripts/qapi/introspect.py > > @@ -15,11 +15,9 @@ > > Any, > > Dict, > > Generic, > > - Iterable, > > List, > > Optional, > > Sequence, > > - Tuple, > > TypeVar, > > Union, > > ) > > @@ -38,6 +36,7 @@ > > QAPISchemaEntity, > > QAPISchemaEnumMember, > > QAPISchemaFeature, > > + QAPISchemaIfCond, > > QAPISchemaObjectType, > > QAPISchemaObjectTypeMember, > > QAPISchemaType, > > @@ -91,11 +90,11 @@ class Annotated(Generic[_ValueT]): > > """ > > # TODO: Remove after Python 3.7 adds @dataclass: > > # pylint: disable=too-few-public-methods > > - def __init__(self, value: _ValueT, ifcond: Iterable[str], > > + def __init__(self, value: _ValueT, ifcond: QAPISchemaIfCond, > > comment: Optional[str] = None): > > self.value = value > > self.comment: Optional[str] = comment > > - self.ifcond: Tuple[str, ...] = tuple(ifcond) > > + self.ifcond = ifcond > > > > > > def _tree_to_qlit(obj: JSONValue, > > @@ -125,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) > > + ret += gen_if(obj.ifcond.ifcond) > > ret += _tree_to_qlit(obj.value, level) > > if obj.ifcond: > > - ret += '\n' + gen_endif(obj.ifcond) > > + ret += '\n' + gen_endif(obj.ifcond.ifcond) > > return ret > > > > ret = '' > > @@ -254,7 +253,7 @@ def _gen_features(features: > Sequence[QAPISchemaFeature] > > return [Annotated(f.name, f.ifcond) for f in features] > > > > def _gen_tree(self, name: str, mtype: str, obj: Dict[str, object], > > - ifcond: Sequence[str] = (), > > + ifcond: QAPISchemaIfCond = QAPISchemaIfCond(), > > features: Sequence[QAPISchemaFeature] = ()) -> None: > > """ > > Build and append a SchemaInfo object to self._trees. > > @@ -305,7 +304,7 @@ def visit_builtin_type(self, name: str, info: > Optional[QAPISourceInfo], > > self._gen_tree(name, 'builtin', {'json-type': json_type}) > > > > def visit_enum_type(self, name: str, info: > Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > features: List[QAPISchemaFeature], > > members: List[QAPISchemaEnumMember], > > prefix: Optional[str]) -> None: > > @@ -316,14 +315,14 @@ def visit_enum_type(self, name: str, info: > Optional[QAPISourceInfo], > > ) > > > > def visit_array_type(self, name: str, info: > Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > element_type: QAPISchemaType) -> None: > > element = self._use_type(element_type) > > self._gen_tree('[' + element + ']', 'array', {'element-type': > element}, > > ifcond) > > > > def visit_object_type_flat(self, name: str, info: > Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > features: List[QAPISchemaFeature], > > members: > List[QAPISchemaObjectTypeMember], > > variants: Optional[QAPISchemaVariants]) > -> None: > > @@ -336,7 +335,7 @@ def visit_object_type_flat(self, name: str, info: > Optional[QAPISourceInfo], > > self._gen_tree(name, 'object', obj, ifcond, features) > > > > def visit_alternate_type(self, name: str, info: > Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > features: List[QAPISchemaFeature], > > variants: QAPISchemaVariants) -> None: > > self._gen_tree( > > @@ -348,7 +347,7 @@ def visit_alternate_type(self, name: str, info: > Optional[QAPISourceInfo], > > ) > > > > def visit_command(self, name: str, info: Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > features: List[QAPISchemaFeature], > > arg_type: Optional[QAPISchemaObjectType], > > ret_type: Optional[QAPISchemaType], gen: bool, > > @@ -367,7 +366,8 @@ def visit_command(self, name: str, info: > Optional[QAPISourceInfo], > > self._gen_tree(name, 'command', obj, ifcond, features) > > > > def visit_event(self, name: str, info: Optional[QAPISourceInfo], > > - ifcond: Sequence[str], features: > List[QAPISchemaFeature], > > + ifcond: QAPISchemaIfCond, > > + features: List[QAPISchemaFeature], > > arg_type: Optional[QAPISchemaObjectType], > > boxed: bool) -> None: > > assert self._schema is not None > > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > > index 3a4172fb74..7d6f390fa6 100644 > > --- a/scripts/qapi/schema.py > > +++ b/scripts/qapi/schema.py > > @@ -25,6 +25,22 @@ > > from .parser import QAPISchemaParser > > > > > > +class QAPISchemaIfCond: > > + def __init__(self, ifcond=None): > > + self.ifcond = ifcond or [] > > + > > + def __bool__(self): > > + return bool(self.ifcond) > > + > > + def __repr__(self): > > + return repr(self.ifcond) > > + > > I suggest using: > > f"QAPISchemaIfCond({self.ifcond!r})" > > unless future patches make that weirder. It's nice when repr() returns > something you can use to make a new, equivalent object. > > eval(repr(x)) == x > > I implemented it this way for compatibility with test-qapi output. But it is simpler to actually no implement __repr__ and just change the test. > + def __eq__(self, other): > > + if not isinstance(other, QAPISchemaIfCond): > > + return NotImplemented > > + return self.ifcond == other.ifcond > > + > > + > > Missing annotations, but ... yeah, schema.py isn't typed yet so I will > handle it myself in pt6. No biggie. > There used to be annotations all over in earlier versions, but I rebased and had to drop most of them. > > class QAPISchemaEntity: > > meta: Optional[str] = None > > > > @@ -42,7 +58,7 @@ def __init__(self, name: str, info, doc, ifcond=None, > features=None): > > # such place). > > self.info = info > > self.doc = doc > > - self._ifcond = ifcond or [] > > + self._ifcond = ifcond or QAPISchemaIfCond() > > self.features = features or [] > > self._checked = False > > > > @@ -77,7 +93,7 @@ def set_module(self, schema): > > > > @property > > def ifcond(self): > > - assert self._checked > > + assert self._checked and isinstance(self._ifcond, > QAPISchemaIfCond) > > return self._ifcond > > > > def is_implicit(self): > > @@ -601,7 +617,7 @@ def check(self, schema, seen): > > else: # simple union > > assert isinstance(self.tag_member.type, QAPISchemaEnumType) > > assert not self.tag_member.optional > > - assert self.tag_member.ifcond == [] > > + assert not self.tag_member.ifcond > > if self._tag_name: # flat union > > # branches that are not explicitly covered get an empty > type > > cases = {v.name for v in self.variants} > > @@ -646,7 +662,7 @@ def __init__(self, name, info, ifcond=None): > > assert isinstance(name, str) > > self.name = name > > self.info = info > > - self.ifcond = ifcond or [] > > + self.ifcond = ifcond or QAPISchemaIfCond() > > self.defined_in = None > > > > def set_defined_in(self, name): > > @@ -958,14 +974,20 @@ def _def_predefineds(self): > > self._def_entity(QAPISchemaEnumType('QType', None, None, None, > None, > > qtype_values, 'QTYPE')) > > > > + def _get_if(self, f) -> QAPISchemaIfCond: > > + ifcond = f.get('if') > > + if isinstance(ifcond, QAPISchemaIfCond): > > would "is not None" suffice here? > > > + return ifcond > > + return QAPISchemaIfCond(ifcond) > > Under which circumstances do we expect 'ifcond' here to be something > other than None but *not* a QAPISchemaIfCond? > > This helper seems a little suspect in the sense that it appears to be > taking liberties with the dynamic runtime object system. > Yes, this is due to the adhoc enum being created in _def_union_type(). There, the ifcond of the variant is used, which is already a QAPISchemaIfCond. I'll lower it again to the list form. > > + > def _make_features(self, features, info): > > if features is None: > > return [] > > - return [QAPISchemaFeature(f['name'], info, f.get('if')) > > + return [QAPISchemaFeature(f['name'], info, self._get_if(f)) > > for f in features] > > > > def _make_enum_members(self, values, info): > > - return [QAPISchemaEnumMember(v['name'], info, v.get('if')) > > + return [QAPISchemaEnumMember(v['name'], info, self._get_if(v)) > > for v in values] > > > > def _make_implicit_enum_type(self, name, info, ifcond, values): > > @@ -1001,7 +1023,7 @@ def _make_implicit_object_type(self, name, info, > ifcond, role, members): > > # TODO kill simple unions or implement the disjunction > > > > # pylint: disable=protected-access > > - assert (ifcond or []) == typ._ifcond > > + assert ifcond == typ._ifcond > > else: > > self._def_entity(QAPISchemaObjectType( > > name, info, None, ifcond, None, None, members, None)) > > @@ -1011,7 +1033,7 @@ def _def_enum_type(self, expr, info, doc): > > name = expr['enum'] > > data = expr['data'] > > prefix = expr.get('prefix') > > - ifcond = expr.get('if') > > + ifcond = QAPISchemaIfCond(expr.get('if')) > > features = self._make_features(expr.get('features'), info) > > self._def_entity(QAPISchemaEnumType( > > name, info, doc, ifcond, features, > > @@ -1029,7 +1051,8 @@ def _make_member(self, name, typ, ifcond, > features, info): > > > self._make_features(features, info)) > > > > def _make_members(self, data, info): > > - return [self._make_member(key, value['type'], value.get('if'), > > + return [self._make_member(key, value['type'], > > + QAPISchemaIfCond(value.get('if')), > > value.get('features'), info) > > for (key, value) in data.items()] > > > > @@ -1037,7 +1060,7 @@ def _def_struct_type(self, expr, info, doc): > > name = expr['struct'] > > base = expr.get('base') > > data = expr['data'] > > - ifcond = expr.get('if') > > + ifcond = QAPISchemaIfCond(expr.get('if')) > > features = self._make_features(expr.get('features'), info) > > self._def_entity(QAPISchemaObjectType( > > name, info, doc, ifcond, features, base, > > @@ -1060,7 +1083,7 @@ def _def_union_type(self, expr, info, doc): > > name = expr['union'] > > data = expr['data'] > > base = expr.get('base') > > - ifcond = expr.get('if') > > + ifcond = QAPISchemaIfCond(expr.get('if')) > > features = self._make_features(expr.get('features'), info) > > tag_name = expr.get('discriminator') > > tag_member = None > > @@ -1069,14 +1092,20 @@ def _def_union_type(self, expr, info, doc): > > name, info, ifcond, > > 'base', self._make_members(base, info)) > > if tag_name: > > - variants = [self._make_variant(key, value['type'], > > - value.get('if'), info) > > - for (key, value) in data.items()] > > + variants = [ > > + self._make_variant(key, value['type'], > > + QAPISchemaIfCond(value.get('if')), > > + info) > > + for (key, value) in data.items() > > + ] > > members = [] > > else: > > - variants = [self._make_simple_variant(key, value['type'], > > - value.get('if'), info) > > - for (key, value) in data.items()] > > + variants = [ > > + self._make_simple_variant(key, value['type'], > > + > QAPISchemaIfCond(value.get('if')), > > + info) > > + for (key, value) in data.items() > > + ] > > enum = [{'name': v.name, 'if': v.ifcond} for v in > variants] > > typ = self._make_implicit_enum_type(name, info, ifcond, > enum) > > tag_member = QAPISchemaObjectTypeMember('type', info, typ, > False) > > @@ -1090,11 +1119,14 @@ def _def_union_type(self, expr, info, doc): > > def _def_alternate_type(self, expr, info, doc): > > name = expr['alternate'] > > data = expr['data'] > > - ifcond = expr.get('if') > > + ifcond = QAPISchemaIfCond(expr.get('if')) > > features = self._make_features(expr.get('features'), info) > > - variants = [self._make_variant(key, value['type'], > value.get('if'), > > - info) > > - for (key, value) in data.items()] > > + variants = [ > > + self._make_variant(key, value['type'], > > + QAPISchemaIfCond(value.get('if')), > > + info) > > + for (key, value) in data.items() > > + ] > > tag_member = QAPISchemaObjectTypeMember('type', info, 'QType', > False) > > self._def_entity( > > QAPISchemaAlternateType(name, info, doc, ifcond, features, > > @@ -1111,7 +1143,7 @@ def _def_command(self, expr, info, doc): > > allow_oob = expr.get('allow-oob', False) > > allow_preconfig = expr.get('allow-preconfig', False) > > coroutine = expr.get('coroutine', False) > > - ifcond = expr.get('if') > > + ifcond = QAPISchemaIfCond(expr.get('if')) > > features = self._make_features(expr.get('features'), info) > > if isinstance(data, OrderedDict): > > data = self._make_implicit_object_type( > > @@ -1130,7 +1162,7 @@ def _def_event(self, expr, info, doc): > > name = expr['event'] > > data = expr.get('data') > > boxed = expr.get('boxed', False) > > - ifcond = expr.get('if') > > + ifcond = QAPISchemaIfCond(expr.get('if')) > > features = self._make_features(expr.get('features'), info) > > if isinstance(data, OrderedDict): > > data = self._make_implicit_object_type( > > diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py > > index 20d572a23a..3673cf0f49 100644 > > --- a/scripts/qapi/types.py > > +++ b/scripts/qapi/types.py > > @@ -13,7 +13,7 @@ > > # See the COPYING file in the top-level directory. > > """ > > > > -from typing import List, Optional, Sequence > > +from typing import List, Optional > > > > from .common import ( > > c_enum_const, > > @@ -27,6 +27,7 @@ > > QAPISchema, > > QAPISchemaEnumMember, > > QAPISchemaFeature, > > + QAPISchemaIfCond, > > QAPISchemaObjectType, > > QAPISchemaObjectTypeMember, > > QAPISchemaType, > > @@ -50,13 +51,13 @@ def gen_enum_lookup(name: str, > > ''', > > c_name=c_name(name)) > > for memb in members: > > - ret += gen_if(memb.ifcond) > > + ret += gen_if(memb.ifcond.ifcond) > > index = c_enum_const(name, memb.name, prefix) > > ret += mcgen(''' > > [%(index)s] = "%(name)s", > > ''', > > index=index, name=memb.name) > > - ret += gen_endif(memb.ifcond) > > + ret += gen_endif(memb.ifcond.ifcond) > > > > ret += mcgen(''' > > }, > > @@ -80,12 +81,12 @@ def gen_enum(name: str, > > c_name=c_name(name)) > > > > for memb in enum_members: > > - ret += gen_if(memb.ifcond) > > + ret += gen_if(memb.ifcond.ifcond) > > ret += mcgen(''' > > %(c_enum)s, > > ''', > > c_enum=c_enum_const(name, memb.name, prefix)) > > - ret += gen_endif(memb.ifcond) > > + ret += gen_endif(memb.ifcond.ifcond) > > > > ret += mcgen(''' > > } %(c_name)s; > > @@ -125,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) > > + ret += gen_if(memb.ifcond.ifcond) > > if memb.optional: > > ret += mcgen(''' > > bool has_%(c_name)s; > > @@ -135,11 +136,11 @@ 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) > > + ret += gen_endif(memb.ifcond.ifcond) > > return ret > > > > > > -def gen_object(name: str, ifcond: Sequence[str], > > +def gen_object(name: str, ifcond: QAPISchemaIfCond, > > base: Optional[QAPISchemaObjectType], > > members: List[QAPISchemaObjectTypeMember], > > variants: Optional[QAPISchemaVariants]) -> str: > > @@ -158,7 +159,7 @@ def gen_object(name: str, ifcond: Sequence[str], > > ret += mcgen(''' > > > > ''') > > - ret += gen_if(ifcond) > > + ret += gen_if(ifcond.ifcond) > > ret += mcgen(''' > > struct %(c_name)s { > > ''', > > @@ -192,7 +193,7 @@ def gen_object(name: str, ifcond: Sequence[str], > > ret += mcgen(''' > > }; > > ''') > > - ret += gen_endif(ifcond) > > + ret += gen_endif(ifcond.ifcond) > > > > return ret > > > > @@ -219,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) > > + ret += gen_if(var.ifcond.ifcond) > > 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) > > + ret += gen_endif(var.ifcond.ifcond) > > > > ret += mcgen(''' > > } u; > > @@ -307,7 +308,7 @@ def _gen_type_cleanup(self, name: str) -> None: > > def visit_enum_type(self, > > name: str, > > info: Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > features: List[QAPISchemaFeature], > > members: List[QAPISchemaEnumMember], > > prefix: Optional[str]) -> None: > > @@ -318,7 +319,7 @@ def visit_enum_type(self, > > def visit_array_type(self, > > name: str, > > info: Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > element_type: QAPISchemaType) -> None: > > with ifcontext(ifcond, self._genh, self._genc): > > self._genh.preamble_add(gen_fwd_object_or_array(name)) > > @@ -328,7 +329,7 @@ def visit_array_type(self, > > def visit_object_type(self, > > name: str, > > info: Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > features: List[QAPISchemaFeature], > > base: Optional[QAPISchemaObjectType], > > members: List[QAPISchemaObjectTypeMember], > > @@ -351,7 +352,7 @@ def visit_object_type(self, > > def visit_alternate_type(self, > > name: str, > > info: Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > features: List[QAPISchemaFeature], > > variants: QAPISchemaVariants) -> None: > > with ifcontext(ifcond, self._genh): > > diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py > > index 9e96f3c566..67721b2470 100644 > > --- a/scripts/qapi/visit.py > > +++ b/scripts/qapi/visit.py > > @@ -13,7 +13,7 @@ > > See the COPYING file in the top-level directory. > > """ > > > > -from typing import List, Optional, Sequence > > +from typing import List, Optional > > > > from .common import ( > > c_enum_const, > > @@ -29,6 +29,7 @@ > > QAPISchemaEnumMember, > > QAPISchemaEnumType, > > QAPISchemaFeature, > > + QAPISchemaIfCond, > > QAPISchemaObjectType, > > QAPISchemaObjectTypeMember, > > QAPISchemaType, > > @@ -78,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) > > + ret += gen_if(memb.ifcond.ifcond) > > if memb.optional: > > ret += mcgen(''' > > if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) { > > @@ -111,7 +112,7 @@ def gen_visit_object_members(name: str, > > ret += mcgen(''' > > } > > ''') > > - ret += gen_endif(memb.ifcond) > > + ret += gen_endif(memb.ifcond.ifcond) > > > > if variants: > > tag_member = variants.tag_member > > @@ -125,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) > > + ret += gen_if(var.ifcond.ifcond) > > if var.type.name == 'q_empty': > > # valid variant and nothing to do > > ret += mcgen(''' > > @@ -141,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) > > + ret += gen_endif(var.ifcond.ifcond) > > ret += mcgen(''' > > default: > > abort(); > > @@ -227,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) > > + ret += gen_if(var.ifcond.ifcond) > > ret += mcgen(''' > > case %(case)s: > > ''', > > @@ -253,7 +254,7 @@ def gen_visit_alternate(name: str, variants: > QAPISchemaVariants) -> str: > > ret += mcgen(''' > > break; > > ''') > > - ret += gen_endif(var.ifcond) > > + ret += gen_endif(var.ifcond.ifcond) > > > > ret += mcgen(''' > > case QTYPE_NONE: > > @@ -352,7 +353,7 @@ def _begin_user_module(self, name: str) -> None: > > def visit_enum_type(self, > > name: str, > > info: Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > features: List[QAPISchemaFeature], > > members: List[QAPISchemaEnumMember], > > prefix: Optional[str]) -> None: > > @@ -363,7 +364,7 @@ def visit_enum_type(self, > > def visit_array_type(self, > > name: str, > > info: Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > element_type: QAPISchemaType) -> None: > > with ifcontext(ifcond, self._genh, self._genc): > > self._genh.add(gen_visit_decl(name)) > > @@ -372,7 +373,7 @@ def visit_array_type(self, > > def visit_object_type(self, > > name: str, > > info: Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > features: List[QAPISchemaFeature], > > base: Optional[QAPISchemaObjectType], > > members: List[QAPISchemaObjectTypeMember], > > @@ -394,7 +395,7 @@ def visit_object_type(self, > > def visit_alternate_type(self, > > name: str, > > info: Optional[QAPISourceInfo], > > - ifcond: Sequence[str], > > + ifcond: QAPISchemaIfCond, > > features: List[QAPISchemaFeature], > > variants: QAPISchemaVariants) -> None: > > with ifcontext(ifcond, self._genh, self._genc): > > > > looks good. > > Tested-by: John Snow > >