On Tue, Nov 21, 2023, 9:17 AM Markus Armbruster <arm...@redhat.com> wrote:
> John Snow <js...@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 <js...@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?
>
