[Qemu-devel] [PATCH v6 06/16] qapi: Avoid assertion failure on union 'type' collision

[Eric Blake]

The previous commit added two tests that triggered an assertion
failure. It's fairly straightforward to avoid the failure by
just outright forbidding the collision between a union's tag
values and its discriminator name (including the implicit name
'kind' supplied for simple unions [*]).  Ultimately, we'd like
to move the collision detection into QAPISchema*.check(), but
for now it is easier just to enhance the existing checks.

[*] Of course, down the road, we have plans to rename the simple
union tag name to 'type' to match the QMP wire name, but the
idea of the collision will still be present even then.

Technically, we could avoid the collision by naming the C union
members representing each enum value as '_case_value' rather
than 'value'; but until we have an actual qapi client (and not
just our testsuite) that has a legitimate reason to match a
case label to the name of a QMP key and needs the name munging
to satisfy the compiler, it's easier to just reject the qapi
as invalid.

Signed-off-by: Eric Blake <address@hidden>

---
v6: new patch
---
 scripts/qapi.py                             | 10 ++++++++--
 tests/qapi-schema/flat-union-clash-type.err | 17 +----------------
 tests/qapi-schema/union-clash-type.err      | 17 +----------------
 3 files changed, 10 insertions(+), 34 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 7e8a99d..ec450a6 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -544,7 +544,7 @@ def check_union(expr, expr_info):
     base = expr.get('base')
     discriminator = expr.get('discriminator')
     members = expr['data']
-    values = {'MAX': '(automatic)'}
+    values = {'MAX': '(automatic)', 'KIND': '(automatic)'}

     # Two types of unions, determined by discriminator.

@@ -603,13 +603,19 @@ def check_union(expr, expr_info):
                                " of branch '%s'" % key)

         # If the discriminator names an enum type, then all members
-        # of 'data' must also be members of the enum type.
+        # of 'data' must also be members of the enum type, which in turn
+        # must not collide with the discriminator name.
         if enum_define:
             if key not in enum_define['enum_values']:
                 raise QAPIExprError(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/qapi-schema/flat-union-clash-type.err 
b/tests/qapi-schema/flat-union-clash-type.err
index 6e64d1d..879beb8 100644
--- a/tests/qapi-schema/flat-union-clash-type.err
+++ b/tests/qapi-schema/flat-union-clash-type.err
@@ -1,16 +1 @@
-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
+tests/qapi-schema/flat-union-clash-type.json:8: Discriminator name 'type' 
collides with enum value in 'TestEnum'
diff --git a/tests/qapi-schema/union-clash-type.err 
b/tests/qapi-schema/union-clash-type.err
index 6e64d1d..773900b 100644
--- a/tests/qapi-schema/union-clash-type.err
+++ b/tests/qapi-schema/union-clash-type.err
@@ -1,16 +1 @@
-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
+tests/qapi-schema/union-clash-type.json:2: Union 'TestUnion' member 'kind' 
clashes with '(automatic)'
-- 
2.4.3