Re: [PATCH 08/19] qapi/schema: add static typing and assertions to lookup_type()

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Wed, Nov 22, 2023 at 7:09 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > On Tue, Nov 21, 2023, 9:21 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >
>> >> John Snow <jsnow@redhat.com> writes:
>> >>
>> >> > This function is a bit hard to type as-is; mypy needs some assertions to
>> >> > assist with the type narrowing.
>> >> >
>> >> > Signed-off-by: John Snow <jsnow@redhat.com>
>> >> > ---
>> >> >  scripts/qapi/schema.py | 8 ++++++--
>> >> >  1 file changed, 6 insertions(+), 2 deletions(-)
>> >> >
>> >> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
>> >> > index a1094283828..3308f334872 100644
>> >> > --- a/scripts/qapi/schema.py
>> >> > +++ b/scripts/qapi/schema.py
>> >> > @@ -968,8 +968,12 @@ def lookup_entity(self, name, typ=None):
>> >> >              return None
>> >> >          return ent
>> >> >
>> >> > -    def lookup_type(self, name):
>> >> > -        return self.lookup_entity(name, QAPISchemaType)
>> >> > +    def lookup_type(self, name: str) -> Optional[QAPISchemaType]:

[...]

>> >> > +        typ = self.lookup_entity(name, QAPISchemaType)
>> >> > +        if typ is None:
>> >> > +            return None
>> >> > +        assert isinstance(typ, QAPISchemaType)
>> >> > +        return typ
>> >>
>> >> Would
>> >>
>> >>            typ = self.lookup_entity(name, QAPISchemaType)
>> >>            assert isinstance(typ, Optional[QAPISchemaType])
>> >>            return typ
>> >>
>> >> work?
>> >>
>> >
>> > I don't *think* so, Optional isn't a runtime construct.
>>
>> Let me try...
>>
>>     $ python
>>     Python 3.11.5 (main, Aug 28 2023, 00:00:00) [GCC 12.3.1 20230508 (Red 
>> Hat 12.3.1-1)] on linux
>>     Type "help", "copyright", "credits" or "license" for more information.
>>     >>> from typing import Optional
>>     >>> x=None
>>     >>> isinstance(x, Optional[str])
>>     True
>>     >>>
>
> Huh. I ... huh!
>
> Well, this apparently only works in Python 3.10!+
>
> TypeError: Subscripted generics cannot be used with class and instance checks

We should be able to use it "soon" after 3.9 reaches EOL, approximately
October 2025.

*Sigh*

>>
>> >                                                         We can combine it
>> > into "assert x is None or isinstance(x, foo)" though - I believe that's
>> > used elsewhere in the qapi generator.
>>
>> >> >
>> >> >      def resolve_type(self, name, info, what):
>> >> >          typ = self.lookup_type(name)
>> >>
>> >>
>>