On Thu, Mar 14, 2024, 5:12 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@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 <arm...@redhat.com> > > Signed-off-by: John Snow <js...@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. > I don't believe you are, I think it was just ... something I didn't notice when rebasing, since I merged this patch by hand. I recall having to insert the super call manually, and I guess my preferences are nondeterministic. > > + > > + 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 > > [...] > >