Re: [PATCH v5 24/25] qapi: Tighten check whether implicit object type already exists

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Fri, Mar 15, 2024, 11:23 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> Entities with names starting with q_obj_ are implicit object types.
>> Therefore, QAPISchema._make_implicit_object_type()'s .lookup_entity()
>> can only return a QAPISchemaObjectType.  Assert that.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  scripts/qapi/schema.py | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
>> index e52930a48a..a6180f93c6 100644
>> --- a/scripts/qapi/schema.py
>> +++ b/scripts/qapi/schema.py
>> @@ -1297,8 +1297,9 @@ def _make_implicit_object_type(
>>              return None
>>          # See also QAPISchemaObjectTypeMember.describe()
>>          name = 'q_obj_%s-%s' % (name, role)
>> -        typ = self.lookup_entity(name, QAPISchemaObjectType)
>> +        typ = self.lookup_entity(name)
>>          if typ:
>> +            assert(isinstance(typ, QAPISchemaObjectType))
>>              # The implicit object type has multiple users.  This can
>>              # only be a duplicate definition, which will be flagged
>>              # later.
>> --
>> 2.44.0
>>
>
> Seems obviously fine, though I don't suppose this narrowing will be
> "remembered" by the type system. Do we care?

mypy passes without it.  It's for catching programming errors and
helping the reader along.  The former are unlikely, and the latter is
debatable, but when in doubt, assert.

> Reviewed-by: John Snow <jsnow@redhat.com>

Thanks!