qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
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 :|

  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).