[Qemu-devel] [PATCH v7 05/18] qapi: Test for various name collisions

[Eric Blake]

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
    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
    (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)
    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
       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)

Ultimately, if we need to have a flat union where an enum value
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
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 <address@hidden>

---
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
---
 tests/Makefile                                         | 10 +++++++++-
 tests/qapi-schema/alternate-clash.err                  |  2 +-
 tests/qapi-schema/alternate-clash.json                 |  9 +++++++--
 ...flat-union-branch-clash.out => args-name-clash.err} |  0
 tests/qapi-schema/args-name-clash.exit                 |  1 +
 tests/qapi-schema/args-name-clash.json                 |  5 +++++
 tests/qapi-schema/args-name-clash.out                  |  6 ++++++
 tests/qapi-schema/duplicate-key.err                    |  2 +-
 tests/qapi-schema/duplicate-key.json                   |  1 +
 tests/qapi-schema/flat-union-base-union.err            |  2 +-
 tests/qapi-schema/flat-union-base-union.json           |  5 ++++-
 tests/qapi-schema/flat-union-branch-clash.err          |  1 -
 tests/qapi-schema/flat-union-clash-branch.err          |  0
 tests/qapi-schema/flat-union-clash-branch.exit         |  1 +
 tests/qapi-schema/flat-union-clash-branch.json         | 18 ++++++++++++++++++
 tests/qapi-schema/flat-union-clash-branch.out          | 14 ++++++++++++++
 tests/qapi-schema/flat-union-clash-member.err          |  1 +
 ...-branch-clash.exit => flat-union-clash-member.exit} |  0
 ...-branch-clash.json => flat-union-clash-member.json} |  2 +-
 tests/qapi-schema/flat-union-clash-member.out          |  0
 tests/qapi-schema/flat-union-clash-type.err            | 16 ++++++++++++++++
 tests/qapi-schema/flat-union-clash-type.exit           |  1 +
 tests/qapi-schema/flat-union-clash-type.json           | 15 +++++++++++++++
 tests/qapi-schema/flat-union-clash-type.out            |  0
 tests/qapi-schema/flat-union-cycle.err                 |  1 +
 tests/qapi-schema/flat-union-cycle.exit                |  1 +
 tests/qapi-schema/flat-union-cycle.json                |  8 ++++++++
 tests/qapi-schema/flat-union-cycle.out                 |  0
 tests/qapi-schema/qapi-schema-test.json                |  7 +++++--
 tests/qapi-schema/qapi-schema-test.out                 |  2 ++
 tests/qapi-schema/struct-base-clash-base.err           |  0
 tests/qapi-schema/struct-base-clash-base.exit          |  1 +
 tests/qapi-schema/struct-base-clash-base.json          |  9 +++++++++
 tests/qapi-schema/struct-base-clash-base.out           |  5 +++++
 tests/qapi-schema/struct-base-clash-deep.err           |  2 +-
 tests/qapi-schema/struct-base-clash-deep.json          |  5 ++++-
 tests/qapi-schema/struct-base-clash.err                |  2 +-
 tests/qapi-schema/struct-base-clash.json               |  3 ++-
 tests/qapi-schema/union-clash-branches.err             |  1 +
 tests/qapi-schema/union-clash-branches.exit            |  1 +
 tests/qapi-schema/union-clash-branches.json            |  5 +++++
 tests/qapi-schema/union-clash-branches.out             |  0
 tests/qapi-schema/union-clash-data.err                 |  0
 tests/qapi-schema/union-clash-data.exit                |  1 +
 tests/qapi-schema/union-clash-data.json                |  7 +++++++
 tests/qapi-schema/union-clash-data.out                 |  6 ++++++
 tests/qapi-schema/union-clash-type.err                 | 16 ++++++++++++++++
 tests/qapi-schema/union-clash-type.exit                |  1 +
 tests/qapi-schema/union-clash-type.json                |  9 +++++++++
 tests/qapi-schema/union-clash-type.out                 |  0
 50 files changed, 190 insertions(+), 15 deletions(-)
 rename tests/qapi-schema/{flat-union-branch-clash.out => args-name-clash.err} 
(100%)
 create mode 100644 tests/qapi-schema/args-name-clash.exit
 create mode 100644 tests/qapi-schema/args-name-clash.json
 create mode 100644 tests/qapi-schema/args-name-clash.out
 delete mode 100644 tests/qapi-schema/flat-union-branch-clash.err
 create mode 100644 tests/qapi-schema/flat-union-clash-branch.err
 create mode 100644 tests/qapi-schema/flat-union-clash-branch.exit
 create mode 100644 tests/qapi-schema/flat-union-clash-branch.json
 create mode 100644 tests/qapi-schema/flat-union-clash-branch.out
 create mode 100644 tests/qapi-schema/flat-union-clash-member.err
 rename tests/qapi-schema/{flat-union-branch-clash.exit => 
flat-union-clash-member.exit} (100%)
 rename tests/qapi-schema/{flat-union-branch-clash.json => 
flat-union-clash-member.json} (85%)
 create mode 100644 tests/qapi-schema/flat-union-clash-member.out
 create mode 100644 tests/qapi-schema/flat-union-clash-type.err
 create mode 100644 tests/qapi-schema/flat-union-clash-type.exit
 create mode 100644 tests/qapi-schema/flat-union-clash-type.json
 create mode 100644 tests/qapi-schema/flat-union-clash-type.out
 create mode 100644 tests/qapi-schema/flat-union-cycle.err
 create mode 100644 tests/qapi-schema/flat-union-cycle.exit
 create mode 100644 tests/qapi-schema/flat-union-cycle.json
 create mode 100644 tests/qapi-schema/flat-union-cycle.out
 create mode 100644 tests/qapi-schema/struct-base-clash-base.err
 create mode 100644 tests/qapi-schema/struct-base-clash-base.exit
 create mode 100644 tests/qapi-schema/struct-base-clash-base.json
 create mode 100644 tests/qapi-schema/struct-base-clash-base.out
 create mode 100644 tests/qapi-schema/union-clash-branches.err
 create mode 100644 tests/qapi-schema/union-clash-branches.exit
 create mode 100644 tests/qapi-schema/union-clash-branches.json
 create mode 100644 tests/qapi-schema/union-clash-branches.out
 create mode 100644 tests/qapi-schema/union-clash-data.err
 create mode 100644 tests/qapi-schema/union-clash-data.exit
 create mode 100644 tests/qapi-schema/union-clash-data.json
 create mode 100644 tests/qapi-schema/union-clash-data.out
 create mode 100644 tests/qapi-schema/union-clash-type.err
 create mode 100644 tests/qapi-schema/union-clash-type.exit
 create mode 100644 tests/qapi-schema/union-clash-type.json
 create mode 100644 tests/qapi-schema/union-clash-type.out

diff --git a/tests/Makefile b/tests/Makefile
index 6fd5885..2fdf88a 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -238,6 +238,7 @@ qapi-schema += args-invalid.json
 qapi-schema += args-member-array-bad.json
 qapi-schema += args-member-array.json
 qapi-schema += args-member-unknown.json
+qapi-schema += args-name-clash.json
 qapi-schema += args-union.json
 qapi-schema += args-unknown.json
 qapi-schema += bad-base.json
@@ -273,7 +274,10 @@ qapi-schema += flat-union-bad-base.json
 qapi-schema += flat-union-bad-discriminator.json
 qapi-schema += flat-union-base-any.json
 qapi-schema += flat-union-base-union.json
-qapi-schema += flat-union-branch-clash.json
+qapi-schema += flat-union-clash-branch.json
+qapi-schema += flat-union-clash-member.json
+qapi-schema += flat-union-clash-type.json
+qapi-schema += flat-union-cycle.json
 qapi-schema += flat-union-inline.json
 qapi-schema += flat-union-int-branch.json
 qapi-schema += flat-union-invalid-branch-key.json
@@ -315,6 +319,7 @@ qapi-schema += returns-dict.json
 qapi-schema += returns-int.json
 qapi-schema += returns-unknown.json
 qapi-schema += returns-whitelist.json
+qapi-schema += struct-base-clash-base.json
 qapi-schema += struct-base-clash-deep.json
 qapi-schema += struct-base-clash.json
 qapi-schema += struct-data-invalid.json
@@ -328,6 +333,9 @@ qapi-schema += unclosed-string.json
 qapi-schema += unicode-str.json
 qapi-schema += union-bad-branch.json
 qapi-schema += union-base-no-discriminator.json
+qapi-schema += union-clash-branches.json
+qapi-schema += union-clash-data.json
+qapi-schema += union-clash-type.json
 qapi-schema += union-invalid-base.json
 qapi-schema += union-max.json
 qapi-schema += union-optional-branch.json
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' } }
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
 { '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
+# 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.
+{ '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
+# 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
-- 
2.4.3