[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 07/12] qapi/schema: make QAPISourceInfo mandatory

From: John Snow
Subject: Re: [PATCH v2 07/12] qapi/schema: make QAPISourceInfo mandatory
Date: Wed, 13 Jan 2021 19:47:17 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0

On 1/13/21 7:29 PM, Eduardo Habkost wrote:
On Wed, Jan 13, 2021 at 06:04:29PM -0500, John Snow wrote:
On 1/13/21 11:12 AM, Markus Armbruster wrote:
John Snow <jsnow@redhat.com> writes:


I think we need to, yes; or we probably really, really want to. Making the
info parameter optional really adds a lot of unidiomatic type-checking
confetti when we go to use info, and in many more contexts than just this
sorta-built-in-enum; it will creep badly.

Which methods would require unidiomatic type-checking because of

Virtually everything that accepts a QAPISourceInfo has to do something like this:

def foo(info: Optional[QAPISourceInfo]):
    if info is None:
        raise Exception("Something very bad has happened!")

Since the great majority of cases *will* have an info, and we take careful pains to make sure this info is preserved, it's less invasive to just assert that info isn't Optional.

This is especially true for the QAPISchemaMember initializer, which several other classes inherit -- if this is allowed to be Optional[QAPISourceInfo], any and all users of a QAPISchemaMember or any derived classes will have to check -- on every access -- to see if 'member.info' is set or not.

Since we expect it to be set 100% of the time for all user-defined code, it's a lot less "if member.info is not None" checks everywhere.

Adding a special "built-in" source info object to make this claim helps avoid making confusing type signatures for the visitors; i.e.

def visit_thing(info: Optional[QAPISourceInfo]): ...

is misleading, because we actually expect thing to *always* have a SourceInfo. To make the documentation be a little more ... statistically truthful, it's easier to bend the rules in the other direction and just fill in the scant few cases where we don't have a QAPISourceInfo.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]