| Message ID | 20231116014350.653792-8-jsnow@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi: statically type schema.py | expand |
John Snow <jsnow@redhat.com> writes: > lookup_type() is capable of returning None, but introspect.py isn't > prepared for that. (And rightly so, if these built-in types are absent, > something has gone hugely wrong.) > > RFC: This is slightly cumbersome as-is, but a patch at the end of this series > tries to address it with some slightly slicker lookup functions that > don't need as much hand-holding. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > scripts/qapi/introspect.py | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > index 67c7d89aae0..42981bce163 100644 > --- a/scripts/qapi/introspect.py > +++ b/scripts/qapi/introspect.py > @@ -227,10 +227,14 @@ def _use_type(self, typ: QAPISchemaType) -> str: > > # Map the various integer types to plain int > if typ.json_type() == 'int': > - typ = self._schema.lookup_type('int') > + tmp = self._schema.lookup_type('int') > + assert tmp is not None More laconic: assert tmp > + typ = tmp > elif (isinstance(typ, QAPISchemaArrayType) and > typ.element_type.json_type() == 'int'): > - typ = self._schema.lookup_type('intList') > + tmp = self._schema.lookup_type('intList') > + assert tmp is not None > + typ = tmp > # Add type to work queue if new > if typ not in self._used_types: > self._used_types.append(typ) Not fond of naming things @tmp, but I don't have a better name to offer. We could avoid the lookup by having _def_predefineds() set suitable attributes, like it serts .the_empty_object_type. Matter of taste. Not now unless you want to.
On Tue, Nov 21, 2023, 9:17 AM Markus Armbruster <armbru@redhat.com> wrote: > John Snow <jsnow@redhat.com> writes: > > > lookup_type() is capable of returning None, but introspect.py isn't > > prepared for that. (And rightly so, if these built-in types are absent, > > something has gone hugely wrong.) > > > > RFC: This is slightly cumbersome as-is, but a patch at the end of this > series > > tries to address it with some slightly slicker lookup functions that > > don't need as much hand-holding. > > > > Signed-off-by: John Snow <jsnow@redhat.com> > > --- > > scripts/qapi/introspect.py | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py > > index 67c7d89aae0..42981bce163 100644 > > --- a/scripts/qapi/introspect.py > > +++ b/scripts/qapi/introspect.py > > @@ -227,10 +227,14 @@ def _use_type(self, typ: QAPISchemaType) -> str: > > > > # Map the various integer types to plain int > > if typ.json_type() == 'int': > > - typ = self._schema.lookup_type('int') > > + tmp = self._schema.lookup_type('int') > > + assert tmp is not None > > More laconic: assert tmp > *looks up "laconic"* hey, "terse" is even fewer letters! (but, you're right. I think I adopted the "is not none" out of a habit for distinguishing false-y values from the None value, but in this case we really wouldn't want to have either, so the shorter form is fine, though for mypy's sake we only care about guarding against None here.) > > + typ = tmp > > elif (isinstance(typ, QAPISchemaArrayType) and > > typ.element_type.json_type() == 'int'): > > - typ = self._schema.lookup_type('intList') > > + tmp = self._schema.lookup_type('intList') > > + assert tmp is not None > > + typ = tmp > > # Add type to work queue if new > > if typ not in self._used_types: > > self._used_types.append(typ) > > Not fond of naming things @tmp, but I don't have a better name to offer. > > We could avoid the lookup by having _def_predefineds() set suitable > attributes, like it serts .the_empty_object_type. Matter of taste. Not > now unless you want to. > Check the end of the series for different lookup methods, too. We can discuss your preferred solution then, perhaps? >
John Snow <jsnow@redhat.com> writes: > On Tue, Nov 21, 2023, 9:17 AM Markus Armbruster <armbru@redhat.com> wrote: > >> John Snow <jsnow@redhat.com> writes: >> >> > lookup_type() is capable of returning None, but introspect.py isn't >> > prepared for that. (And rightly so, if these built-in types are absent, >> > something has gone hugely wrong.) >> > >> > RFC: This is slightly cumbersome as-is, but a patch at the end of this >> series >> > tries to address it with some slightly slicker lookup functions that >> > don't need as much hand-holding. >> > >> > Signed-off-by: John Snow <jsnow@redhat.com> >> > --- >> > scripts/qapi/introspect.py | 8 ++++++-- >> > 1 file changed, 6 insertions(+), 2 deletions(-) >> > >> > diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py >> > index 67c7d89aae0..42981bce163 100644 >> > --- a/scripts/qapi/introspect.py >> > +++ b/scripts/qapi/introspect.py >> > @@ -227,10 +227,14 @@ def _use_type(self, typ: QAPISchemaType) -> str: >> > >> > # Map the various integer types to plain int >> > if typ.json_type() == 'int': >> > - typ = self._schema.lookup_type('int') >> > + tmp = self._schema.lookup_type('int') >> > + assert tmp is not None >> >> More laconic: assert tmp >> > > *looks up "laconic"* > > hey, "terse" is even fewer letters! Touché! > (but, you're right. I think I adopted the "is not none" out of a habit for > distinguishing false-y values from the None value, but in this case we It's a good habit. > really wouldn't want to have either, so the shorter form is fine, though > for mypy's sake we only care about guarding against None here.) Right. >> > + typ = tmp >> > elif (isinstance(typ, QAPISchemaArrayType) and >> > typ.element_type.json_type() == 'int'): >> > - typ = self._schema.lookup_type('intList') >> > + tmp = self._schema.lookup_type('intList') >> > + assert tmp is not None >> > + typ = tmp >> > # Add type to work queue if new >> > if typ not in self._used_types: >> > self._used_types.append(typ) >> >> Not fond of naming things @tmp, but I don't have a better name to offer. >> >> We could avoid the lookup by having _def_predefineds() set suitable >> attributes, like it serts .the_empty_object_type. Matter of taste. Not >> now unless you want to. >> > > Check the end of the series for different lookup methods, too. We can > discuss your preferred solution then, perhaps? Works for me.
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py index 67c7d89aae0..42981bce163 100644 --- a/scripts/qapi/introspect.py +++ b/scripts/qapi/introspect.py @@ -227,10 +227,14 @@ def _use_type(self, typ: QAPISchemaType) -> str: # Map the various integer types to plain int if typ.json_type() == 'int': - typ = self._schema.lookup_type('int') + tmp = self._schema.lookup_type('int') + assert tmp is not None + typ = tmp elif (isinstance(typ, QAPISchemaArrayType) and typ.element_type.json_type() == 'int'): - typ = self._schema.lookup_type('intList') + tmp = self._schema.lookup_type('intList') + assert tmp is not None + typ = tmp # Add type to work queue if new if typ not in self._used_types: self._used_types.append(typ)
lookup_type() is capable of returning None, but introspect.py isn't prepared for that. (And rightly so, if these built-in types are absent, something has gone hugely wrong.) RFC: This is slightly cumbersome as-is, but a patch at the end of this series tries to address it with some slightly slicker lookup functions that don't need as much hand-holding. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/introspect.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)