Re: [Qemu-devel] [PATCH v8 17/18] qapi: Add test for alternate branch 'kind' clash

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> Rename alternate-clash to alternate-clash-members, and add a
> new test alternate-clash-type.  While similar to the earlier
> addition of union-clash-type, we have one major difference: a
> future patch will be simplifying alternates to not need an
> implict AlternateKind enum, but we still need to detect the
> collision with the resulting C 'qtype_code type;' tag.

You're alluding to a future change of the generated code from

    struct BlockdevRef {
        BlockdevRefKind kind;
        union { /* union tag is @kind */
            void *data;
            BlockdevOptions *definition;
            char *reference;
        };
    };

to

    struct BlockdevRef {
        qtype_code type;
        union { /* union tag is @type */
            void *data;
            BlockdevOptions *definition; /* QTYPE_QDICT */
            char *reference;             /* QTYPE_QSTRING */
        };
    };

right?

I don't think that affects collision checking at all.  Both before and
after, we have a tag member, and we need to check for collisions with
its name.

> No change to generated code.
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v8: no change
> v7: retitle 10/12 and limit to just testsuite changes
> v6: New patch
> ---
>  tests/Makefile                                                 |  3 ++-
>  tests/qapi-schema/alternate-clash-members.err                  |  1 +
>  .../{alternate-clash.exit => alternate-clash-members.exit}     |  0
>  .../{alternate-clash.json => alternate-clash-members.json}     |  0
>  .../{alternate-clash.out => alternate-clash-members.out}       |  0
>  tests/qapi-schema/alternate-clash-type.err                     |  1 +
>  tests/qapi-schema/alternate-clash-type.exit                    |  1 +
>  tests/qapi-schema/alternate-clash-type.json                    | 10 
> ++++++++++
>  tests/qapi-schema/alternate-clash-type.out                     |  0
>  tests/qapi-schema/alternate-clash.err                          |  1 -
>  10 files changed, 15 insertions(+), 2 deletions(-)
>  create mode 100644 tests/qapi-schema/alternate-clash-members.err
>  rename tests/qapi-schema/{alternate-clash.exit =>
> alternate-clash-members.exit} (100%)
>  rename tests/qapi-schema/{alternate-clash.json =>
> alternate-clash-members.json} (100%)
>  rename tests/qapi-schema/{alternate-clash.out =>
> alternate-clash-members.out} (100%)
>  create mode 100644 tests/qapi-schema/alternate-clash-type.err
>  create mode 100644 tests/qapi-schema/alternate-clash-type.exit
>  create mode 100644 tests/qapi-schema/alternate-clash-type.json
>  create mode 100644 tests/qapi-schema/alternate-clash-type.out
>  delete mode 100644 tests/qapi-schema/alternate-clash.err
>
> diff --git a/tests/Makefile b/tests/Makefile
> index 2cd5d31..443e345 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -226,7 +226,8 @@ check-qtest-generic-y += tests/qom-test$(EXESUF)
>
>  qapi-schema += alternate-array.json
>  qapi-schema += alternate-base.json
> -qapi-schema += alternate-clash.json
> +qapi-schema += alternate-clash-members.json
> +qapi-schema += alternate-clash-type.json
>  qapi-schema += alternate-conflict-dict.json
>  qapi-schema += alternate-conflict-string.json
>  qapi-schema += alternate-empty.json
> diff --git a/tests/qapi-schema/alternate-clash-members.err
> b/tests/qapi-schema/alternate-clash-members.err
> new file mode 100644
> index 0000000..0adf737
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-clash-members.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/alternate-clash-members.json:7: Alternate 'Alt1'
> branch 'a_b' clashes with 'a-b'
> diff --git a/tests/qapi-schema/alternate-clash.exit
> b/tests/qapi-schema/alternate-clash-members.exit
> similarity index 100%
> rename from tests/qapi-schema/alternate-clash.exit
> rename to tests/qapi-schema/alternate-clash-members.exit
> diff --git a/tests/qapi-schema/alternate-clash.json
> b/tests/qapi-schema/alternate-clash-members.json
> similarity index 100%
> rename from tests/qapi-schema/alternate-clash.json
> rename to tests/qapi-schema/alternate-clash-members.json
> diff --git a/tests/qapi-schema/alternate-clash.out
> b/tests/qapi-schema/alternate-clash-members.out
> similarity index 100%
> rename from tests/qapi-schema/alternate-clash.out
> rename to tests/qapi-schema/alternate-clash-members.out
> diff --git a/tests/qapi-schema/alternate-clash-type.err
> b/tests/qapi-schema/alternate-clash-type.err
> new file mode 100644
> index 0000000..cdd2090
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-clash-type.err
> @@ -0,0 +1 @@
> +tests/qapi-schema/alternate-clash-type.json:9: 'kind' (branch of
> Alt1) collides with 'kind' (implicit tag of Alt1)
> diff --git a/tests/qapi-schema/alternate-clash-type.exit
> b/tests/qapi-schema/alternate-clash-type.exit
> new file mode 100644
> index 0000000..d00491f
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-clash-type.exit
> @@ -0,0 +1 @@
> +1
> diff --git a/tests/qapi-schema/alternate-clash-type.json
> b/tests/qapi-schema/alternate-clash-type.json
> new file mode 100644
> index 0000000..629584b
> --- /dev/null
> +++ b/tests/qapi-schema/alternate-clash-type.json
> @@ -0,0 +1,10 @@
> +# Alternate branch 'type'
> +# Reject this, because we would have a clash in generated C, between the
> +# alternate's implicit tag member 'kind' and the branch name 'kind'
> +# within the alternate.
> +# TODO: Even if alternates are simplified in the future to use a simpler
> +# 'qtype_code type' tag, rather than a full QAPISchemaObjectTypeMember,
> +# we must still flag the collision, or else munge the generated C branch
> +# names to allow compilation.

I don't think there's a TODO here.

> +{ 'alternate': 'Alt1',
> +  'data': { 'kind': 'str', 'type': 'int' } }
> diff --git a/tests/qapi-schema/alternate-clash-type.out
> b/tests/qapi-schema/alternate-clash-type.out
> new file mode 100644
> index 0000000..e69de29
> diff --git a/tests/qapi-schema/alternate-clash.err
> b/tests/qapi-schema/alternate-clash.err
> deleted file mode 100644
> index 7fd3069..0000000
> --- a/tests/qapi-schema/alternate-clash.err
> +++ /dev/null
> @@ -1 +0,0 @@
> -tests/qapi-schema/alternate-clash.json:7: Alternate 'Alt1' branch
> 'a_b' clashes with 'a-b'