On Thu, Mar 14, 2024, 9:58 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@redhat.com> writes: > > > On Thu, Mar 14, 2024, 9:01 AM Markus Armbruster <arm...@redhat.com> > wrote: > > > >> John Snow <js...@redhat.com> writes: > >> > >> > Declare, but don't initialize the "members" field with type > >> > List[QAPISchemaObjectTypeMember]. > >> > > >> > This simplifies the typing from what would otherwise be > >> > Optional[List[T]] to merely List[T]. This removes the need to add > >> > assertions to several callsites that this value is not None - which it > >> > never will be after the delayed initialization in check() anyway. > >> > > >> > The type declaration without initialization trick will cause > accidental > >> > uses of this field prior to full initialization to raise an > >> > AttributeError. > >> > > >> > (Note that it is valid to have an empty members list, see the internal > >> > q_empty object as an example. For this reason, we cannot use the empty > >> > list as a replacement test for full initialization and instead rely on > >> > the _checked/_check_complete fields.) > >> > > >> > Signed-off-by: John Snow <js...@redhat.com> > >> > --- > >> > scripts/qapi/schema.py | 12 +++++++----- > >> > 1 file changed, 7 insertions(+), 5 deletions(-) > >> > > >> > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > >> > index 50ebc4f12de..fb30314741a 100644 > >> > --- a/scripts/qapi/schema.py > >> > +++ b/scripts/qapi/schema.py > >> > @@ -20,7 +20,7 @@ > >> > from collections import OrderedDict > >> > import os > >> > import re > >> > -from typing import List, Optional > >> > +from typing import List, Optional, cast > >> > > >> > from .common import ( > >> > POINTER_SUFFIX, > >> > @@ -449,7 +449,7 @@ def __init__(self, name, info, doc, ifcond, > features, > >> > self.base = None > >> > self.local_members = local_members > >> > self.variants = variants > >> > - self.members = None > >> > + self.members: List[QAPISchemaObjectTypeMember] > >> > self._check_complete = False > >> > > >> > def check(self, schema): > >> > @@ -482,7 +482,11 @@ def check(self, schema): > >> > for m in self.local_members: > >> > m.check(schema) > >> > m.check_clash(self.info, seen) > >> > - members = seen.values() > >> > + > >> > + # check_clash works in terms of the supertype, but > local_members > >> > + # is asserted to be List[QAPISchemaObjectTypeMember]. > >> > >> Do you mean "but self.members is declared as > >> List[QAPISchemaObjectTypeMember]"? > > > > Argh. I meant asserted in the linguistic sense. mypy asserts it to be; > not > > a runtime assertion. > > > > I do this a lot, apparently. > > Okay to adjust your comment to my version? > Absolutely, of course! > [...] > >
