Re: [PATCH 07/16] qapi/expr.py: Add casts in a few select cases

[Cleber Rosa]

On Tue, Sep 22, 2020 at 05:13:04PM -0400, John Snow wrote:
> 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 

Reviewed-by: Cleber Rosa 


signature.asc
Description: PGP signature

Re: [PATCH 07/16] qapi/expr.py: Add casts in a few select cases

[Eduardo Habkost]

On Tue, Sep 22, 2020 at 05:13:04PM -0400, John Snow wrote:
> 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 

Reviewed-by: Eduardo Habkost 

-- 
Eduardo

[PATCH 07/16] qapi/expr.py: Add casts in a few select cases

[John Snow]

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 
---
 scripts/qapi/expr.py | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 633f9b9172..4a3352a2bd 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -15,7 +15,11 @@
 # 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
@@ -225,7 +229,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']
 
 check_type(members, info, "'data'", allow_dict=name)
@@ -233,7 +237,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']
@@ -330,7 +334,7 @@ def check_exprs(exprs):
 else:
 raise QAPISemError(info, "expression is missing metatype")
 
-name = expr[meta]
+name = cast(str, expr[meta])  # asserted right below:
 check_name_is_str(name, info, "'%s'" % meta)
 info.set_defn(meta, name)
 check_defn_name_str(name, info, meta)
-- 
2.26.2