Eric Blake <ebl...@redhat.com> writes: > Expose some weaknesses in the generator: we don't always forbid > the generation of structs that contain multiple members that map > to the same C or QMP name. This has already been marked FIXME in > qapi.py in commit d90675f, but having more tests will make sure > future patches produce desired behavior; and updating existing > patches to better document things doesn't hurt, either. Some of > these collisions are already caught in the old-style parser > checks, but ultimately we want all collisions to be caught in the > new-style QAPISchema*.check() methods. > > This patch focuses on C struct members, and does not consider > collisions between commands and events (affecting C function > names), or even collisions between generated C type names with > user type names (for things like automatic FOOList struct > representing array types or FOOKind for an implicit enum). > > There are two types of struct collisions we want to catch: > 1) Collision between two keys in a JSON object. qapi.py prevents > that within a single struct (see duplicate-key), but it is
"(see test duplicate-key)" or maybe "(see test duplicate-key.json)". > possible to have collisions between a type's members and its > base type's members (struct-base-clash, struct-base-clash-deep, > flat-union-cycle), and its flat union variant members (existing tests struct-base-clash, struct-base-clash-deep, new test flat-union-cycle) > (flat-union-clash-member). (new test flat-union-clash-member) > 2) Collision between two members of the C struct that is generated > for a given QAPI type: > a) Multiple QAPI names map to the same C name (args-name-clash) (new test args-name-clash) > b) A QAPI name maps to a C name that is used for another purpose > (flat-union-clash-branch, struct-base-clash-base, > union-clash-data). We already fixed some such cases in commit (new tests ...) > 0f61af3e and 1e6c1616, but more remain. > c) Two C names generated for other purposes clash > (alternate-clash, union-clash-branches, union-clash-type, > flat-union-clash-type) (updated test alternate-clash, new tests ...) > > Ultimately, if we need to have a flat union where an enum value Suggest "where a tag value", to make it clear it's not just any enum. > clashes with a base member name, we could change the generator to > name the union (using 'foo.u.value' rather than 'foo.value') or > otherwise munge the C name corresponding to enum tag values. But Suggest to drop enum here. > unless such a need arises, it will probably be easier to just > forbid these collisions. > > Some of these negative tests will be deleted later, and positive > tests added to qapi-schema-test.json in their place, when the > generator code is reworked to avoid particular code generation > collisions in class 2). > > [Note that viewing this patch with git rename detection enabled > may see some confusion due to renaming some tests while adding > others, but where the content is similar enough that git picks > the wrong pre- and post-patch files to associate] > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- > v7: rework commit message again, even more comments in tests, yet > another rename (union-clash-members is now union-clash-branches) > v6: rework commit message, alter test names and comments to be > more descriptive, repurpose flat-union-cycle to better match its > name (and not duplicate what the renamed flat-union-clash-branch > tests), add new tests (union-clash-type, flat-union-clash-type) > that detect an assertion failures [...] > diff --git a/tests/qapi-schema/alternate-clash.err > b/tests/qapi-schema/alternate-clash.err > index 51bea3e..a475ab6 100644 > --- a/tests/qapi-schema/alternate-clash.err > +++ b/tests/qapi-schema/alternate-clash.err > @@ -1 +1 @@ > -tests/qapi-schema/alternate-clash.json:2: Alternate 'Alt1' member 'ONE' > clashes with 'one' > +tests/qapi-schema/alternate-clash.json:7: Alternate 'Alt1' member 'a_b' > clashes with 'a-b' > diff --git a/tests/qapi-schema/alternate-clash.json > b/tests/qapi-schema/alternate-clash.json > index 3947935..6d73bc5 100644 > --- a/tests/qapi-schema/alternate-clash.json > +++ b/tests/qapi-schema/alternate-clash.json > @@ -1,3 +1,8 @@ > -# we detect C enum collisions in an alternate > +# Alternate branch name collision > +# Reject an alternate that would result in a collision in generated C > +# names (this would try to generate two enum values 'ALT1_KIND_A_B'). > +# TODO: In the future, if alternates are simplified to not generate > +# the implicit Alt1Kind enum, we would still have a collision with the > +# resulting C union trying to have two members named 'a_b'. > { 'alternate': 'Alt1', > - 'data': { 'one': 'str', 'ONE': 'int' } } > + 'data': { 'a-b': 'str', 'a_b': 'int' } } Ah, you're making the test slightly more robust. Works for me. > diff --git a/tests/qapi-schema/flat-union-branch-clash.out > b/tests/qapi-schema/args-name-clash.err > similarity index 100% > rename from tests/qapi-schema/flat-union-branch-clash.out > rename to tests/qapi-schema/args-name-clash.err > diff --git a/tests/qapi-schema/args-name-clash.exit > b/tests/qapi-schema/args-name-clash.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/args-name-clash.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/args-name-clash.json > b/tests/qapi-schema/args-name-clash.json > new file mode 100644 > index 0000000..9e8f889 > --- /dev/null > +++ b/tests/qapi-schema/args-name-clash.json > @@ -0,0 +1,5 @@ > +# C member name collision > +# FIXME - This parses, but fails to compile, because the C struct is given > +# two 'a_b' members. Either reject this at parse time, or munge the C names > +# to avoid the collision. > +{ 'command': 'oops', 'data': { 'a-b': 'str', 'a_b': 'str' } } > diff --git a/tests/qapi-schema/args-name-clash.out > b/tests/qapi-schema/args-name-clash.out > new file mode 100644 > index 0000000..9b2f6e4 > --- /dev/null > +++ b/tests/qapi-schema/args-name-clash.out > @@ -0,0 +1,6 @@ > +object :empty > +object :obj-oops-arg > + member a-b: str optional=False > + member a_b: str optional=False > +command oops :obj-oops-arg -> None > + gen=True success_response=True > diff --git a/tests/qapi-schema/duplicate-key.err > b/tests/qapi-schema/duplicate-key.err > index 768b276..6d02f83 100644 > --- a/tests/qapi-schema/duplicate-key.err > +++ b/tests/qapi-schema/duplicate-key.err > @@ -1 +1 @@ > -tests/qapi-schema/duplicate-key.json:2:10: Duplicate key "key" > +tests/qapi-schema/duplicate-key.json:3:10: Duplicate key "key" > diff --git a/tests/qapi-schema/duplicate-key.json > b/tests/qapi-schema/duplicate-key.json > index 1b55d88..14ac0e8 100644 > --- a/tests/qapi-schema/duplicate-key.json > +++ b/tests/qapi-schema/duplicate-key.json > @@ -1,2 +1,3 @@ > +# QAPI cannot include the same key more than once in any {} > { 'key': 'value', > 'key': 'value' } > diff --git a/tests/qapi-schema/flat-union-base-union.err > b/tests/qapi-schema/flat-union-base-union.err > index ede9859..28725ed 100644 > --- a/tests/qapi-schema/flat-union-base-union.err > +++ b/tests/qapi-schema/flat-union-base-union.err > @@ -1 +1 @@ > -tests/qapi-schema/flat-union-base-union.json:11: Base 'UnionBase' is not a > valid struct > +tests/qapi-schema/flat-union-base-union.json:14: Base 'UnionBase' is not a > valid struct > diff --git a/tests/qapi-schema/flat-union-base-union.json > b/tests/qapi-schema/flat-union-base-union.json > index 6a8ea68..98b4eba 100644 > --- a/tests/qapi-schema/flat-union-base-union.json > +++ b/tests/qapi-schema/flat-union-base-union.json > @@ -1,4 +1,7 @@ > -# we require the base to be a struct > +# For now, we require the base to be a struct without variants > +# TODO: It would be possible to allow a union as a base, as long as all > +# permutations of QMP names exposed by base do not clash with any QMP > +# member names added by local variants. > { 'enum': 'TestEnum', > 'data': [ 'value1', 'value2' ] } > { 'struct': 'TestTypeA', > diff --git a/tests/qapi-schema/flat-union-branch-clash.err > b/tests/qapi-schema/flat-union-branch-clash.err > deleted file mode 100644 > index f112766..0000000 > --- a/tests/qapi-schema/flat-union-branch-clash.err > +++ /dev/null > @@ -1 +0,0 @@ > -tests/qapi-schema/flat-union-branch-clash.json:10: Member name 'name' of > branch 'value1' clashes with base 'Base' > diff --git a/tests/qapi-schema/flat-union-clash-branch.err > b/tests/qapi-schema/flat-union-clash-branch.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/flat-union-clash-branch.exit > b/tests/qapi-schema/flat-union-clash-branch.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/flat-union-clash-branch.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/flat-union-clash-branch.json > b/tests/qapi-schema/flat-union-clash-branch.json > new file mode 100644 > index 0000000..e593336 > --- /dev/null > +++ b/tests/qapi-schema/flat-union-clash-branch.json > @@ -0,0 +1,18 @@ > +# Flat union branch name collision > +# FIXME: this parses, but then fails to compile due to a duplicate 'c_d' > +# (one from the base member, the other from the branch name). We should > +# either reject the collision at parse time, or munge the generated branch > +# name to allow this to compile. > +{ 'enum': 'TestEnum', > + 'data': [ 'base', 'c-d' ] } > +{ 'struct': 'Base', > + 'data': { 'enum1': 'TestEnum', '*c_d': 'str' } } > +{ 'struct': 'Branch1', > + 'data': { 'string': 'str' } } > +{ 'struct': 'Branch2', > + 'data': { 'value': 'int' } } > +{ 'union': 'TestUnion', > + 'base': 'Base', > + 'discriminator': 'enum1', > + 'data': { 'base': 'Branch1', > + 'c-d': 'Branch2' } } > diff --git a/tests/qapi-schema/flat-union-clash-branch.out > b/tests/qapi-schema/flat-union-clash-branch.out > new file mode 100644 > index 0000000..8e0da73 > --- /dev/null > +++ b/tests/qapi-schema/flat-union-clash-branch.out > @@ -0,0 +1,14 @@ > +object :empty > +object Base > + member enum1: TestEnum optional=False > + member c_d: str optional=True > +object Branch1 > + member string: str optional=False > +object Branch2 > + member value: int optional=False > +enum TestEnum ['base', 'c-d'] > +object TestUnion > + base Base > + tag enum1 > + case base: Branch1 > + case c-d: Branch2 > diff --git a/tests/qapi-schema/flat-union-clash-member.err > b/tests/qapi-schema/flat-union-clash-member.err > new file mode 100644 > index 0000000..152d402 > --- /dev/null > +++ b/tests/qapi-schema/flat-union-clash-member.err > @@ -0,0 +1 @@ > +tests/qapi-schema/flat-union-clash-member.json:10: Member name 'name' of > branch 'value1' clashes with base 'Base' > diff --git a/tests/qapi-schema/flat-union-branch-clash.exit > b/tests/qapi-schema/flat-union-clash-member.exit > similarity index 100% > rename from tests/qapi-schema/flat-union-branch-clash.exit > rename to tests/qapi-schema/flat-union-clash-member.exit > diff --git a/tests/qapi-schema/flat-union-branch-clash.json > b/tests/qapi-schema/flat-union-clash-member.json > similarity index 85% > rename from tests/qapi-schema/flat-union-branch-clash.json > rename to tests/qapi-schema/flat-union-clash-member.json > index 8fb054f..3d7e6c6 100644 > --- a/tests/qapi-schema/flat-union-branch-clash.json > +++ b/tests/qapi-schema/flat-union-clash-member.json > @@ -1,4 +1,4 @@ > -# we check for no duplicate keys between branches and base > +# we check for no duplicate keys between branch members and base Spelling out the clashing members wouldn't hurt. > { 'enum': 'TestEnum', > 'data': [ 'value1', 'value2' ] } > { 'struct': 'Base', > diff --git a/tests/qapi-schema/flat-union-clash-member.out > b/tests/qapi-schema/flat-union-clash-member.out > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/flat-union-clash-type.err > b/tests/qapi-schema/flat-union-clash-type.err > new file mode 100644 > index 0000000..6e64d1d > --- /dev/null > +++ b/tests/qapi-schema/flat-union-clash-type.err > @@ -0,0 +1,16 @@ > +Traceback (most recent call last): > + File "tests/qapi-schema/test-qapi.py", line 55, in <module> > + schema = QAPISchema(sys.argv[1]) > + File "scripts/qapi.py", line 1116, in __init__ > + self.check() > + File "scripts/qapi.py", line 1299, in check > + ent.check(self) > + File "scripts/qapi.py", line 962, in check > + self.variants.check(schema, members, seen) > + File "scripts/qapi.py", line 1024, in check > + v.check(schema, self.tag_member.type, vseen) > + File "scripts/qapi.py", line 1032, in check > + QAPISchemaObjectTypeMember.check(self, schema, [], seen) > + File "scripts/qapi.py", line 994, in check > + assert self.name not in seen > +AssertionError > diff --git a/tests/qapi-schema/flat-union-clash-type.exit > b/tests/qapi-schema/flat-union-clash-type.exit > new file mode 100644 > index 0000000..d00491f > --- /dev/null > +++ b/tests/qapi-schema/flat-union-clash-type.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/flat-union-clash-type.json > b/tests/qapi-schema/flat-union-clash-type.json > new file mode 100644 > index 0000000..eac51a4 > --- /dev/null > +++ b/tests/qapi-schema/flat-union-clash-type.json > @@ -0,0 +1,15 @@ > +# Flat union branch 'type' > +# FIXME: this triggers an assertion failure. But even with that fixed, we > +# would have a clash in generated C, between the outer tag 'type' and "outer tag"? I guess you mean the member type we inherit from Base. > +# the branch name 'type' within the union. We should either reject this, > +# or munge the generated C to let it compile. > +{ 'enum': 'TestEnum', > + 'data': [ 'type' ] } > +{ 'struct': 'Base', > + 'data': { 'type': 'TestEnum' } } > +{ 'struct': 'Branch1', > + 'data': { 'string': 'str' } } > +{ 'union': 'TestUnion', > + 'base': 'Base', > + 'discriminator': 'type', > + 'data': { 'type': 'Branch1' } } > diff --git a/tests/qapi-schema/flat-union-clash-type.out > b/tests/qapi-schema/flat-union-clash-type.out > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/flat-union-cycle.err > b/tests/qapi-schema/flat-union-cycle.err > new file mode 100644 > index 0000000..9b7978a > --- /dev/null > +++ b/tests/qapi-schema/flat-union-cycle.err > @@ -0,0 +1 @@ > +tests/qapi-schema/flat-union-cycle.json:7: Base 'Union' is not a valid struct > diff --git a/tests/qapi-schema/flat-union-cycle.exit > b/tests/qapi-schema/flat-union-cycle.exit > new file mode 100644 > index 0000000..d00491f > --- /dev/null > +++ b/tests/qapi-schema/flat-union-cycle.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/flat-union-cycle.json > b/tests/qapi-schema/flat-union-cycle.json > new file mode 100644 > index 0000000..c66c4c9 > --- /dev/null > +++ b/tests/qapi-schema/flat-union-cycle.json > @@ -0,0 +1,8 @@ > +# Ensure that we have a sane error message for attempts at self-inheritance > +# This test currently fails because we don't permit a union base, but > +# even if we relax things to allow that in the future (see > +# flat-union-base-union), self-inheritance should still fail. Do we have a test for the simpler case of a struct inheriting from itself? I believe us merging struct and union types into a single object type is more likely than us implementing union bases. If I'm correct, we won't need this test. > +{ 'enum': 'Enum', 'data': [ 'okay' ] } > +{ 'struct': 'Okay', 'data': { 'int': 'int' } } > +{ 'union': 'Union', 'base': 'Union', 'discriminator': 'switch', > + 'data': { 'okay': 'Okay' } } > diff --git a/tests/qapi-schema/flat-union-cycle.out > b/tests/qapi-schema/flat-union-cycle.out > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/qapi-schema-test.json > b/tests/qapi-schema/qapi-schema-test.json > index 6897a6e..c511338 100644 > --- a/tests/qapi-schema/qapi-schema-test.json > +++ b/tests/qapi-schema/qapi-schema-test.json > @@ -32,11 +32,14 @@ > 'dict1': 'UserDefTwoDict' } } > > # for testing unions > +# Among other things, test that a name collision between branches does > +# not cause any problems (since only one branch can be in use at a time), > +# by intentionally using two branches that both have a C member 'a_b' > { 'struct': 'UserDefA', > - 'data': { 'boolean': 'bool' } } > + 'data': { 'boolean': 'bool', '*a_b': 'int' } } > > { 'struct': 'UserDefB', > - 'data': { 'intb': 'int' } } > + 'data': { 'intb': 'int', '*a-b': 'bool' } } > > { 'union': 'UserDefFlatUnion', > 'base': 'UserDefUnionBase', # intentional forward reference > diff --git a/tests/qapi-schema/qapi-schema-test.out > b/tests/qapi-schema/qapi-schema-test.out > index 1f6e858..28a0b3c 100644 > --- a/tests/qapi-schema/qapi-schema-test.out > +++ b/tests/qapi-schema/qapi-schema-test.out > @@ -71,6 +71,7 @@ enum QEnumTwo ['value1', 'value2'] > prefix QENUM_TWO > object UserDefA > member boolean: bool optional=False > + member a_b: int optional=True > alternate UserDefAlternate > case uda: UserDefA > case s: str > @@ -78,6 +79,7 @@ alternate UserDefAlternate > enum UserDefAlternateKind ['uda', 's', 'i'] > object UserDefB > member intb: int optional=False > + member a-b: bool optional=True > object UserDefC > member string1: str optional=False > member string2: str optional=False > diff --git a/tests/qapi-schema/struct-base-clash-base.err > b/tests/qapi-schema/struct-base-clash-base.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/struct-base-clash-base.exit > b/tests/qapi-schema/struct-base-clash-base.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/struct-base-clash-base.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/struct-base-clash-base.json > b/tests/qapi-schema/struct-base-clash-base.json > new file mode 100644 > index 0000000..0c84025 > --- /dev/null > +++ b/tests/qapi-schema/struct-base-clash-base.json > @@ -0,0 +1,9 @@ > +# Struct member 'base' > +# FIXME: this parses, but then fails to compile due to a duplicate 'base' > +# (one explicit in QMP, the other used to box the base class members). > +# We should either reject the collision at parse time, or change the > +# generated struct to allow this to compile. > +{ 'struct': 'Base', 'data': {} } > +{ 'struct': 'Sub', > + 'base': 'Base', > + 'data': { 'base': 'str' } } > diff --git a/tests/qapi-schema/struct-base-clash-base.out > b/tests/qapi-schema/struct-base-clash-base.out > new file mode 100644 > index 0000000..e69a416 > --- /dev/null > +++ b/tests/qapi-schema/struct-base-clash-base.out > @@ -0,0 +1,5 @@ > +object :empty > +object Base > +object Sub > + base Base > + member base: str optional=False > diff --git a/tests/qapi-schema/struct-base-clash-deep.err > b/tests/qapi-schema/struct-base-clash-deep.err > index e3e9f8d..f7a25a3 100644 > --- a/tests/qapi-schema/struct-base-clash-deep.err > +++ b/tests/qapi-schema/struct-base-clash-deep.err > @@ -1 +1 @@ > -tests/qapi-schema/struct-base-clash-deep.json:7: Member name 'name' clashes > with base 'Base' > +tests/qapi-schema/struct-base-clash-deep.json:10: Member name 'name' clashes > with base 'Base' > diff --git a/tests/qapi-schema/struct-base-clash-deep.json > b/tests/qapi-schema/struct-base-clash-deep.json > index 552fe94..fa873ab 100644 > --- a/tests/qapi-schema/struct-base-clash-deep.json > +++ b/tests/qapi-schema/struct-base-clash-deep.json > @@ -1,4 +1,7 @@ > -# we check for no duplicate keys with indirect base > +# Reject attempts to duplicate QMP members > +# Here, 'name' would have to appear twice on the wire, locally and > +# indirectly for the grandparent base; the collision doesn't care that > +# one instance is optional. > { 'struct': 'Base', > 'data': { 'name': 'str' } } > { 'struct': 'Mid', > diff --git a/tests/qapi-schema/struct-base-clash.err > b/tests/qapi-schema/struct-base-clash.err > index 3ac37fb..3a9f66b 100644 > --- a/tests/qapi-schema/struct-base-clash.err > +++ b/tests/qapi-schema/struct-base-clash.err > @@ -1 +1 @@ > -tests/qapi-schema/struct-base-clash.json:4: Member name 'name' clashes with > base 'Base' > +tests/qapi-schema/struct-base-clash.json:5: Member name 'name' clashes with > base 'Base' > diff --git a/tests/qapi-schema/struct-base-clash.json > b/tests/qapi-schema/struct-base-clash.json > index f2afc9b..11aec80 100644 > --- a/tests/qapi-schema/struct-base-clash.json > +++ b/tests/qapi-schema/struct-base-clash.json > @@ -1,4 +1,5 @@ > -# we check for no duplicate keys with base > +# Reject attempts to duplicate QMP members > +# Here, 'name' would have to appear twice on the wire, locally and for base. > { 'struct': 'Base', > 'data': { 'name': 'str' } } > { 'struct': 'Sub', > diff --git a/tests/qapi-schema/union-clash-branches.err > b/tests/qapi-schema/union-clash-branches.err > new file mode 100644 > index 0000000..005c48d > --- /dev/null > +++ b/tests/qapi-schema/union-clash-branches.err > @@ -0,0 +1 @@ > +tests/qapi-schema/union-clash-branches.json:4: Union 'TestUnion' member > 'a_b' clashes with 'a-b' > diff --git a/tests/qapi-schema/union-clash-branches.exit > b/tests/qapi-schema/union-clash-branches.exit > new file mode 100644 > index 0000000..d00491f > --- /dev/null > +++ b/tests/qapi-schema/union-clash-branches.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/union-clash-branches.json > b/tests/qapi-schema/union-clash-branches.json > new file mode 100644 > index 0000000..31d135f > --- /dev/null > +++ b/tests/qapi-schema/union-clash-branches.json > @@ -0,0 +1,5 @@ > +# Union branch name collision > +# Reject a union that would result in a collision in generated C names (this > +# would try to generate two enum values 'TEST_UNION_KIND_A_B'). > +{ 'union': 'TestUnion', > + 'data': { 'a-b': 'int', 'a_b': 'str' } } > diff --git a/tests/qapi-schema/union-clash-branches.out > b/tests/qapi-schema/union-clash-branches.out > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/union-clash-data.err > b/tests/qapi-schema/union-clash-data.err > new file mode 100644 > index 0000000..e69de29 > diff --git a/tests/qapi-schema/union-clash-data.exit > b/tests/qapi-schema/union-clash-data.exit > new file mode 100644 > index 0000000..573541a > --- /dev/null > +++ b/tests/qapi-schema/union-clash-data.exit > @@ -0,0 +1 @@ > +0 > diff --git a/tests/qapi-schema/union-clash-data.json > b/tests/qapi-schema/union-clash-data.json > new file mode 100644 > index 0000000..7308e69 > --- /dev/null > +++ b/tests/qapi-schema/union-clash-data.json > @@ -0,0 +1,7 @@ > +# Union branch 'data' > +# FIXME: this parses, but then fails to compile due to a duplicate 'data' > +# (one from the branch name, another as a filler to avoid an empty union). > +# we should either detect the collision at parse time, or change the > +# generated struct to allow this to compile. > +{ 'union': 'TestUnion', > + 'data': { 'data': 'int' } } > diff --git a/tests/qapi-schema/union-clash-data.out > b/tests/qapi-schema/union-clash-data.out > new file mode 100644 > index 0000000..6277239 > --- /dev/null > +++ b/tests/qapi-schema/union-clash-data.out > @@ -0,0 +1,6 @@ > +object :empty > +object :obj-int-wrapper > + member data: int optional=False > +object TestUnion > + case data: :obj-int-wrapper > +enum TestUnionKind ['data'] > diff --git a/tests/qapi-schema/union-clash-type.err > b/tests/qapi-schema/union-clash-type.err > new file mode 100644 > index 0000000..6e64d1d > --- /dev/null > +++ b/tests/qapi-schema/union-clash-type.err > @@ -0,0 +1,16 @@ > +Traceback (most recent call last): > + File "tests/qapi-schema/test-qapi.py", line 55, in <module> > + schema = QAPISchema(sys.argv[1]) > + File "scripts/qapi.py", line 1116, in __init__ > + self.check() > + File "scripts/qapi.py", line 1299, in check > + ent.check(self) > + File "scripts/qapi.py", line 962, in check > + self.variants.check(schema, members, seen) > + File "scripts/qapi.py", line 1024, in check > + v.check(schema, self.tag_member.type, vseen) > + File "scripts/qapi.py", line 1032, in check > + QAPISchemaObjectTypeMember.check(self, schema, [], seen) > + File "scripts/qapi.py", line 994, in check > + assert self.name not in seen > +AssertionError > diff --git a/tests/qapi-schema/union-clash-type.exit > b/tests/qapi-schema/union-clash-type.exit > new file mode 100644 > index 0000000..d00491f > --- /dev/null > +++ b/tests/qapi-schema/union-clash-type.exit > @@ -0,0 +1 @@ > +1 > diff --git a/tests/qapi-schema/union-clash-type.json > b/tests/qapi-schema/union-clash-type.json > new file mode 100644 > index 0000000..38330b5 > --- /dev/null > +++ b/tests/qapi-schema/union-clash-type.json > @@ -0,0 +1,9 @@ > +# Union branch 'type' > +# FIXME: this triggers an assertion failure. But even with that fixed, we > +# would have a clash in generated C, between the outer union tag 'kind' and Suggest "the simple union's implicit tag member 'kind' and" > +# the branch name 'kind' within the union. We should either reject this, > +# or munge the generated C to let it compile. > +# TODO: Even when the generated C is switched to use 'type' rather than > +# 'kind', to match the QMP spelling, the collision should still be detected. > +{ 'union': 'TestUnion', > + 'data': { 'kind': 'int', 'type': 'str' } } > diff --git a/tests/qapi-schema/union-clash-type.out > b/tests/qapi-schema/union-clash-type.out > new file mode 100644 > index 0000000..e69de29 Found nothing that couldn't be touched up on commit.