| Message ID | 20201026213637.47087-8-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: static typing conversion, pt3 | expand |
John Snow <jsnow@redhat.com> 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 <jsnow@redhat.com> > Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> > Reviewed-by: Cleber Rosa <crosa@redhat.com> > --- > 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 2654a72e8333..b8720b723377 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 Suggest # check_exprs() ensures this > members = expr['data'] > > 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 Likewise. > base = expr.get('base') > discriminator = expr.get('discriminator') > members = expr['data'] > @@ -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: > check_name_is_str(name, info, "'%s'" % meta) Uh, why do we need to cast then? It's not actually "asserted". check_name_is_str() ensures str by raising an error if not str. > info.set_defn(meta, name) > check_defn_name_str(name, info, meta)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index 2654a72e8333..b8720b723377 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'] 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'] @@ -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: check_name_is_str(name, info, "'%s'" % meta) info.set_defn(meta, name) check_defn_name_str(name, info, meta)