Re: [PATCH 20/25] qapi: Improve reporting of missing / unknown definition keys

[Markus Armbruster]

Eric Blake <address@hidden> 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 <address@hidden>
>> ---
>
>> +++ 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 <address@hidden>

Thanks!