Re: [Qemu-devel] [PATCH v6 01/12] qapi: Use predicate callback to determine visit filtering

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> Previously, qapi-types and qapi-visit filtered out implicit
> objects during visit_object_type() by using 'info' (works since
> implicit objects do not [yet] have associated info); meanwhile
> qapi-introspect filtered out all schema types on the first pass
> by returning a python type from visit_begin(), which was then
> used in QAPISchema.visit().  Rather than keeping these ad hoc
> approaches, add a new visitor callback visit_predicate() which
> returns False to skip a given entity, and which defaults to
> True unless overridden.  Use the new mechanism to simplify all
> three visitors that need filtering.  No change to the generated
> code.

Let's call it visit_wanted().

> Suggested-by: Markus Armbruster <address@hidden>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v6: new patch
> ---
>  scripts/qapi-introspect.py |  5 ++++-
>  scripts/qapi-types.py      | 18 ++++++++++--------
>  scripts/qapi-visit.py      | 16 +++++++++-------
>  scripts/qapi.py            | 11 +++++++----
>  4 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
> index 7d39320..f30fac3 100644
> --- a/scripts/qapi-introspect.py
> +++ b/scripts/qapi-introspect.py
> @@ -54,7 +54,6 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaVisitor):
>          self._jsons = []
>          self._used_types = []
>          self._name_map = {}
> -        return QAPISchemaType   # don't visit types for now
>
>      def visit_end(self):
>          # visit the types that are actually used
> @@ -82,6 +81,10 @@ const char %(c_name)s[] = %(c_string)s;
>          self._used_types = None
>          self._name_map = None
>
> +    def visit_predicate(self, entity):
> +        # Ignore types on first pass; visit_end() will pick up used types
> +        return not 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..c5b71b0 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -233,6 +233,9 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>          self.decl = self._btin + self.decl
>          self._btin = None
>
> +    def visit_predicate(self, entity):
> +        return not isinstance(entity, QAPISchemaObjectType) or entity.info
> +

This is the faithful translation.  But the left hand side is superfluous
because non-types laways have info, isn't it?

>      def _gen_type_cleanup(self, name):
>          self.decl += gen_type_cleanup_decl(name)
>          self.defn += gen_type_cleanup(name)
> @@ -254,14 +257,13 @@ 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)
> +        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..0f47614 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -333,6 +333,9 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
>          self.decl = self._btin + self.decl
>          self._btin = None
>
> +    def visit_predicate(self, entity):
> +        return not isinstance(entity, QAPISchemaObjectType) or entity.info
> +

Likewise.

>      def visit_enum_type(self, name, info, values, prefix):
>          self.decl += gen_visit_decl(name, scalar=True)
>          self.defn += gen_visit_enum(name)
> @@ -349,13 +352,12 @@ 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)
> +        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 26cff3f..7d359c8 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -811,6 +811,9 @@ class QAPISchemaVisitor(object):
>      def visit_end(self):
>          pass
>
> +    def visit_predicate(self, entity):
> +        return True
> +
>      def visit_builtin_type(self, name, info, json_type):
>          pass
>
> @@ -1304,10 +1307,10 @@ class QAPISchema(object):
>              ent.check(self)
>
>      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):
> -                self._entity_dict[name].visit(visitor)
> +        visitor.visit_begin(self)
> +        for (name, entity) in sorted(self._entity_dict.items()):
> +            if visitor.visit_predicate(entity):
> +                entity.visit(visitor)
>          visitor.visit_end()

Much nicer than my ad hoc hackery!