Re: [PATCH 12/22] qapi/parser: add type hint annotations

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 4/25/21 8:34 AM, Markus Armbruster wrote:
>> value: object isn't wrong, but why not _ExprValue?
>> 
>
> Updated excuse:
>
> because all the way back outside in _parse, we know that:
>
> 1. expr is a dict (because of get_expr(False))
> 2. expr['pragma'] is also a dict, because we explicitly check it there.

Yes:

                pragma = expr['pragma']
-->             if not isinstance(pragma, dict):
-->                 raise QAPISemError(
-->                     info, "value of 'pragma' must be an object")
                for name, value in pragma.items():
                    self._pragma(name, value, info)

> 3. We iterate over the keys; all we know so far is that the values are
> ... something.

Actually, *we* know more about the values.  get_expr() returns a tree
whose inner nodes are dict or list, and whose leaves are str or bool.
Therefore, the values are dict, list, str, or bool.

It's *mypy* that doesn't know, because it lacks recursive types.

I know that you're prbably using "we" in the sense of "the toolchain".
I'm annoying you with the difference between "the toolchain" and "we
(you, me, and other humans) because I'm concerned about us humans
dumbing ourselves down to mypy's level of understanding.

To be honest, I'm less and less sure typing these trees without the
necessary typing tools is worth the bother.  The notational overhead it
more oppressive than elsewhere, and yet the typing remains weak.  The
result fails to satisfy, and that's a constant source of discussions
(between us as well as just in my head) on how to best mitigate.

> 4. _pragma()'s job is to validate the type(s) anyway.

_pragma() can safely assume @value is dict, list, str, or bool.  It just
happens not to rely on this assumption.

> More or less, the _ExprValue type union isn't remembered here -- even
> though it was once upon a time something returned by get_expr, it 
> happened in a nested call that is now opaque to mypy in this context.

Understand.

> So, it's some combination of "That's all we know about it" and "It
> happens to be exactly sufficient for this function to operate."

I can accept "it's all mypy can figure out by itself, and it's good
enough to get the static checking we want".