Re: [Qemu-devel] [PATCH v11 19/28] qapi: Change munging of CamelCase enum values

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 11/13/2015 11:13 AM, Markus Armbruster wrote:
>
>> We need c_name() to protect ticklish identifiers only when its result is
>> used as identifier.  Not when it's *part* of an identifier,
>> e.g. prefixed with qapi_, or camel_to_upper(type_name) + '_'.
>> 
>> We can protect even when we don't need to, if that helps keeping things
>> simple.
>
> As far as I can tell, as soon as we reserve Q_ in addition to q_, and
> fix the bug in c_name() applying munging backwards, then we are safe to
> state that any comparison between two names for collisions will be
> accurate as long as both sides of the comparison picked the same value
> of c_name(name, protect), whether or not our later use of that name is
> done with protect=True or protect=False.

Okay.

>> The obvious simple way to check for collisions works like this:
>> 
>> 1. Every QAPI name is mangled in exactly one way, modulo case: always
>>    with c_name(), and always with the same value of protect.
>
> That part is easy.  Maybe I could even reuse guardname(name) instead of
> c_name(name).upper(), although I don't know that it buys any clarity.

Certainly not without a rename :)

>> 2. We require the mangled name to be case-insensitively unique in its
>>    name space.
>
> That part is a bit harder: we unfortunately have the existing clash
> between the command 'stop' and the event 'STOP'.  I didn't find any
> other clashes in existing clients, though (phew), so we can either
> whitelist just that one, or more likely, set up separate namespaces for
> commands vs. events when doing the case collision tests.

I think the natural split is three: commands, events, types.

> But it's a bummer that it won't be quite as easy as I had hoped (using a
> single dictionary for both lookups and case collisions is easier than
> having two dictionaries to separate namespaces, while still searching
> both dictionaries for lookups).
>
> At any rate, I'm playing with patches along these lines.

Before I dive into yet another lengthy treatise: I strongly recommend to
kick clash detection work as far back in the queue as practical.  It's
hairy, and improving it isn't a priority.  It has clogged the queue more
than enough already.


Let's take a step back and consider requirements.  I think there are
broadly two: the QAPI schema must make sense, and generation of clashing
C identifiers should be caught.


QAPI's requirements are pretty simple.  It's always clear whether a name
denotes a command, event, type, or member of a certain type.  Thus,
requiring names to be unique within their kind suffices.  In other
words, separate name spaces for commands, events, types and for each
type's members.

To avoid confusion, we can require more.  Right now, we require command,
event and type names to be unique within their combined kinds.  In other
words, a combined name space for commands, events and types.


Catching C clashes is more complicated, because we need to analyze the
generators' (less than disciplined) use of C's name spaces to find its
requirements.

QAPI names get mangled and combined with prefixes or suffixes to become
C identifiers.

Prefixes and suffixes can only reduce clashes among generated names
(they can require reserving names, but let's ignore that here).
Therefore, considering clashes between mangled QAPI names suffices.

If we mangled each name in exactly one way, this would be trivial: use
the mangled name as dictionary key, and use a separate dictionary for
each C name space.

Note it's trivial even with multiple ways to mangle, as long as each
name gets mangled in exactly one of them.

Unfortunately, this is not the case.

We mangle in the following ways:

* Standard mangling with c_name()

  Variations: protect=False, shouted, unshouted.

* camel_to_upper() mangling

Names that get mangled in more than one way, to the best of my
knowledge:

(1) Enumeration member names

  - Shouted standard mangling for the enumeration constant
  - Standard mangling for the union member when used as variant tag

(2) Enumeration type names

  - Standard mangling for the typedef
  - camel_to_upper() mangling for the enumeration constants

(3) Event names

  - Unshouted standard mangling for the function to emit the event
  - Shouted standard mangling for the enumeration constants

(4) Possibly built-in type names

  - Standard mangling with protect=False
  - If we screw up somehow, standard mangling without protect=False

General solution: put all possible manglings into the name space's
dictionary.  For example, enter a command name in standard mangling, but
enter an event name twice, once in unshouted standard mangling, and once
in shouted standard mangling.

If we peek into a few black boxes, we can simplify things a bit.

We use C's ordinary identifier name space, the struct tag name space,
and the member name spaces of structs and unions we generate.

We don't need dictionaries for the C struct, union and enum tag name
spaces, because we always use the same identifier in the ordinary name
space.

We can safely ignore protect=False since we reserve the q_ prefix in all
variations, and avoid its use outside c_name().  Standard mangling as
key suffices for (4).

We can ignore a mangling when we can prove it can clash only when
another one clashes:

* An event name's unshouted standard mangling can clash only when the
  shouted mangling clashes.  Down to one key, but it's still an unusual
  one.

* If we enforce "enumeration member names must be lower-case", then
  shouted standard mangling can clash only when standard mangling
  clashes.  Use of standard mangling as key suffices for (1).

We can cover *all* case variations of standard mangling by making the
dictionary case-insensitive.  Unfortunately, this runs into the STOP
vs. stop roadblock in the ordinary identifier dictionary.  No go.

We could split the ordinary identifier dictionary into events, commands
and types (the split that exists in QAPI) if generators ensure
identifiers from different parts can't clash, by restricting names or by
adding prefixes or siffixes.  I think that's the case, but it's a
non-trivial argument.  The resulting dictionaries could then be made
case-insensitive separately.  But this is getting too complex for my
taste.

Instead of or in addition to simplifying clash detection, we can
simplify mangling:

* If we enforce "event name must be ALL_CAPS", then shouted standard
  mangling can be trivially replaced by standard mangling.  Since we
  already showed that unshouted standard mangling can be ignored, use of
  standard mangling as key suffices for (3).

* That leaves (2).  To make standard mangling suffice there as well, we
  can simply replace camel_to_upper() mangling by standard mangling.  If
  that's inopportune, we'll need to find another solution.

To be honest, I'm *this* close to giving up on clash detection entirely.
It's 100% self-inflicted pain.