John Snow <[email protected]> writes:
> Casts are instructions to the type checker only, they aren't "safe" and
> should probably be avoided in general. In this case, when we perform
> type checking on a nested structure, the type of each field does not
> "stick".
>
> We don't need to assert that something is a str if we've already checked
> that it is -- use a cast instead for these cases.
>
> Signed-off-by: John Snow <[email protected]>
> Reviewed-by: Eduardo Habkost <[email protected]>
> Reviewed-by: Cleber Rosa <[email protected]>
> ---
> scripts/qapi/expr.py | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index afa6bd07769..f45d6be1f4c 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -15,7 +15,7 @@
> # See the COPYING file in the top-level directory.
>
> import re
> -from typing import MutableMapping, Optional
> +from typing import MutableMapping, Optional, cast
>
> from .common import c_name
> from .error import QAPISemError
> @@ -232,7 +232,7 @@ def check_enum(expr, info):
>
>
> def check_struct(expr, info):
> - name = expr['struct']
> + name = cast(str, expr['struct']) # Asserted in check_exprs
> members = expr['data']
>
I believe you need this only so you can declare check_type()'s
allow_dict to be Optional[str]. More busy-work around allow_dict...
I'm tempted to ask for allow_dict: Any and call it a day.
> check_type(members, info, "'data'", allow_dict=name)
> @@ -240,7 +240,7 @@ def check_struct(expr, info):
>
>
> def check_union(expr, info):
> - name = expr['union']
> + name = cast(str, expr['union']) # Asserted in check_exprs
> base = expr.get('base')
> discriminator = expr.get('discriminator')
> members = expr['data']
Likwewise.
> @@ -337,7 +337,7 @@ def check_exprs(exprs):
> else:
> raise QAPISemError(info, "expression is missing metatype")
>
> - name = expr[meta]
> + name = cast(str, expr[meta]) # Asserted right below:
"Checked", not "asserted".
> check_name_is_str(name, info, "'%s'" % meta)
> info.set_defn(meta, name)
> check_defn_name_str(name, info, meta)
Cast before check gives me a queasy feeling. It's lying to the type
checker.
Hamfisted way to avoid lying:
check_name_is_str(expr[meta], ...)
name = cast(str, expr[meta])
Something like
name = check_name_is_str(name, ...)
might be neater, but then we'd have to find a better function name.
