From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57061) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZuMya-00011V-Nz for qemu-devel@nongnu.org; Thu, 05 Nov 2015 11:01:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZuMyU-0003la-Lb for qemu-devel@nongnu.org; Thu, 05 Nov 2015 11:01:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45257) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZuMyU-0003lB-EK for qemu-devel@nongnu.org; Thu, 05 Nov 2015 11:01:18 -0500 Date: Thu, 5 Nov 2015 16:01:14 +0000 From: "Daniel P. Berrange" Message-ID: <20151105160114.GG15525@redhat.com> References: <1446618049-13596-22-git-send-email-eblake@redhat.com> <1446737402-15597-1-git-send-email-armbru@redhat.com> <1446737402-15597-4-git-send-email-armbru@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1446737402-15597-4-git-send-email-armbru@redhat.com> Subject: Re: [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants Reply-To: "Daniel P. Berrange" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com On Thu, Nov 05, 2015 at 04:30:00PM +0100, Markus Armbruster wrote: > QAPI names needn't be valid C identifiers, so we mangle them with > c_name(). Except for enumeration constants, which we mangle with > camel_to_upper(). > > c_name() is easy enough to understand: replace '.' and '-' by '_', > prefix certain ticklish identifiers with 'q_'. > > camel_to_upper() is a hairball of heuristics, and guessing how it'll > mangle interesting input could serve as a (nerdy) game. Despite some > tweaking (commit 5d371f4), it's still inadqeuate for some QAPI names > (commit 351d36e). > > Example: QAPI definition > > { 'enum': 'BlockDeviceIoStatus', 'data': [ 'ok', 'failed', 'nospace' ] } > > generates > > typedef enum BlockDeviceIoStatus { > BLOCK_DEVICE_IO_STATUS_OK = 0, > BLOCK_DEVICE_IO_STATUS_FAILED = 1, > BLOCK_DEVICE_IO_STATUS_NOSPACE = 2, > BLOCK_DEVICE_IO_STATUS_MAX = 3, > } BlockDeviceIoStatus; > > Observe that c_name() maps BlockDeviceIoStatus to itself, and > camel_to_upper() maps it to BLOCK_DEVICE_IO_STATUS, i.e. the > enumeration constants are shouted, the enumeration type isn't. > > Because mangled names must not clash, name mangling restricts the > names you can use. For example, you can't have member 'a-b' and > 'a_b'. > > Having two separate manglings complicates this. Enumeration constants > must be distinct after mangling with camel_to_upper(). But as soon as > you use the enumeration as union tag, they must *also* be distinct > after mangling with c_name(). > > Having shouted enumeration constants isn't worth the complexity cost. I rather disagree with this. Having all-uppercase for enum constants is a really widely used convention and I think it is pretty useful when reading to code to have constants (whether #define/enum) clearly marked in uppercase. After this change we'll have situation where QAPI enums uses CamelCase, while all other non-QAPI enums (whether defined in QEMU source, or via #included 3rd party headers use UPPER_CASE for constants. I think that's rather unpleasant. I agree with your premise that predicting the output of the qapi name mangler though is essentially impossible for mortals. How about a counter proposal.... First make 'prefix' compulsory for enums, instead of trying to figure out a suitable prefix automatically. Then, have a very simple rule for the enum constants where you just uppercase a-z and translate any non-alpha-numeric character into a _. Don't try todo anything else special like figuring out word boundaries. That would get of much of the complexity in the qapi name mangler and give a easily predictable enum constant name. Thus the vast majority of .c source files (possibly even all of them) would not need any change. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|