Eric Blake <ebl...@redhat.com> writes: > On 9/24/19 8:28 AM, Markus Armbruster wrote: >> Have check_exprs() call check_keys() later, so its error messages gain >> an "in definition" line. >> >> Both check_keys() and check_name_is_str() check the definition's name >> is a string. Since check_keys() now runs after check_name_is_str() >> rather than before, its check is dead. Bury it. Checking values in >> check_keys() is unclean anyway. >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- > >> +++ b/scripts/qapi/common.py >> @@ -905,8 +905,6 @@ def check_known_keys(value, info, source, required, >> optional): >> >> def check_keys(expr, info, meta, required, optional=[]): >> name = expr[meta] >> - if not isinstance(name, str): >> - raise QAPISemError(info, "'%s' key must have a string value" % meta) > > Should this be replaced with an assert? But I'm also okay just dropping > it, since our testsuite shows that we still flag the problems that this > message was originally used for.
I'd prefer not to assert, because as of this patch, check_keys() *only* checks keys, just like its name suggests. > Reviewed-by: Eric Blake <ebl...@redhat.com> Thanks!
