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

[Markus Armbruster]

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?

> +
>          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