From 1d067e3953e76af28ba20c995b176fcbcb7a10aa Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 15 Mar 2024 20:32:41 +0100 Subject: [PATCH 1/7] qapi: New QAPISchemaBranches, QAPISchemaAlternatives QAPISchemaVariants represents either a union type's branches, or an alternate type's alternatives. Much of its code is conditional on which one it actually is. Create QAPISchemaBranches for branches, and QAPISchemaAlternatives for alternatives, both subtypes of QAPISchemaVariants. Replace QAPISchemaVariants by one of them where possible. Keep it only where we actually deal with either of them. QAPISchemaVariants.__init__() takes @tag_name and @tag_member, where exactly one must be None: @tag_name for alternatives, @tag_member for branches. Let QAPISchemaBranches.__init__() take just @tag_name, and QAPISchemaAlternatives.__init__() take just @tag_member. A later patch will move the conditional code to the subtypes. Signed-off-by: Markus Armbruster --- scripts/qapi/introspect.py | 7 ++++--- scripts/qapi/schema.py | 32 ++++++++++++++++++++++++-------- scripts/qapi/types.py | 6 ++++-- scripts/qapi/visit.py | 11 ++++++----- 4 files changed, 38 insertions(+), 18 deletions(-) diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index 4679b1bc2c..b866517942 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -26,6 +26,8 @@ from .common import c_name, mcgen from .gen import QAPISchemaMonolithicCVisitor from .schema import ( QAPISchema, + QAPISchemaAlternatives, + QAPISchemaBranches, QAPISchemaArrayType, QAPISchemaBuiltinType, QAPISchemaEntity, @@ -36,7 +38,6 @@ from .schema import ( QAPISchemaObjectTypeMember, QAPISchemaType, QAPISchemaVariant, - QAPISchemaVariants, ) from .source import QAPISourceInfo @@ -335,7 +336,7 @@ const QLitObject %(c_name)s = %(c_string)s; ifcond: QAPISchemaIfCond, features: List[QAPISchemaFeature], members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaVariants]) -> None: + variants: Optional[QAPISchemaBranches]) -> None: obj: SchemaInfoObject = { 'members': [self._gen_object_member(m) for m in members] } @@ -347,7 +348,7 @@ const QLitObject %(c_name)s = %(c_string)s; def visit_alternate_type(self, name: str, info: Optional[QAPISourceInfo], ifcond: QAPISchemaIfCond, features: List[QAPISchemaFeature], - variants: QAPISchemaVariants) -> None: + variants: QAPISchemaAlternatives) -> None: self._gen_tree( name, 'alternate', {'members': [Annotated({'type': self._use_type(m.type)}, diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 5924947fc3..5cdedfc2c8 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -215,7 +215,7 @@ class QAPISchemaVisitor: features: List[QAPISchemaFeature], base: Optional[QAPISchemaObjectType], members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaVariants], + variants: Optional[QAPISchemaBranches], ) -> None: pass @@ -226,7 +226,7 @@ class QAPISchemaVisitor: ifcond: QAPISchemaIfCond, features: List[QAPISchemaFeature], members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaVariants], + variants: Optional[QAPISchemaBranches], ) -> None: pass @@ -236,7 +236,7 @@ class QAPISchemaVisitor: info: Optional[QAPISourceInfo], ifcond: QAPISchemaIfCond, features: List[QAPISchemaFeature], - variants: QAPISchemaVariants, + variants: QAPISchemaAlternatives, ) -> None: pass @@ -524,7 +524,7 @@ class QAPISchemaObjectType(QAPISchemaType): features: Optional[List[QAPISchemaFeature]], base: Optional[str], local_members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaVariants], + variants: Optional[QAPISchemaBranches], ): # struct has local_members, optional base, and no variants # union has base, variants, and no local_members @@ -651,7 +651,7 @@ class QAPISchemaAlternateType(QAPISchemaType): doc: Optional[QAPIDoc], ifcond: Optional[QAPISchemaIfCond], features: List[QAPISchemaFeature], - variants: QAPISchemaVariants, + variants: QAPISchemaAlternatives, ): super().__init__(name, info, doc, ifcond, features) assert variants.tag_member @@ -833,6 +833,22 @@ class QAPISchemaVariants: v.type.check_clash(info, dict(seen)) +class QAPISchemaBranches(QAPISchemaVariants): + def __init__(self, + info: QAPISourceInfo, + variants: List[QAPISchemaVariant], + tag_name: str): + super().__init__(tag_name, info, None, variants) + + +class QAPISchemaAlternatives(QAPISchemaVariants): + def __init__(self, + info: QAPISourceInfo, + variants: List[QAPISchemaVariant], + tag_member: QAPISchemaObjectTypeMember): + super().__init__(None, info, tag_member, variants) + + class QAPISchemaMember: """ Represents object members, enum members and features """ role = 'member' @@ -1388,8 +1404,8 @@ class QAPISchema: self._def_definition( QAPISchemaObjectType(name, info, expr.doc, ifcond, features, base, members, - QAPISchemaVariants( - tag_name, info, None, variants))) + QAPISchemaBranches( + info, variants, tag_name))) def _def_alternate_type(self, expr: QAPIExpression) -> None: name = expr['alternate'] @@ -1407,7 +1423,7 @@ class QAPISchema: self._def_definition( QAPISchemaAlternateType( name, info, expr.doc, ifcond, features, - QAPISchemaVariants(None, info, tag_member, variants))) + QAPISchemaAlternatives(info, variants, tag_member))) def _def_command(self, expr: QAPIExpression) -> None: name = expr['command'] diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index c39d054d2c..23cdf3e83e 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -23,6 +23,8 @@ from .gen import ( ) from .schema import ( QAPISchema, + QAPISchemaAlternatives, + QAPISchemaBranches, QAPISchemaEnumMember, QAPISchemaFeature, QAPISchemaIfCond, @@ -348,7 +350,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor): features: List[QAPISchemaFeature], base: Optional[QAPISchemaObjectType], members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaVariants]) -> None: + variants: Optional[QAPISchemaBranches]) -> None: # Nothing to do for the special empty builtin if name == 'q_empty': return @@ -369,7 +371,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor): info: Optional[QAPISourceInfo], ifcond: QAPISchemaIfCond, features: List[QAPISchemaFeature], - variants: QAPISchemaVariants) -> None: + variants: QAPISchemaAlternatives) -> None: with ifcontext(ifcond, self._genh): self._genh.preamble_add(gen_fwd_object_or_array(name)) self._genh.add(gen_object(name, ifcond, None, diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index a21b7b1468..990685498f 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -28,6 +28,8 @@ from .gen import ( ) from .schema import ( QAPISchema, + QAPISchemaAlternatives, + QAPISchemaBranches, QAPISchemaEnumMember, QAPISchemaEnumType, QAPISchemaFeature, @@ -35,7 +37,6 @@ from .schema import ( QAPISchemaObjectType, QAPISchemaObjectTypeMember, QAPISchemaType, - QAPISchemaVariants, ) from .source import QAPISourceInfo @@ -63,7 +64,7 @@ bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp); def gen_visit_object_members(name: str, base: Optional[QAPISchemaObjectType], members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaVariants]) -> str: + variants: Optional[QAPISchemaBranches]) -> str: ret = mcgen(''' bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp) @@ -222,7 +223,7 @@ bool visit_type_%(c_name)s(Visitor *v, const char *name, c_name=c_name(name)) -def gen_visit_alternate(name: str, variants: QAPISchemaVariants) -> str: +def gen_visit_alternate(name: str, variants: QAPISchemaAlternatives) -> str: ret = mcgen(''' bool visit_type_%(c_name)s(Visitor *v, const char *name, @@ -393,7 +394,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor): features: List[QAPISchemaFeature], base: Optional[QAPISchemaObjectType], members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaVariants]) -> None: + variants: Optional[QAPISchemaBranches]) -> None: # Nothing to do for the special empty builtin if name == 'q_empty': return @@ -413,7 +414,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor): info: Optional[QAPISourceInfo], ifcond: QAPISchemaIfCond, features: List[QAPISchemaFeature], - variants: QAPISchemaVariants) -> None: + variants: QAPISchemaAlternatives) -> None: with ifcontext(ifcond, self._genh, self._genc): self._genh.add(gen_visit_decl(name)) self._genc.add(gen_visit_alternate(name, variants)) From d1da8af897340ed3773c09add93c3b9f494f2c2b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 15 Mar 2024 16:28:22 +0100 Subject: [PATCH 2/7] qapi: Rename visitor parameter @variants to @branches The previous commit narrowed the type of .visit_object_type() parameter @variants from QAPISchemaVariants to QAPISchemaBranches. Rename it to @branches. Same for .visit_object_type_flat(). A few of these pass @branches to helper functions: QAPISchemaGenRSTVisitor.visit_object_type() to ._nodes_for_members() and ._nodes_for_variant_when(), and QAPISchemaGenVisitVisitor.visit_object_type() to gen_visit_object_members(). Rename the helpers' @variants parameters to @branches as well. Signed-off-by: Markus Armbruster --- docs/sphinx/qapidoc.py | 18 +++++++++--------- scripts/qapi/introspect.py | 8 ++++---- scripts/qapi/schema.py | 4 ++-- scripts/qapi/types.py | 4 ++-- scripts/qapi/visit.py | 12 ++++++------ tests/qapi-schema/test-qapi.py | 4 ++-- 6 files changed, 25 insertions(+), 25 deletions(-) diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index 8d428c64b0..71362ba929 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -145,22 +145,22 @@ class QAPISchemaGenRSTVisitor(QAPISchemaVisitor): term.extend(self._nodes_for_ifcond(member.ifcond)) return term - def _nodes_for_variant_when(self, variants, variant): + def _nodes_for_variant_when(self, branches, variant): """Return list of Text, literal nodes for variant 'when' clause Return a list of doctree nodes which give text like 'when tagname is variant (If: ...)' suitable for use in - the 'variants' part of a definition list. + the 'branches' part of a definition list. """ term = [nodes.Text(' when '), - nodes.literal('', variants.tag_member.name), + nodes.literal('', branches.tag_member.name), nodes.Text(' is '), nodes.literal('', '"%s"' % variant.name)] if variant.ifcond.is_present(): term.extend(self._nodes_for_ifcond(variant.ifcond)) return term - def _nodes_for_members(self, doc, what, base=None, variants=None): + def _nodes_for_members(self, doc, what, base=None, branches=None): """Return list of doctree nodes for the table of members""" dlnode = nodes.definition_list() for section in doc.args.values(): @@ -178,14 +178,14 @@ class QAPISchemaGenRSTVisitor(QAPISchemaVisitor): nodes.literal('', base.doc_type())], None) - if variants: - for v in variants.variants: + if branches: + for v in branches.variants: if v.type.name == 'q_empty': continue assert not v.type.is_implicit() term = [nodes.Text('The members of '), nodes.literal('', v.type.doc_type())] - term.extend(self._nodes_for_variant_when(variants, v)) + term.extend(self._nodes_for_variant_when(branches, v)) dlnode += self._make_dlitem(term, None) if not dlnode.children: @@ -308,12 +308,12 @@ class QAPISchemaGenRSTVisitor(QAPISchemaVisitor): + self._nodes_for_if_section(ifcond)) def visit_object_type(self, name, info, ifcond, features, - base, members, variants): + base, members, branches): doc = self._cur_doc if base and base.is_implicit(): base = None self._add_doc('Object', - self._nodes_for_members(doc, 'Members', base, variants) + self._nodes_for_members(doc, 'Members', base, branches) + self._nodes_for_features(doc) + self._nodes_for_sections(doc) + self._nodes_for_if_section(ifcond)) diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index b866517942..7852591490 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -336,13 +336,13 @@ const QLitObject %(c_name)s = %(c_string)s; ifcond: QAPISchemaIfCond, features: List[QAPISchemaFeature], members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaBranches]) -> None: + branches: Optional[QAPISchemaBranches]) -> None: obj: SchemaInfoObject = { 'members': [self._gen_object_member(m) for m in members] } - if variants: - obj['tag'] = variants.tag_member.name - obj['variants'] = [self._gen_variant(v) for v in variants.variants] + if branches: + obj['tag'] = branches.tag_member.name + obj['variants'] = [self._gen_variant(v) for v in branches.variants] self._gen_tree(name, 'object', obj, ifcond, features) def visit_alternate_type(self, name: str, info: Optional[QAPISourceInfo], diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 5cdedfc2c8..65c82dd4f1 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -215,7 +215,7 @@ class QAPISchemaVisitor: features: List[QAPISchemaFeature], base: Optional[QAPISchemaObjectType], members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaBranches], + branches: Optional[QAPISchemaBranches], ) -> None: pass @@ -226,7 +226,7 @@ class QAPISchemaVisitor: ifcond: QAPISchemaIfCond, features: List[QAPISchemaFeature], members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaBranches], + branches: Optional[QAPISchemaBranches], ) -> None: pass diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index 23cdf3e83e..0abb78f3a8 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -350,13 +350,13 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor): features: List[QAPISchemaFeature], base: Optional[QAPISchemaObjectType], members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaBranches]) -> None: + branches: Optional[QAPISchemaBranches]) -> None: # Nothing to do for the special empty builtin if name == 'q_empty': return with ifcontext(ifcond, self._genh): self._genh.preamble_add(gen_fwd_object_or_array(name)) - self._genh.add(gen_object(name, ifcond, base, members, variants)) + self._genh.add(gen_object(name, ifcond, base, members, branches)) with ifcontext(ifcond, self._genh, self._genc): if base and not base.is_implicit(): self._genh.add(gen_upcast(name, base)) diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index 990685498f..fbae5f3e4e 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -64,7 +64,7 @@ bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp); def gen_visit_object_members(name: str, base: Optional[QAPISchemaObjectType], members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaBranches]) -> str: + branches: Optional[QAPISchemaBranches]) -> str: ret = mcgen(''' bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp) @@ -132,8 +132,8 @@ bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp) ''') ret += memb.ifcond.gen_endif() - if variants: - tag_member = variants.tag_member + if branches: + tag_member = branches.tag_member assert isinstance(tag_member.type, QAPISchemaEnumType) ret += mcgen(''' @@ -141,7 +141,7 @@ bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp) ''', c_name=c_name(tag_member.name)) - for var in variants.variants: + for var in branches.variants: case_str = c_enum_const(tag_member.type.name, var.name, tag_member.type.prefix) ret += var.ifcond.gen_if() @@ -394,14 +394,14 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor): features: List[QAPISchemaFeature], base: Optional[QAPISchemaObjectType], members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaBranches]) -> None: + branches: Optional[QAPISchemaBranches]) -> None: # Nothing to do for the special empty builtin if name == 'q_empty': return with ifcontext(ifcond, self._genh, self._genc): self._genh.add(gen_visit_members_decl(name)) self._genc.add(gen_visit_object_members(name, base, - members, variants)) + members, branches)) # TODO Worth changing the visitor signature, so we could # directly use rather than repeat type.is_implicit()? if not name.startswith('q_'): diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index 40095431ae..7c67ad8d9b 100755 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -48,7 +48,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): self._print_if(ifcond) def visit_object_type(self, name, info, ifcond, features, - base, members, variants): + base, members, branches): print('object %s' % name) if base: print(' base %s' % base.name) @@ -57,7 +57,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): % (m.name, m.type.name, m.optional)) self._print_if(m.ifcond, 8) self._print_features(m.features, indent=8) - self._print_variants(variants) + self._print_variants(branches) self._print_if(ifcond) self._print_features(features) From 41d0ad1d045a1af51200ff5f2a1309e4aada0a96 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 16 Mar 2024 07:43:36 +0100 Subject: [PATCH 3/7] qapi: Rename visitor parameter @variants to @alternatives A previous commit narrowed the type of .visit_alternate_type() parameter @variants from QAPISchemaVariants to QAPISchemaAlternatives. Rename it to @alternatives. One of them passes @alternatives to helper function gen_visit_alternate(). Rename its @variants parameter to @alternatives as well. Signed-off-by: Markus Armbruster --- docs/sphinx/qapidoc.py | 3 ++- scripts/qapi/introspect.py | 4 ++-- scripts/qapi/schema.py | 2 +- scripts/qapi/types.py | 4 ++-- scripts/qapi/visit.py | 9 +++++---- tests/qapi-schema/test-qapi.py | 5 +++-- 6 files changed, 15 insertions(+), 12 deletions(-) diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py index 71362ba929..f270b494f0 100644 --- a/docs/sphinx/qapidoc.py +++ b/docs/sphinx/qapidoc.py @@ -318,7 +318,8 @@ class QAPISchemaGenRSTVisitor(QAPISchemaVisitor): + self._nodes_for_sections(doc) + self._nodes_for_if_section(ifcond)) - def visit_alternate_type(self, name, info, ifcond, features, variants): + def visit_alternate_type(self, name, info, ifcond, features, + alternatives): doc = self._cur_doc self._add_doc('Alternate', self._nodes_for_members(doc, 'Members') diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index 7852591490..86c075a6ad 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -348,12 +348,12 @@ const QLitObject %(c_name)s = %(c_string)s; def visit_alternate_type(self, name: str, info: Optional[QAPISourceInfo], ifcond: QAPISchemaIfCond, features: List[QAPISchemaFeature], - variants: QAPISchemaAlternatives) -> None: + alternatives: QAPISchemaAlternatives) -> None: self._gen_tree( name, 'alternate', {'members': [Annotated({'type': self._use_type(m.type)}, m.ifcond) - for m in variants.variants]}, + for m in alternatives.variants]}, ifcond, features ) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 65c82dd4f1..2b67992aee 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -236,7 +236,7 @@ class QAPISchemaVisitor: info: Optional[QAPISourceInfo], ifcond: QAPISchemaIfCond, features: List[QAPISchemaFeature], - variants: QAPISchemaAlternatives, + alternatives: QAPISchemaAlternatives, ) -> None: pass diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index 0abb78f3a8..69f5f6ffd0 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -371,11 +371,11 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor): info: Optional[QAPISourceInfo], ifcond: QAPISchemaIfCond, features: List[QAPISchemaFeature], - variants: QAPISchemaAlternatives) -> None: + alternatives: QAPISchemaAlternatives) -> None: with ifcontext(ifcond, self._genh): self._genh.preamble_add(gen_fwd_object_or_array(name)) self._genh.add(gen_object(name, ifcond, None, - [variants.tag_member], variants)) + [alternatives.tag_member], alternatives)) with ifcontext(ifcond, self._genh, self._genc): self._gen_type_cleanup(name) diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index fbae5f3e4e..e766acaac9 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -223,7 +223,8 @@ bool visit_type_%(c_name)s(Visitor *v, const char *name, c_name=c_name(name)) -def gen_visit_alternate(name: str, variants: QAPISchemaAlternatives) -> str: +def gen_visit_alternate(name: str, + alternatives: QAPISchemaAlternatives) -> str: ret = mcgen(''' bool visit_type_%(c_name)s(Visitor *v, const char *name, @@ -245,7 +246,7 @@ bool visit_type_%(c_name)s(Visitor *v, const char *name, ''', c_name=c_name(name)) - for var in variants.variants: + for var in alternatives.variants: ret += var.ifcond.gen_if() ret += mcgen(''' case %(case)s: @@ -414,10 +415,10 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor): info: Optional[QAPISourceInfo], ifcond: QAPISchemaIfCond, features: List[QAPISchemaFeature], - variants: QAPISchemaAlternatives) -> None: + alternatives: QAPISchemaAlternatives) -> None: with ifcontext(ifcond, self._genh, self._genc): self._genh.add(gen_visit_decl(name)) - self._genc.add(gen_visit_alternate(name, variants)) + self._genc.add(gen_visit_alternate(name, alternatives)) def gen_visit(schema: QAPISchema, diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py index 7c67ad8d9b..7e3f9f4aa1 100755 --- a/tests/qapi-schema/test-qapi.py +++ b/tests/qapi-schema/test-qapi.py @@ -61,9 +61,10 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor): self._print_if(ifcond) self._print_features(features) - def visit_alternate_type(self, name, info, ifcond, features, variants): + def visit_alternate_type(self, name, info, ifcond, features, + alternatives): print('alternate %s' % name) - self._print_variants(variants) + self._print_variants(alternatives) self._print_if(ifcond) self._print_features(features) From 3ff2a5a35c387a4deb86101474c7e181b36e82f2 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 15 Mar 2024 16:33:23 +0100 Subject: [PATCH 4/7] qapi: Rename QAPISchemaObjectType.variants to .branches A previous commit narrowed the type of QAPISchemaObjectType.variants from QAPISchemaVariants to QAPISchemaBranches. Rename it to .branches. Same for .__init__() parameter @variants. Signed-off-by: Markus Armbruster --- scripts/qapi/commands.py | 2 +- scripts/qapi/events.py | 2 +- scripts/qapi/gen.py | 2 +- scripts/qapi/schema.py | 36 ++++++++++++++++++------------------ scripts/qapi/types.py | 2 +- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py index d1fdf4182c..79951a841f 100644 --- a/scripts/qapi/commands.py +++ b/scripts/qapi/commands.py @@ -64,7 +64,7 @@ def gen_call(name: str, assert arg_type argstr = '&arg, ' elif arg_type: - assert not arg_type.variants + assert not arg_type.branches for memb in arg_type.members: assert not memb.ifcond.is_present() if memb.need_has(): diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py index 3cf01e96b6..d1f639981a 100644 --- a/scripts/qapi/events.py +++ b/scripts/qapi/events.py @@ -51,7 +51,7 @@ def gen_param_var(typ: QAPISchemaObjectType) -> str: Initialize it with the function arguments defined in `gen_event_send`. """ - assert not typ.variants + assert not typ.branches ret = mcgen(''' %(c_name)s param = { ''', diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index 5412716617..6a8abe0041 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -118,7 +118,7 @@ def build_params(arg_type: Optional[QAPISchemaObjectType], ret += '%s arg' % arg_type.c_param_type() sep = ', ' elif arg_type: - assert not arg_type.variants + assert not arg_type.branches for memb in arg_type.members: assert not memb.ifcond.is_present() ret += sep diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 2b67992aee..c9ff794d0c 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -524,20 +524,20 @@ class QAPISchemaObjectType(QAPISchemaType): features: Optional[List[QAPISchemaFeature]], base: Optional[str], local_members: List[QAPISchemaObjectTypeMember], - variants: Optional[QAPISchemaBranches], + branches: Optional[QAPISchemaBranches], ): - # struct has local_members, optional base, and no variants - # union has base, variants, and no local_members + # struct has local_members, optional base, and no branches + # union has base, branches, and no local_members super().__init__(name, info, doc, ifcond, features) - self.meta = 'union' if variants else 'struct' + self.meta = 'union' if branches else 'struct' for m in local_members: m.set_defined_in(name) - if variants is not None: - variants.set_defined_in(name) + if branches is not None: + branches.set_defined_in(name) self._base_name = base self.base = None self.local_members = local_members - self.variants = variants + self.branches = branches self.members: List[QAPISchemaObjectTypeMember] self._check_complete = False @@ -561,7 +561,7 @@ class QAPISchemaObjectType(QAPISchemaType): self.base = schema.resolve_type(self._base_name, self.info, "'base'") if (not isinstance(self.base, QAPISchemaObjectType) - or self.base.variants): + or self.base.branches): raise QAPISemError( self.info, "'base' requires a struct type, %s isn't" @@ -577,9 +577,9 @@ class QAPISchemaObjectType(QAPISchemaType): # Cast down to the subtype. members = cast(List[QAPISchemaObjectTypeMember], list(seen.values())) - if self.variants: - self.variants.check(schema, seen) - self.variants.check_clash(self.info, seen) + if self.branches: + self.branches.check(schema, seen) + self.branches.check_clash(self.info, seen) self.members = members self._check_complete = True # mark completed @@ -595,8 +595,8 @@ class QAPISchemaObjectType(QAPISchemaType): assert self._checked for m in self.members: m.check_clash(info, seen) - if self.variants: - self.variants.check_clash(info, seen) + if self.branches: + self.branches.check_clash(info, seen) def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None: super().connect_doc(doc) @@ -612,7 +612,7 @@ class QAPISchemaObjectType(QAPISchemaType): return self.name.startswith('q_') def is_empty(self) -> bool: - return not self.members and not self.variants + return not self.members and not self.branches def has_conditional_members(self) -> bool: return any(m.ifcond.is_present() for m in self.members) @@ -635,10 +635,10 @@ class QAPISchemaObjectType(QAPISchemaType): super().visit(visitor) visitor.visit_object_type( self.name, self.info, self.ifcond, self.features, - self.base, self.local_members, self.variants) + self.base, self.local_members, self.branches) visitor.visit_object_type_flat( self.name, self.info, self.ifcond, self.features, - self.members, self.variants) + self.members, self.branches) class QAPISchemaAlternateType(QAPISchemaType): @@ -1035,7 +1035,7 @@ class QAPISchemaCommand(QAPISchemaDefinition): "command's 'data' cannot take %s" % arg_type.describe()) self.arg_type = arg_type - if self.arg_type.variants and not self.boxed: + if self.arg_type.branches and not self.boxed: raise QAPISemError( self.info, "command's 'data' can take %s only with 'boxed': true" @@ -1103,7 +1103,7 @@ class QAPISchemaEvent(QAPISchemaDefinition): "event's 'data' cannot take %s" % typ.describe()) self.arg_type = typ - if self.arg_type.variants and not self.boxed: + if self.arg_type.branches and not self.boxed: raise QAPISemError( self.info, "event's 'data' can take %s only with 'boxed': true" diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index 69f5f6ffd0..0dd0b00ada 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -171,7 +171,7 @@ def gen_object(name: str, ifcond: QAPISchemaIfCond, if not isinstance(obj, QAPISchemaObjectType): continue ret += gen_object(obj.name, obj.ifcond, obj.base, - obj.local_members, obj.variants) + obj.local_members, obj.branches) ret += mcgen(''' From e0a28f39b4602de56d3c0f66a386ededd25ea109 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 15 Mar 2024 16:36:02 +0100 Subject: [PATCH 5/7] qapi: Rename QAPISchemaAlternateType.variants to .alternatives A previous commit narrowed the type of QAPISchemaAlternateType.variants from QAPISchemaVariants to QAPISchemaAlternatives. Rename it to .alternatives. Same for .__init__() parameter @variants. Signed-off-by: Markus Armbruster --- scripts/qapi/schema.py | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index c9ff794d0c..9bdbfd52b2 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -651,25 +651,25 @@ class QAPISchemaAlternateType(QAPISchemaType): doc: Optional[QAPIDoc], ifcond: Optional[QAPISchemaIfCond], features: List[QAPISchemaFeature], - variants: QAPISchemaAlternatives, + alternatives: QAPISchemaAlternatives, ): super().__init__(name, info, doc, ifcond, features) - assert variants.tag_member - variants.set_defined_in(name) - variants.tag_member.set_defined_in(self.name) - self.variants = variants + assert alternatives.tag_member + alternatives.set_defined_in(name) + alternatives.tag_member.set_defined_in(self.name) + self.alternatives = alternatives def check(self, schema: QAPISchema) -> None: super().check(schema) - self.variants.tag_member.check(schema) - # Not calling self.variants.check_clash(), because there's nothing - # to clash with - self.variants.check(schema, {}) + self.alternatives.tag_member.check(schema) + # Not calling self.alternatives.check_clash(), because there's + # nothing to clash with + self.alternatives.check(schema, {}) # Alternate branch names have no relation to the tag enum values; # so we have to check for potential name collisions ourselves. seen: Dict[str, QAPISchemaMember] = {} types_seen: Dict[str, str] = {} - for v in self.variants.variants: + for v in self.alternatives.variants: v.check_clash(self.info, seen) qtype = v.type.alternate_qtype() if not qtype: @@ -700,7 +700,7 @@ class QAPISchemaAlternateType(QAPISchemaType): def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None: super().connect_doc(doc) doc = doc or self.doc - for v in self.variants.variants: + for v in self.alternatives.variants: v.connect_doc(doc) def c_type(self) -> str: @@ -712,7 +712,8 @@ class QAPISchemaAlternateType(QAPISchemaType): def visit(self, visitor: QAPISchemaVisitor) -> None: super().visit(visitor) visitor.visit_alternate_type( - self.name, self.info, self.ifcond, self.features, self.variants) + self.name, self.info, self.ifcond, self.features, + self.alternatives) class QAPISchemaVariants: From 8152bc7de6d4377d5104078115aa61986b436f44 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 15 Mar 2024 20:57:56 +0100 Subject: [PATCH 6/7] qapi: Move conditional code from QAPISchemaVariants to its subtypes QAPISchemaVariants.check()'s code is almost entirely conditional on union vs. alternate type. Move the conditional code to QAPISchemaBranches.check() and QAPISchemaAlternatives.check(), where the conditions are always satisfied. Attribute QAPISchemaVariants.tag_name is now only used by QAPISchemaBranches. Move it there. Refactor the three types' .__init__() to make them a bit simpler. Signed-off-by: Markus Armbruster --- scripts/qapi/schema.py | 138 ++++++++++++++++++++--------------------- 1 file changed, 69 insertions(+), 69 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index 9bdbfd52b2..c5b824f1fd 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -719,20 +719,11 @@ class QAPISchemaAlternateType(QAPISchemaType): class QAPISchemaVariants: def __init__( self, - tag_name: Optional[str], info: QAPISourceInfo, - tag_member: Optional[QAPISchemaObjectTypeMember], variants: List[QAPISchemaVariant], ): - # Unions pass tag_name but not tag_member. - # Alternates pass tag_member but not tag_name. - # After check(), tag_member is always set. - assert bool(tag_member) != bool(tag_name) - assert (isinstance(tag_name, str) or - isinstance(tag_member, QAPISchemaObjectTypeMember)) - self._tag_name = tag_name self.info = info - self._tag_member = tag_member + self._tag_member: Optional[QAPISchemaObjectTypeMember] = None self.variants = variants @property @@ -749,58 +740,66 @@ class QAPISchemaVariants: v.set_defined_in(name) def check( - self, schema: QAPISchema, seen: Dict[str, QAPISchemaMember] + self, schema: QAPISchema, seen: Dict[str, QAPISchemaMember] ) -> None: - if self._tag_name: # union - # We need to narrow the member type: - tmp = seen.get(c_name(self._tag_name)) - assert tmp is None or isinstance(tmp, QAPISchemaObjectTypeMember) - self._tag_member = tmp + for v in self.variants: + v.check(schema) - base = "'base'" - # Pointing to the base type when not implicit would be - # nice, but we don't know it here - if not self._tag_member or self._tag_name != self._tag_member.name: - raise QAPISemError( - self.info, - "discriminator '%s' is not a member of %s" - % (self._tag_name, base)) - # Here we do: - assert self.tag_member.defined_in - base_type = schema.lookup_type(self.tag_member.defined_in) - assert base_type - if not base_type.is_implicit(): - base = "base type '%s'" % self.tag_member.defined_in - if not isinstance(self.tag_member.type, QAPISchemaEnumType): - raise QAPISemError( - self.info, - "discriminator member '%s' of %s must be of enum type" - % (self._tag_name, base)) - if self.tag_member.optional: - raise QAPISemError( - self.info, - "discriminator member '%s' of %s must not be optional" - % (self._tag_name, base)) - if self.tag_member.ifcond.is_present(): - raise QAPISemError( - self.info, - "discriminator member '%s' of %s must not be conditional" - % (self._tag_name, base)) - else: # alternate - assert self._tag_member - assert isinstance(self.tag_member.type, QAPISchemaEnumType) - assert not self.tag_member.optional - assert not self.tag_member.ifcond.is_present() - if self._tag_name: # union - # branches that are not explicitly covered get an empty type - assert self.tag_member.defined_in - cases = {v.name for v in self.variants} - for m in self.tag_member.type.members: - if m.name not in cases: - v = QAPISchemaVariant(m.name, self.info, - 'q_empty', m.ifcond) - v.set_defined_in(self.tag_member.defined_in) - self.variants.append(v) + +class QAPISchemaBranches(QAPISchemaVariants): + def __init__(self, + info: QAPISourceInfo, + variants: List[QAPISchemaVariant], + tag_name: str): + super().__init__(info, variants) + self._tag_name = tag_name + + def check( + self, schema: QAPISchema, seen: Dict[str, QAPISchemaMember] + ) -> None: + # We need to narrow the member type: + tmp = seen.get(c_name(self._tag_name)) + assert tmp is None or isinstance(tmp, QAPISchemaObjectTypeMember) + self._tag_member = tmp + + base = "'base'" + # Pointing to the base type when not implicit would be + # nice, but we don't know it here + if not self._tag_member or self._tag_name != self._tag_member.name: + raise QAPISemError( + self.info, + "discriminator '%s' is not a member of %s" + % (self._tag_name, base)) + # Here we do: + assert self.tag_member.defined_in + base_type = schema.lookup_type(self.tag_member.defined_in) + assert base_type + if not base_type.is_implicit(): + base = "base type '%s'" % self.tag_member.defined_in + if not isinstance(self.tag_member.type, QAPISchemaEnumType): + raise QAPISemError( + self.info, + "discriminator member '%s' of %s must be of enum type" + % (self._tag_name, base)) + if self.tag_member.optional: + raise QAPISemError( + self.info, + "discriminator member '%s' of %s must not be optional" + % (self._tag_name, base)) + if self.tag_member.ifcond.is_present(): + raise QAPISemError( + self.info, + "discriminator member '%s' of %s must not be conditional" + % (self._tag_name, base)) + # branches that are not explicitly covered get an empty type + assert self.tag_member.defined_in + cases = {v.name for v in self.variants} + for m in self.tag_member.type.members: + if m.name not in cases: + v = QAPISchemaVariant(m.name, self.info, + 'q_empty', m.ifcond) + v.set_defined_in(self.tag_member.defined_in) + self.variants.append(v) if not self.variants: raise QAPISemError(self.info, "union has no branches") for v in self.variants: @@ -834,20 +833,21 @@ class QAPISchemaVariants: v.type.check_clash(info, dict(seen)) -class QAPISchemaBranches(QAPISchemaVariants): - def __init__(self, - info: QAPISourceInfo, - variants: List[QAPISchemaVariant], - tag_name: str): - super().__init__(tag_name, info, None, variants) - - class QAPISchemaAlternatives(QAPISchemaVariants): def __init__(self, info: QAPISourceInfo, variants: List[QAPISchemaVariant], tag_member: QAPISchemaObjectTypeMember): - super().__init__(None, info, tag_member, variants) + super().__init__(info, variants) + self._tag_member = tag_member + + def check( + self, schema: QAPISchema, seen: Dict[str, QAPISchemaMember] + ) -> None: + super().check(schema, seen) + assert isinstance(self.tag_member.type, QAPISchemaEnumType) + assert not self.tag_member.optional + assert not self.tag_member.ifcond.is_present() class QAPISchemaMember: From 285a8f209af7b4992aa91e8bea03a303fb6406ab Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sat, 16 Mar 2024 08:46:12 +0100 Subject: [PATCH 7/7] qapi: Simplify QAPISchemaVariants @tag_member For union types, the tag member is known only after .check(). We used to code this in a simple way: QAPISchemaVariants attribute .tag_member was None for union types until .check(). Since this complicated typing, recent commit "qapi/schema: fix typing for QAPISchemaVariants.tag_member" hid it behind a property. The previous commit lets us treat .tag_member just like the other attributes that become known only in .check(): declare, but don't initialize it in .__init__(). Signed-off-by: Markus Armbruster --- scripts/qapi/schema.py | 44 +++++++++++++++++------------------------- 1 file changed, 18 insertions(+), 26 deletions(-) diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index c5b824f1fd..721c470d2b 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -723,18 +723,9 @@ class QAPISchemaVariants: variants: List[QAPISchemaVariant], ): self.info = info - self._tag_member: Optional[QAPISchemaObjectTypeMember] = None + self.tag_member: QAPISchemaObjectTypeMember self.variants = variants - @property - def tag_member(self) -> QAPISchemaObjectTypeMember: - if self._tag_member is None: - raise RuntimeError( - "QAPISchemaVariants has no tag_member property until " - "after check() has been run." - ) - return self._tag_member - def set_defined_in(self, name: str) -> None: for v in self.variants: v.set_defined_in(name) @@ -758,47 +749,48 @@ class QAPISchemaBranches(QAPISchemaVariants): self, schema: QAPISchema, seen: Dict[str, QAPISchemaMember] ) -> None: # We need to narrow the member type: - tmp = seen.get(c_name(self._tag_name)) - assert tmp is None or isinstance(tmp, QAPISchemaObjectTypeMember) - self._tag_member = tmp + tag_member = seen.get(c_name(self._tag_name)) + assert (tag_member is None + or isinstance(tag_member, QAPISchemaObjectTypeMember)) base = "'base'" # Pointing to the base type when not implicit would be # nice, but we don't know it here - if not self._tag_member or self._tag_name != self._tag_member.name: + if not tag_member or self._tag_name != tag_member.name: raise QAPISemError( self.info, "discriminator '%s' is not a member of %s" % (self._tag_name, base)) + self.tag_member = tag_member # Here we do: - assert self.tag_member.defined_in - base_type = schema.lookup_type(self.tag_member.defined_in) + assert tag_member.defined_in + base_type = schema.lookup_type(tag_member.defined_in) assert base_type if not base_type.is_implicit(): - base = "base type '%s'" % self.tag_member.defined_in - if not isinstance(self.tag_member.type, QAPISchemaEnumType): + base = "base type '%s'" % tag_member.defined_in + if not isinstance(tag_member.type, QAPISchemaEnumType): raise QAPISemError( self.info, "discriminator member '%s' of %s must be of enum type" % (self._tag_name, base)) - if self.tag_member.optional: + if tag_member.optional: raise QAPISemError( self.info, "discriminator member '%s' of %s must not be optional" % (self._tag_name, base)) - if self.tag_member.ifcond.is_present(): + if tag_member.ifcond.is_present(): raise QAPISemError( self.info, "discriminator member '%s' of %s must not be conditional" % (self._tag_name, base)) # branches that are not explicitly covered get an empty type - assert self.tag_member.defined_in + assert tag_member.defined_in cases = {v.name for v in self.variants} - for m in self.tag_member.type.members: + for m in tag_member.type.members: if m.name not in cases: v = QAPISchemaVariant(m.name, self.info, 'q_empty', m.ifcond) - v.set_defined_in(self.tag_member.defined_in) + v.set_defined_in(tag_member.defined_in) self.variants.append(v) if not self.variants: raise QAPISemError(self.info, "union has no branches") @@ -807,11 +799,11 @@ class QAPISchemaBranches(QAPISchemaVariants): # Union names must match enum values; alternate names are # checked separately. Use 'seen' to tell the two apart. if seen: - if v.name not in self.tag_member.type.member_names(): + if v.name not in tag_member.type.member_names(): raise QAPISemError( self.info, "branch '%s' is not a value of %s" - % (v.name, self.tag_member.type.describe())) + % (v.name, tag_member.type.describe())) if not isinstance(v.type, QAPISchemaObjectType): raise QAPISemError( self.info, @@ -839,7 +831,7 @@ class QAPISchemaAlternatives(QAPISchemaVariants): variants: List[QAPISchemaVariant], tag_member: QAPISchemaObjectTypeMember): super().__init__(info, variants) - self._tag_member = tag_member + self.tag_member = tag_member def check( self, schema: QAPISchema, seen: Dict[str, QAPISchemaMember]