Re: [PATCH v3 4/7] qapi/expr: add typing workaround for AbstractSet

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> mypy can only narrow the type of `Mapping[str, ...].keys() & Set[str]`
> to `AbstractSet[str]` and not a `Set[str]`. As a result, if the type of
> an expression is changed to a Mapping[], mypy is unsure if the .pop() is
> safe.
>
> A forthcoming commit does exactly that, so wrap the expression in a
> set() constructor to force the intermediate expression to be resolved as
> a mutable type.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/expr.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index b56353bdf84..af802367eff 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -622,8 +622,8 @@ def check_expr(expr_elem: _JSONObject) -> None:
>      if 'include' in expr:
>          return
>  
> -    metas = expr.keys() & {'enum', 'struct', 'union', 'alternate',
> -                           'command', 'event'}
> +    metas = set(expr.keys() & {
> +        'enum', 'struct', 'union', 'alternate', 'command', 'event'})
>      if len(metas) != 1:
>          raise QAPISemError(
>              info,
                   "expression must have exactly one key"
                   " 'enum', 'struct', 'union', 'alternate',"
                   " 'command', 'event'")
           meta = metas.pop()

I'm mildly surprised that the result of operator & is considered
immutable.  How could it *not* be a freshly created set?  Oh well.

Passing a set to set() is a no-op, so

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Regardless, the code feels a bit too clever to me.  It actually did
already when I wrote it, but I the less clever ways I could think of
were also much more verbose.

The code checks that exactly one of a set of keys is present, and which
one it is.  Any ideas?