Re: [PATCH v5 34/36] qapi/visit.py: assert tag_member contains a QAPISchemaEnumType

[John Snow]

On 10/7/20 8:43 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > This is true by design, but not presently able to be expressed in the
> > type system. An assertion helps mypy understand our constraints.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
> > Reviewed-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   scripts/qapi/visit.py | 12 +++++++-----
> >   1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> > index 14f30c228b7..4f11fd325b8 100644
> > --- a/scripts/qapi/visit.py
> > +++ b/scripts/qapi/visit.py
> > @@ -22,7 +22,7 @@
> >       mcgen,
> >   )
> >   from .gen import QAPISchemaModularCVisitor, ifcontext
> > -from .schema import QAPISchemaObjectType
> > +from .schema import QAPISchemaEnumType, QAPISchemaObjectType
> >   
> >   
> >   def gen_visit_decl(name, scalar=False):
> > @@ -84,15 +84,17 @@ def gen_visit_object_members(name, base, members, variants):
> >           ret += gen_endif(memb.ifcond)
> >   
> >       if variants:
> > +        tag_member = variants.tag_member
> > +        assert isinstance(tag_member.type, QAPISchemaEnumType)
>
> I'm curious: do you need the local variable to make the assertion stick?

No, but it only sticks to the binding and not the data. i.e. assertions 
to downcast work on the *name*.

(This comes up somewhere in the schema.py patches where I make a change 
that looks completely pointless, but it makes mypy happy.)

I could have left it alone. I just saw a lot of repeated multi-dots and 
habitually created a temporary local for the purpose.

> > +
> >           ret += mcgen('''
> >       switch (obj->%(c_name)s) {
> >   ''',
> > -                     c_name=c_name(variants.tag_member.name))
> > +                     c_name=c_name(tag_member.name))
> >   
> >           for var in variants.variants:
> > -            case_str = c_enum_const(variants.tag_member.type.name,
> > -                                    var.name,
> > -                                    variants.tag_member.type.prefix)
> > +            case_str = c_enum_const(tag_member.type.name, var.name,
> > +                                    tag_member.type.prefix)
> >               ret += gen_if(var.ifcond)
> >               if var.type.name == 'q_empty':
> >                   # valid variant and nothing to do