From: "Daniel P. Berrange" <berrange@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com
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 [thread overview]
Message-ID: <20151105160114.GG15525@redhat.com> (raw)
In-Reply-To: <1446737402-15597-4-git-send-email-armbru@redhat.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 :|
next prev parent reply other threads:[~2015-11-05 16:01 UTC|newest]
Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-04 6:20 [Qemu-devel] [PATCH v9 00/27] alternate layout (post-introspection cleanups, subset C) Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 01/27] qapi: Use generated TestStruct machinery in tests Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 02/27] qapi: Strengthen test of TestStructList Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 03/27] qapi: Plug leaks in test-qmp-* Eric Blake
2015-11-04 8:19 ` Markus Armbruster
2015-11-04 17:24 ` Eric Blake
2015-11-04 17:44 ` Markus Armbruster
2015-11-05 13:09 ` Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 04/27] qapi: Simplify error testing " Eric Blake
2015-11-04 8:40 ` Markus Armbruster
2015-11-04 21:05 ` Eric Blake
2015-11-05 7:53 ` Markus Armbruster
2015-11-05 15:04 ` Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 05/27] qapi: More tests of alternate output Eric Blake
2015-11-04 9:04 ` Markus Armbruster
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 06/27] qapi: Test failure in middle of array parse Eric Blake
2015-11-04 9:07 ` Markus Armbruster
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 07/27] qapi: More tests of input arrays Eric Blake
2015-11-04 9:11 ` Markus Armbruster
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 08/27] qapi: Provide nicer array names in introspection Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 09/27] qapi-introspect: Document lack of sorting Eric Blake
2015-11-04 10:09 ` Markus Armbruster
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 10/27] qapi: Track simple union tag in object.local_members Eric Blake
2015-11-04 11:02 ` Markus Armbruster
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 11/27] qapi-types: Consolidate gen_struct() and gen_union() Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 12/27] qapi-types: Simplify gen_struct_field[s] Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 13/27] qapi: Drop obsolete tag value collision assertions Eric Blake
2015-11-04 13:30 ` Markus Armbruster
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 14/27] qapi: Fix up commit 7618b91's clash sanity checking change Eric Blake
2015-11-04 13:36 ` Markus Armbruster
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 15/27] qapi: Simplify QAPISchemaObjectTypeMember.check() Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 16/27] qapi: Eliminate QAPISchemaObjectType.check() variable members Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 17/27] qapi: Clean up after previous commit Eric Blake
2015-11-04 13:43 ` Markus Armbruster
2015-11-04 23:03 ` Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 18/27] qapi: Factor out QAPISchemaObjectTypeMember.check_clash() Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 19/27] qapi: Check for qapi collisions of flat union branches Eric Blake
2015-11-04 19:01 ` Markus Armbruster
2015-11-04 23:11 ` Eric Blake
2015-11-04 23:25 ` Eric Blake
2015-11-05 7:59 ` Markus Armbruster
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 20/27] qapi: Simplify QAPISchemaObjectTypeVariants.check() Eric Blake
2015-11-04 19:02 ` Markus Armbruster
2015-11-04 23:12 ` Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 21/27] qapi: Factor out QAPISchemaObjectType.check_clash() Eric Blake
2015-11-05 15:29 ` [Qemu-devel] [PATCH RFC 0/5] qapi: Use common name mangling for enumeration constants Markus Armbruster
2015-11-05 15:29 ` [Qemu-devel] [PATCH RFC 1/5] qapi: Generate a sed script to help eliminate camel_to_upper() Markus Armbruster
2015-11-05 15:29 ` [Qemu-devel] [PATCH RFC 2/5] Revert "qapi: Generate a sed script to help eliminate camel_to_upper()" Markus Armbruster
2015-11-05 15:30 ` [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants Markus Armbruster
2015-11-05 16:01 ` Daniel P. Berrange [this message]
2015-11-05 16:41 ` Eric Blake
2015-11-05 22:36 ` Eric Blake
2015-11-06 10:03 ` Markus Armbruster
2015-11-06 13:35 ` Markus Armbruster
2015-11-10 14:35 ` [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-16 22:13 ` [Qemu-devel] blkdebug event names [was: What to do about QAPI naming convention violations] Eric Blake
2015-11-17 7:38 ` Markus Armbruster
2015-11-09 9:34 ` [Qemu-devel] [PATCH RFC 3/5] qapi: Use common name mangling for enumeration constants Markus Armbruster
2015-11-09 10:53 ` Daniel P. Berrange
2015-11-05 15:30 ` [Qemu-devel] [PATCH RFC 4/5] crypto: Drop name mangling override Markus Armbruster
2015-11-05 15:30 ` [Qemu-devel] [PATCH RFC 5/5] Revert "qapi: allow override of default enum prefix naming" Markus Armbruster
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 22/27] qapi: Remove outdated tests related to QMP/branch collisions Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 23/27] qapi: Simplify visiting of alternate types Eric Blake
2015-11-05 17:01 ` Markus Armbruster
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 24/27] qapi: Fix alternates that accept 'number' but not 'int' Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 25/27] qapi: Add positive tests to qapi-schema-test Eric Blake
2015-11-05 18:44 ` Markus Armbruster
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 26/27] qapi: Remove dead visitor code Eric Blake
2015-11-05 19:05 ` Markus Armbruster
2015-11-11 6:13 ` Eric Blake
2015-11-04 6:20 ` [Qemu-devel] [PATCH v9 27/27] qapi: Simplify visits of optional fields Eric Blake
2015-11-04 10:22 ` [Qemu-devel] [PATCH v9 00/27] alternate layout (post-introspection cleanups, subset C) Markus Armbruster
2015-11-04 15:06 ` Eric Blake
2015-11-04 18:04 ` Markus Armbruster
2015-11-05 19:45 ` Markus Armbruster
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151105160114.GG15525@redhat.com \
--to=berrange@redhat.com \
--cc=armbru@redhat.com \
--cc=mdroth@linux.vnet.ibm.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).