[Qemu-devel] [PATCH v7 06/14] qapi: Update tests related to QMP/branch collisions

[Eric Blake <eblake@redhat.com>]

Now that branches are in a separate C namespace, we can remove
the restrictions in the parser that claim a branch name would
collide with QMP, delete the negative tests that are no longer
problematic, and add positive tests to qapi-schema-test to
ensure things compile correctly.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v7: new patch
---
 scripts/qapi.py                                | 15 +--------------
 tests/Makefile                                 |  3 ---
 tests/qapi-schema/flat-union-clash-branch.err  |  1 -
 tests/qapi-schema/flat-union-clash-branch.exit |  1 -
 tests/qapi-schema/flat-union-clash-branch.json | 16 ----------------
 tests/qapi-schema/flat-union-clash-branch.out  |  0
 tests/qapi-schema/flat-union-clash-type.err    |  1 -
 tests/qapi-schema/flat-union-clash-type.exit   |  1 -
 tests/qapi-schema/flat-union-clash-type.json   | 14 --------------
 tests/qapi-schema/flat-union-clash-type.out    |  0
 tests/qapi-schema/qapi-schema-test.json        | 12 +++++++++---
 tests/qapi-schema/qapi-schema-test.out         | 12 +++++++++++-
 tests/qapi-schema/union-clash-type.err         |  1 -
 tests/qapi-schema/union-clash-type.exit        |  1 -
 tests/qapi-schema/union-clash-type.json        |  7 -------
 tests/qapi-schema/union-clash-type.out         |  0
 16 files changed, 21 insertions(+), 64 deletions(-)
 delete mode 100644 tests/qapi-schema/flat-union-clash-branch.err
 delete mode 100644 tests/qapi-schema/flat-union-clash-branch.exit
 delete mode 100644 tests/qapi-schema/flat-union-clash-branch.json
 delete mode 100644 tests/qapi-schema/flat-union-clash-branch.out
 delete mode 100644 tests/qapi-schema/flat-union-clash-type.err
 delete mode 100644 tests/qapi-schema/flat-union-clash-type.exit
 delete mode 100644 tests/qapi-schema/flat-union-clash-type.json
 delete mode 100644 tests/qapi-schema/flat-union-clash-type.out
 delete mode 100644 tests/qapi-schema/union-clash-type.err
 delete mode 100644 tests/qapi-schema/union-clash-type.exit
 delete mode 100644 tests/qapi-schema/union-clash-type.json
 delete mode 100644 tests/qapi-schema/union-clash-type.out

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 098ba5d..7aa451e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -546,7 +546,7 @@ def check_union(expr, expr_info):
     base = expr.get('base')
     discriminator = expr.get('discriminator')
     members = expr['data']
-    values = {'MAX': '(automatic)', 'TYPE': '(automatic tag)'}
+    values = {'MAX': '(automatic)'}

     # Two types of unions, determined by discriminator.

@@ -592,14 +592,6 @@ def check_union(expr, expr_info):
     # Check every branch
     for (key, value) in members.items():
         check_name(expr_info, "Member of union '%s'" % name, key)
-        # TODO: As long as branch names can collide with QMP names, we
-        # must prevent branches starting with 'has_'. However, we do not
-        # need to reject 'u', because that is reserved for when we start
-        # sticking branch names in a C union named 'u'.
-        if key.startswith('has-') or key.startswith('has_'):
-            raise QAPIExprError(expr_info,
-                                "Branch of union '%s' uses reserved name '%s'"
-                                % (name, key))

         # Each value must name a known type; furthermore, in flat unions,
         # branches must be a struct with no overlapping member names
@@ -620,11 +612,6 @@ def check_union(expr, expr_info):
                                     "Discriminator value '%s' is not found in "
                                     "enum '%s'" %
                                     (key, enum_define["enum_name"]))
-            if discriminator in enum_define['enum_values']:
-                raise QAPIExprError(expr_info,
-                                    "Discriminator name '%s' collides with "
-                                    "enum value in '%s'" %
-                                    (discriminator, enum_define["enum_name"]))

         # Otherwise, check for conflicts in the generated enum
         else:
diff --git a/tests/Makefile b/tests/Makefile
index b3516ad..8c1843a 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -276,9 +276,7 @@ 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-clash-branch.json
 qapi-schema += flat-union-clash-member.json
-qapi-schema += flat-union-clash-type.json
 qapi-schema += flat-union-empty.json
 qapi-schema += flat-union-inline.json
 qapi-schema += flat-union-int-branch.json
@@ -336,7 +334,6 @@ 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-empty.json
 qapi-schema += union-invalid-base.json
 qapi-schema += union-max.json
diff --git a/tests/qapi-schema/flat-union-clash-branch.err b/tests/qapi-schema/flat-union-clash-branch.err
deleted file mode 100644
index e6b6294..0000000
--- a/tests/qapi-schema/flat-union-clash-branch.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/flat-union-clash-branch.json:13: Branch of union 'TestUnion' uses reserved name 'has-a'
diff --git a/tests/qapi-schema/flat-union-clash-branch.exit b/tests/qapi-schema/flat-union-clash-branch.exit
deleted file mode 100644
index d00491f..0000000
--- a/tests/qapi-schema/flat-union-clash-branch.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/flat-union-clash-branch.json b/tests/qapi-schema/flat-union-clash-branch.json
deleted file mode 100644
index 8efbcfd..0000000
--- a/tests/qapi-schema/flat-union-clash-branch.json
+++ /dev/null
@@ -1,16 +0,0 @@
-# Flat union branch name collision
-# This is rejected because the C struct would have duplicate 'has_a'
-# (one as the implicit flag for the optional base member, the other from
-# the C member for the branch name).
-# TODO: we should munge generated branch names to not collide with the
-# non-variant struct members.
-{ 'enum': 'TestEnum',
-  'data': [ 'has-a' ] }
-{ 'struct': 'Base',
-  'data': { 'enum1': 'TestEnum', '*a': 'str' } }
-{ 'struct': 'Branch1',
-  'data': { 'string': 'str' } }
-{ 'union': 'TestUnion',
-  'base': 'Base',
-  'discriminator': 'enum1',
-  'data': { 'has-a': 'Branch1' } }
diff --git a/tests/qapi-schema/flat-union-clash-branch.out b/tests/qapi-schema/flat-union-clash-branch.out
deleted file mode 100644
index e69de29..0000000
diff --git a/tests/qapi-schema/flat-union-clash-type.err b/tests/qapi-schema/flat-union-clash-type.err
deleted file mode 100644
index b44dd40..0000000
--- a/tests/qapi-schema/flat-union-clash-type.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/flat-union-clash-type.json:11: Discriminator name 'type' collides with enum value in 'TestEnum'
diff --git a/tests/qapi-schema/flat-union-clash-type.exit b/tests/qapi-schema/flat-union-clash-type.exit
deleted file mode 100644
index d00491f..0000000
--- a/tests/qapi-schema/flat-union-clash-type.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/flat-union-clash-type.json b/tests/qapi-schema/flat-union-clash-type.json
deleted file mode 100644
index 8f710f0..0000000
--- a/tests/qapi-schema/flat-union-clash-type.json
+++ /dev/null
@@ -1,14 +0,0 @@
-# Flat union branch 'type'
-# Reject this, because we would have a clash in generated C, between the
-# outer tag 'type' and the branch name 'type' within the union.
-# TODO: We could munge the generated C branch name 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
deleted file mode 100644
index e69de29..0000000
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index 926bd7e..26a5b76 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -116,10 +116,16 @@
 # Even though 'u' and 'has_*' are forbidden as struct member names, they
 # should still be valid as a type or union branch name. And although
 # '*Kind' and '*List' are forbidden as type names, they should not be
-# forbidden as a member or branch name.
-{ 'struct': 'has_a', 'data': { 'MyKind': 'int', 'MyList': ['int'] } }
+# forbidden as a member or branch name.  Flat union branches do not
+# collide with base members.
+{ 'enum': 'EnumName', 'data': [ 'value1', 'has_a', 'u', 'type' ] }
+{ 'struct': 'has_a', 'data': { 'MyKind': 'int', 'MyList': ['int'],
+                               'value1': 'EnumName' } }
 { 'union': 'u', 'data': { 'u': 'uint8', 'myKind': 'has_a',
-                          'myList': 'has_a' } }
+                          'myList': 'has_a', 'has_a': 'has_a' } }
+{ 'union': 'UnionName', 'base': 'has_a', 'discriminator': 'value1',
+  'data': { 'value1': 'UserDefZero', 'has_a': 'UserDefZero',
+            'u': 'UserDefZero', 'type': 'UserDefZero' } }
 { 'alternate': 'AltName', 'data': { 'type': 'int', 'u': 'bool',
                                     'myKind': 'has_a' } }

diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 1c39a2a..719bdf1 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -90,6 +90,7 @@ event EVENT_A None
 event EVENT_B None
 event EVENT_C :obj-EVENT_C-arg
 event EVENT_D :obj-EVENT_D-arg
+enum EnumName ['value1', 'has_a', 'u', 'type']
 enum EnumOne ['value1', 'value2', 'value3']
 object EventStructOne
     member struct1: UserDefOne optional=False
@@ -111,6 +112,13 @@ object TestStruct
     member integer: int optional=False
     member boolean: bool optional=False
     member string: str optional=False
+object UnionName
+    base has_a
+    tag value1
+    case value1: UserDefZero
+    case has_a: UserDefZero
+    case u: UserDefZero
+    case type: UserDefZero
 object UserDefA
     member boolean: bool optional=False
     member a_b: int optional=True
@@ -211,11 +219,13 @@ command guest-sync :obj-guest-sync-arg -> any
 object has_a
     member MyKind: int optional=False
     member MyList: intList optional=False
+    member value1: EnumName optional=False
 object u
     case u: :obj-uint8-wrapper
     case myKind: :obj-has_a-wrapper
     case myList: :obj-has_a-wrapper
-enum uKind ['u', 'myKind', 'myList']
+    case has_a: :obj-has_a-wrapper
+enum uKind ['u', 'myKind', 'myList', 'has_a']
 command user_def_cmd None -> None
    gen=True success_response=True
 command user_def_cmd1 :obj-user_def_cmd1-arg -> None
diff --git a/tests/qapi-schema/union-clash-type.err b/tests/qapi-schema/union-clash-type.err
deleted file mode 100644
index c14bbdd..0000000
--- a/tests/qapi-schema/union-clash-type.err
+++ /dev/null
@@ -1 +0,0 @@
-tests/qapi-schema/union-clash-type.json:6: Union 'TestUnion' member 'type' clashes with '(automatic tag)'
diff --git a/tests/qapi-schema/union-clash-type.exit b/tests/qapi-schema/union-clash-type.exit
deleted file mode 100644
index d00491f..0000000
--- a/tests/qapi-schema/union-clash-type.exit
+++ /dev/null
@@ -1 +0,0 @@
-1
diff --git a/tests/qapi-schema/union-clash-type.json b/tests/qapi-schema/union-clash-type.json
deleted file mode 100644
index 641b2d5..0000000
--- a/tests/qapi-schema/union-clash-type.json
+++ /dev/null
@@ -1,7 +0,0 @@
-# Union branch 'type'
-# Reject this, because we would have a clash in generated C, between the
-# simple union's implicit tag member 'type' and the branch name 'type'
-# within the union.
-# TODO: If desired, we could munge the branch name to allow compilation.
-{ '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
deleted file mode 100644
index e69de29..0000000
-- 
2.4.3