On Mon, Jan 15, 2024 at 8:53 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> John Snow <jsnow@redhat.com> writes:
>
> > declare, but don't initialize the type of "type" to be QAPISchemaType -
>
> Declare
>
> > and allow the value to be initialized during check(). This creates a
> > form of delayed initialization for QAPISchemaType objects where the
> > static typing only represents the fully-realized object, which occurs
> > after check() has been called.
> >
> > This avoids the need for several "assert type is not None" statements
> > littered throughout the code by asserting it "will always be set."
>
> Uh, I find "will always be set" confusing.
>
> I think you mean "cannot be None".
Yuh
>
> But "be set" can also be read as "will have a value".
...yuh
>
> It actually doesn't exist until .check(), and once it exists, it cannot
> be None.
>
> > Note that the static typing information for this object will be
> > incorrect prior to check() being called. If this field is accessed
> > before it is initialized in check(), you'll be treated to an
> > AttributeError exception.
>
> We could now enter a philosophical debate on whether the static typing
> is actually incorrect. Clearly v: T asserts that the value of @v is
> always a T, and the type checker proves this. Does it also assert that
> @v exists? The type checker doesn't care, and that's a feature.
*clutches TAPL and cries*
>
> Maybe start with the problem, and then develop the solution:
>
> A QAPISchemaObjectTypeMember's type gets resolved only during .check().
> We have QAPISchemaObjectTypeMember.__init__() initialize self.type =
> None, and .check() assign the actual type. Using .type before .check()
> is wrong, and hopefully crashes due to the value being None. Works.
>
> However, it makes for awkward typing. With .type:
> Optional[QAPISchemaType], mypy is of course unable to see that it's None
> before .check(), and a QAPISchemaType after. To help it over the hump,
> we'd have to assert self.type is not None before all the (valid) uses.
> The assertion catches invalid uses, but only at run time; mypy can't
> flag them.
>
> Instead, declare .type in .__init__() as QAPISchemaType *without*
> initializing it. Using .type before .check() now certainly crashes,
> which is an improvement. Mypy still can't flag invalid uses, but that's
> okay.
>
OK.
--js
> > Fixes stuff like this:
> >
> > qapi/schema.py:657: error: "None" has no attribute "alternate_qtype" [attr-defined]
> > qapi/schema.py:662: error: "None" has no attribute "describe" [attr-defined]
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> > scripts/qapi/schema.py | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> > index e39ed972a80..48a51dcd188 100644
> > --- a/scripts/qapi/schema.py
> > +++ b/scripts/qapi/schema.py
> > @@ -794,7 +794,7 @@ def __init__(self, name, info, typ, optional, ifcond=None, features=None):
> > assert isinstance(f, QAPISchemaFeature)
> > f.set_defined_in(name)
> > self._type_name = typ
> > - self.type = None
> > + self.type: QAPISchemaType # set during check()
> > self.optional = optional
> > self.features = features or []
>