John Snow <js...@redhat.com> writes:
> 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.
I'd like to move it back, for consistency with its subtypes' .check(),
which all call super().check() early. Okay?
>> > +
>> > + 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
>>
>> [...]
>>
>>
