[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enum
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants |
Date: |
Thu, 5 Nov 2015 16:01:14 +0000 |
User-agent: |
Mutt/1.5.23 (2015-06-09) |
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 :|
- Re: [Qemu-devel] [PATCH v9 04/27] qapi: Simplify error testing in test-qmp-*, (continued)
[Qemu-devel] [PATCH v9 21/27] qapi: Factor out QAPISchemaObjectType.check_clash(), Eric Blake, 2015/11/04
- [Qemu-devel] [PATCH RFC 0/5] qapi: Use common name mangling for enumeration constants, Markus Armbruster, 2015/11/05
- [Qemu-devel] [PATCH RFC 1/5] qapi: Generate a sed script to help eliminate camel_to_upper(), Markus Armbruster, 2015/11/05
- [Qemu-devel] [PATCH RFC 2/5] Revert "qapi: Generate a sed script to help eliminate camel_to_upper()", Markus Armbruster, 2015/11/05
- [Qemu-devel] [PATCH RFC 4/5] crypto: Drop name mangling override, Markus Armbruster, 2015/11/05
- [Qemu-devel] [PATCH RFC 5/5] Revert "qapi: allow override of default enum prefix naming", Markus Armbruster, 2015/11/05
- [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants, Markus Armbruster, 2015/11/05
- Re: [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants,
Daniel P. Berrange <=
- Re: [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants, Eric Blake, 2015/11/05
- Re: [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants, Eric Blake, 2015/11/05
- Re: [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants, Markus Armbruster, 2015/11/06
- Re: [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants, Markus Armbruster, 2015/11/06
- [Qemu-devel] What to do about QAPI naming convention violations (was: [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants), Markus Armbruster, 2015/11/10
- [Qemu-devel] blkdebug event names [was: What to do about QAPI naming convention violations], Eric Blake, 2015/11/16
- Re: [Qemu-devel] blkdebug event names [was: What to do about QAPI naming convention violations], Markus Armbruster, 2015/11/17
Re: [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants, Markus Armbruster, 2015/11/09
Re: [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants, Daniel P. Berrange, 2015/11/09
[Qemu-devel] [PATCH v9 17/27] qapi: Clean up after previous commit, Eric Blake, 2015/11/04