Re: [PATCH v4 05/23] qapi: create QAPISchemaDefinition

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> Include entities don't have names, but we generally expect "entities" to
> have names. Reclassify all entities with names as *definitions*, leaving
> the nameless include entities as QAPISchemaEntity instances.
>
> This is primarily to help simplify typing around expectations of what
> callers expect for properties of an "entity".
>
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/schema.py | 144 +++++++++++++++++++++++------------------
>  1 file changed, 82 insertions(+), 62 deletions(-)
>
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 117f0f78f0c..da273c1649d 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py

[...]

> @@ -107,6 +90,46 @@ def _set_module(self, schema, info):
>      def set_module(self, schema):
>          self._set_module(schema, self.info)
>  
> +    def visit(self, visitor):
> +        # pylint: disable=unused-argument
> +        assert self._checked
> +
> +
> +class QAPISchemaDefinition(QAPISchemaEntity):
> +    meta: Optional[str] = None
> +
> +    def __init__(self, name: str, info, doc, ifcond=None, features=None):
> +        assert isinstance(name, str)
> +        super().__init__(info)
> +        for f in features or []:
> +            assert isinstance(f, QAPISchemaFeature)
> +            f.set_defined_in(name)
> +        self.name = name
> +        self.doc = doc
> +        self._ifcond = ifcond or QAPISchemaIfCond()
> +        self.features = features or []
> +
> +    def __repr__(self):
> +        return "<%s:%s at 0x%x>" % (type(self).__name__, self.name,
> +                                    id(self))
> +
> +    def c_name(self):
> +        return c_name(self.name)
> +
> +    def check(self, schema):
> +        assert not self._checked
> +        seen = {}
> +        for f in self.features:
> +            f.check_clash(self.info, seen)
> +        super().check(schema)

v3 called super().check() first.  Why did you move it?  I'm asking just
to make sure I'm not missing something subtle.

> +
> +    def connect_doc(self, doc=None):
> +        super().connect_doc(doc)
> +        doc = doc or self.doc
> +        if doc:
> +            for f in self.features:
> +                doc.connect_feature(f)
> +
>      @property
>      def ifcond(self):
>          assert self._checked

[...]