Eric Blake <ebl...@redhat.com> writes:
> Now that we have separated union tag values from colliding with
> non-variant C names, by naming the union 'u', we should reserve
> this name for our use. Note that we want to forbid 'u' even in
> a struct with no variants, because it is possible for a future
> qemu release to extend QMP in a backwards-compatible manner while
> converting from a struct to a flat union. Fortunately, no
> existing clients were using this member name. If we ever find
> the need for QMP to have a member 'u', we could at that time
> relax things, perhaps by having c_name() munge the QMP member to
> 'q_u'.
>
> Note that we cannot forbid 'u' everywhere (by adding the
> rejection code to check_name()), because the existing QKeyCode
> enum already uses it; therefore we only reserve it as a struct
> type member name.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
>
> ---
> v11: commit message tweaks, use c_name(), tweak test names
> v10: new patch, split from 1/17 and 3/17; by deferring this until
> after the rename was complete, it reduces churn due to the new feature
> ---
> scripts/qapi.py | 2 +-
> tests/Makefile | 1 +
> tests/qapi-schema/reserved-member-u.err | 1 +
> tests/qapi-schema/reserved-member-u.exit | 1 +
> tests/qapi-schema/reserved-member-u.json | 7 +++++++
> tests/qapi-schema/reserved-member-u.out | 0
> 6 files changed, 11 insertions(+), 1 deletion(-)
> create mode 100644 tests/qapi-schema/reserved-member-u.err
> create mode 100644 tests/qapi-schema/reserved-member-u.exit
> create mode 100644 tests/qapi-schema/reserved-member-u.json
> create mode 100644 tests/qapi-schema/reserved-member-u.out
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 3fead04..89f3207 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -490,7 +490,7 @@ def check_type(expr_info, source, value,
> allow_array=False,
> for (key, arg) in value.items():
> check_name(expr_info, "Member of %s" % source, key,
> allow_optional=allow_optional)
> - if c_name(key).startswith('has_'):
> + if c_name(key, False) == 'u' or c_name(key).startswith('has_'):
Slightly odd: new c_name() has protect=False, the existing one doesn't.
While we don't really need protect=False, it feels a bit cleaner. If
you like, I can add it to the existing one when it gets added in PATCH
05.
> raise QAPIExprError(expr_info,
> "Member of %s uses reserved name '%s'"
> % (source, key))
> diff --git a/tests/Makefile b/tests/Makefile
> index 6d7c07e..f448b5a 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -318,6 +318,7 @@ qapi-schema += redefined-type.json
> qapi-schema += reserved-command-q.json
> qapi-schema += reserved-member-has.json
> qapi-schema += reserved-member-q.json
> +qapi-schema += reserved-member-u.json
> qapi-schema += reserved-type-kind.json
> qapi-schema += reserved-type-list.json
> qapi-schema += returns-alternate.json
> diff --git a/tests/qapi-schema/reserved-member-u.err
> b/tests/qapi-schema/reserved-member-u.err
> new file mode 100644
> index 0000000..87d4229
> --- /dev/null
> +++ b/tests/qapi-schema/reserved-member-u.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/reserved-member-u.json:7: Member of 'data' for struct
> 'Oops' uses reserved name 'u'
> diff --git a/tests/qapi-schema/reserved-member-u.exit
> b/tests/qapi-schema/reserved-member-u.exit
> new file mode 100644
> index 0000000..d00491f
> --- /dev/null
> +++ b/tests/qapi-schema/reserved-member-u.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/reserved-member-u.json
> b/tests/qapi-schema/reserved-member-u.json
> new file mode 100644
> index 0000000..1eaf0f3
> --- /dev/null
> +++ b/tests/qapi-schema/reserved-member-u.json
> @@ -0,0 +1,7 @@
> +# Potential C member name collision
> +# We reject use of 'u' as a member name, to allow it for internal use in
> +# putting union branch members in a separate namespace from QMP members.
> +# This is true even for non-unions, because it is possible to convert a
> +# struct to flat union while remaining backwards compatible in QMP.
> +# TODO - we could munge the member name to 'q_u' to avoid the collision
> +{ 'struct': 'Oops', 'data': { 'u': 'str' } }
> diff --git a/tests/qapi-schema/reserved-member-u.out
> b/tests/qapi-schema/reserved-member-u.out
> new file mode 100644
> index 0000000..e69de29
