On 11/10/2015 11:51 PM, Eric Blake wrote: > When munging enum values, the fact that we were passing the entire > prefix + value through camel_to_upper() meant that enum values > spelled with CamelCase could be turned into CAMEL_CASE. However, > this provides a potential collision (both OneTwo and One-Two would > munge into ONE_TWO). By changing the generation of enum constants > to always be prefix + '_' + c_name(value).upper(), we can avoid > any risk of collisions (if we can also ensure no case collisions, > in the next patch) without having to think about what the > heuristics in camel_to_upper() will actually do to the value. > > +++ b/scripts/qapi.py > @@ -1439,7 +1439,7 @@ def camel_to_upper(value): > def c_enum_const(type_name, const_name, prefix=None): > if prefix is not None: > type_name = prefix > - return camel_to_upper(type_name + '_' + const_name) > + return camel_to_upper(type_name) + '_' + c_name(const_name, False).upper() Doesn't match the commit message, because it used c_name(,False), while c_name(name) is short for c_name(name, True). What's more, looking at it exposed a bug: c_name('_Thread-local') returns '_Thread_local', but this collides with a C11 keyword (and was supposed to be munged to q__Thread_local); add '_Thread-local':'int' to a struct to see the resulting hilarity: CC tests/test-qmp-output-visitor.o In file included from tests/test-qmp-output-visitor.c:17:0: tests/test-qapi-types.h:781:13: error: expected identifier or ‘(’ before ‘_Thread_local’ int64_t _Thread_local; ^ But it also made me realize that c_name('Q-int') happily returns 'Q_int' (we only reserved the leading 'q_' namespace, not 'Q_'). Alas, that means c_name('Q-int', False).upper() and c_name('int', False).upper() both produce 'Q_INT', and we have a collision. So I think enum names have to be munged by c_name(name, True). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org