Re: [PATCH v2 11/16] scripts/qapi: pull c_name from camel_to_upper to caller

Paolo Bonzini <[email protected]> writes:

> Allow using it for other languages too.
>
> Signed-off-by: Paolo Bonzini <[email protected]>
> ---
>  scripts/qapi/common.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 14d5dd259c4..c75396a01b5 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -61,7 +61,7 @@ def camel_to_upper(value: str) -> str:
>          ret += ch
>          upc = ch.isupper()
>  
> -    return c_name(ret.upper()).lstrip('_')
> +    return ret.upper()
>  
>  
>  def c_enum_const(type_name: str,
> @@ -75,7 +75,7 @@ def c_enum_const(type_name: str,
>      :param prefix: Optional, prefix that overrides the type_name.
>      """
>      if prefix is None:
> -        prefix = camel_to_upper(type_name)
> +        prefix = c_name(camel_to_upper(type_name)).lstrip('_')
>      return prefix + '_' + c_name(const_name, False).upper()

No change in behavior, because this is the only use of camel_to_upper().

The move of c_name() out of camel_to_upper() is clearly wanted to make
it usable for other languages.

What about .lstrip('_')?  I wonder why we even have it.  Goes back all
the way to commit 6299659f544 (qapi script: code move for
generate_enum_name()) from 2014.  It seems to affect just
tests/qapi-schema/qapi-schema-test.json's

    { 'enum': '__org.qemu_x-Enum', 'data': [ '__org.qemu_x-value' ] }

This tests a downstream extension prefix __RFQDN_.

As is, we generate

    typedef enum __org_qemu_x_Enum {
        ORG_QEMU_X_ENUM___ORG_QEMU_X_VALUE,
        ORG_QEMU_X_ENUM__MAX,
    } __org_qemu_x_Enum;

Without the .lstrip('_'), we'd generate

    typedef enum __org_qemu_x_Enum {
        __ORG_QEMU_X_ENUM___ORG_QEMU_X_VALUE,
        __ORG_QEMU_X_ENUM__MAX,
    } __org_qemu_x_Enum;

Meh.

Turns out this isn't on purpose.  Back then, the loop to map camel to
upper worked differently, and produced unwanted leading '_'.  For
instance, it mapped 'AbraCadabra' to '_ABRA_CADABRA'.  The .lstrip()
made the function produce 'ABRA_CADABRA'.  It also makes it eat
downstream extensions' leading '__'.  Oopsie.

By moving it, we insulate Rust from this accident.  Okay.

We could delete it instead.  Might inconvenience downstreams.  Not a
demand; you may wish to keep this series focused on Rust.