Eric Blake <ebl...@redhat.com> writes: > Previously, qapi-types and qapi-visit filtered an object visit > to bypass implicit types by inspecting whether 'info' was passed > in (since implicit objects do not [yet] have associated info); > meanwhile qapi-introspect filtered by returning a python type > from visit_begin() in order to exclude all type entities on the > first pass. Rather than keeping these ad hoc methods, rework > the contract of visit_begin() to state that it may return a > predicate function that returns True to ignore a given entity, > and use that mechanism to simplify all three visitors. > > Suggested-by: Markus Armbruster <arm...@redhat.com> > Signed-off-by: Eric Blake <ebl...@redhat.com> > --- > >>> Another option is generalizing QAPISchema's filter. How? > >> We can always add an indirection: instead of parameterizing a fixed >> predicate (ignore and isinstance(entity, ignore)) with a type ignore, we >> could pass a predicate, i.e. a function mapping entity to bool. > > Like this? Generates the same code before and after the > patch. I'm open to suggestions on any way to make it more > idiomatic python, althouth it at least passed pep8. In > particular, I'm wondering if the predicate should have its > sense reversed (pass False to skip, True to visit). > > I'll probably drop the 'assert info' lines in the two > visit_object_type() calls (I put it there to make sure the > predicate was working).
Yes, please. > scripts/qapi-introspect.py | 5 ++++- > scripts/qapi-types.py | 20 ++++++++++++-------- > scripts/qapi-visit.py | 18 +++++++++++------- > scripts/qapi.py | 4 +++- > 4 files changed, 30 insertions(+), 17 deletions(-) > > diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py > index 7d39320..37b4306 100644 > --- a/scripts/qapi-introspect.py > +++ b/scripts/qapi-introspect.py > @@ -54,7 +54,7 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor): > self._jsons = [] > self._used_types = [] > self._name_map = {} > - return QAPISchemaType # don't visit types for now > + return self._visit_filter # don't visit types for now > > def visit_end(self): > # visit the types that are actually used > @@ -82,6 +82,9 @@ const char %(c_name)s[] = %(c_string)s; > self._used_types = None > self._name_map = None > > + def _visit_filter(self, entity): > + return isinstance(entity, QAPISchemaType) > + > def _name(self, name): > if self._unmask: > return name > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py > index d405f8d..fbe74e1 100644 > --- a/scripts/qapi-types.py > +++ b/scripts/qapi-types.py > @@ -213,12 +213,16 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor): > self._fwdefn = None > self._btin = None > > + def _visit_filter(self, entity): > + return isinstance(entity, QAPISchemaObjectType) and not entity.info > + > def visit_begin(self, schema): > self.decl = '' > self.defn = '' > self._fwdecl = '' > self._fwdefn = '' > self._btin = guardstart('QAPI_TYPES_BUILTIN') > + return self._visit_filter # ignore builtin types > > def visit_end(self): > self.decl = self._fwdecl + self.decl > @@ -254,14 +258,14 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor): > self._gen_type_cleanup(name) > > def visit_object_type(self, name, info, base, members, variants): > - if info: > - self._fwdecl += gen_fwd_object_or_array(name) > - if variants: > - assert not members # not implemented > - self.decl += gen_union(name, base, variants) > - else: > - self.decl += gen_struct(name, base, members) > - self._gen_type_cleanup(name) > + assert info > + self._fwdecl += gen_fwd_object_or_array(name) > + if variants: > + assert not members # not implemented > + self.decl += gen_union(name, base, variants) > + else: > + self.decl += gen_struct(name, base, members) > + self._gen_type_cleanup(name) > > def visit_alternate_type(self, name, info, variants): > self._fwdecl += gen_fwd_object_or_array(name) > diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py > index 4f97781..be1bba7 100644 > --- a/scripts/qapi-visit.py > +++ b/scripts/qapi-visit.py > @@ -319,10 +319,14 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor): > self.defn = None > self._btin = None > > + def _visit_filter(self, entity): > + return isinstance(entity, QAPISchemaObjectType) and not entity.info > + > def visit_begin(self, schema): > self.decl = '' > self.defn = '' > self._btin = guardstart('QAPI_VISIT_BUILTIN') > + return self._visit_filter # ignore builtin types > > def visit_end(self): > # To avoid header dependency hell, we always generate > @@ -349,13 +353,13 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor): > self.defn += defn > > def visit_object_type(self, name, info, base, members, variants): > - if info: > - self.decl += gen_visit_decl(name) > - if variants: > - assert not members # not implemented > - self.defn += gen_visit_union(name, base, variants) > - else: > - self.defn += gen_visit_struct(name, base, members) > + assert info > + self.decl += gen_visit_decl(name) > + if variants: > + assert not members # not implemented > + self.defn += gen_visit_union(name, base, variants) > + else: > + self.defn += gen_visit_struct(name, base, members) > > def visit_alternate_type(self, name, info, variants): > self.decl += gen_visit_decl(name) > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 7761b4b..0fadc05 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -805,6 +805,8 @@ class QAPISchemaEntity(object): > > > class QAPISchemaVisitor(object): > + # To ignore certain entities, return a predicate function such > + # that predicate(entity) returns True to ignore that entity > def visit_begin(self, schema): > pass > > @@ -1306,7 +1308,7 @@ class QAPISchema(object): > def visit(self, visitor): > ignore = visitor.visit_begin(self) > for name in sorted(self._entity_dict.keys()): > - if not ignore or not isinstance(self._entity_dict[name], ignore): > + if not ignore or not ignore(self._entity_dict[name]): > self._entity_dict[name].visit(visitor) > visitor.visit_end() I think this turned out rather nicely. Can we go one step further? Unconditionally call visitor.visit_filter() here, define the pass-everything filter in QAPISchemaVisitor, override it as needed. Name it visit_filter_out() to make the sense of the return value obvious?