On 05/05/2015 03:11 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> 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
