John Snow <js...@redhat.com> writes:
> 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!
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.
