Re: [Qemu-devel] [PATCH v8 20/40] qapi: Better error messages for duplicated expressions

[Eric Blake]

On 05/05/2015 03:11 AM, Markus Armbruster wrote:
> Eric Blake <address@hidden> writes:
> 
>> The previous commit demonstrated that the generator overlooked
>> duplicate expressions:
>> - a complex type or command reusing a built-in type name
>> - redeclaration of a type name, whether by the same or different
>> metatype
>> - redeclaration of a command or event
>> - collision of a type with implicit 'Kind' enum for a union
>> - collision with an implicit MAX enum constant
>>
>> Since the c_type() function in the generator treats all names
>> as being in the same namespace, this patch adds a global array
>> to track all known names and their source, to prevent collisions
>> before it can cause further problems.  While valid .json files
>> won't trigger any of these cases, we might as well be nicer to
>> developers that make a typo while trying to add new QAPI code.
>>

>> +def add_name(name, info, meta, implicit = False):
>> +    global all_names
>> +    if name in all_names:
>> +        raise QAPIExprError(info,
>> +                            "%s '%s' is already defined"
>> +                            %(all_names[name], name))
> 
> Let's put a space between binary operator % and its right operand.

I was copying existing examples, but I can easily do a separate followup
patch that unifies the style of % formatting (or switches to +
concatenation where that is more legible).

> 
>> +    if not implicit and name[-4:] == 'Kind':
>> +        raise QAPIExprError(info,
>> +                            "%s '%s' should not end in 'Kind'"
>> +                            %(meta, name))
> 
> Likewise.  Can fix up on commit.

Of course, I'll wait until any of your fixups in the pull request are
already in place to write such a followup, to minimize rebase churn.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature