@@ -533,7 +533,6 @@ def check_union(expr, expr_info):
base = expr.get('base')
discriminator = expr.get('discriminator')
members = expr['data']
- values = {'MAX': '(automatic)'}
# Two types of unions, determined by discriminator.
@@ -593,15 +592,6 @@ def check_union(expr, expr_info):
"enum '%s'" %
(key, enum_define["enum_name"]))
- # Otherwise, check for conflicts in the generated enum
- else:
- c_key = camel_to_upper(key)
- if c_key in values:
- raise QAPIExprError(expr_info,
- "Union '%s' member '%s' clashes with '%s'"
- % (name, key, values[c_key]))
- values[c_key] = key
-
def check_alternate(expr, expr_info):
name = expr['alternate']
@@ -630,7 +620,6 @@ def check_enum(expr, expr_info):
name = expr['enum']
members = expr.get('data')
prefix = expr.get('prefix')
- values = {'MAX': '(automatic)'}
if not isinstance(members, list):
raise QAPIExprError(expr_info,
@@ -641,12 +630,6 @@ def check_enum(expr, expr_info):
for member in members:
check_name(expr_info, "Member of enum '%s'" % name, member,
enum_member=True)
- key = camel_to_upper(member)
- if key in values:
- raise QAPIExprError(expr_info,
- "Enum '%s' member '%s' clashes with '%s'"
- % (name, member, values[key]))
- values[key] = member
def check_struct(expr, expr_info):
@@ -873,8 +856,30 @@ class QAPISchemaEnumType(QAPISchemaType):
self.values = values
self.prefix = prefix
+ def _describe(self, schema):
+ # If the enum is implicit, report the error on behalf of
+ # the union or alternate that triggered the enum
+ if self.is_implicit():
+ owner = schema.lookup_type(self.name[:-4])
+ assert owner
+ if isinstance(owner, QAPISchemaAlternateType):
+ return "Alternate '%s' branch" % owner.name
+ else:
+ return "Union '%s' branch" % owner.name
+ else:
+ return "Enum '%s' value" % self.name
+
def check(self, schema):
- assert len(set(self.values)) == len(self.values)
+ # Check for collisions on the generated C enum values
+ seen = {c_enum_const(self.name, 'MAX'): '(automatic MAX)'}
+ for value in self.values:
+ c_value = c_enum_const(self.name, value)
+ if c_value in seen:
+ raise QAPIExprError(self.info,
+ "%s '%s' clashes with '%s'"
+ % (self._describe(schema), value,
+ seen[c_value]))
+ seen[c_value] = value
def is_implicit(self):
# See QAPISchema._make_implicit_enum_type()
@@ -335,7 +335,6 @@ qapi-schema += unclosed-list.json
qapi-schema += unclosed-object.json
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
@@ -1 +1 @@
-tests/qapi-schema/enum-clash-member.json:2: Enum 'MyEnum' member 'ONE' clashes with 'one'
+tests/qapi-schema/enum-clash-member.json:2: Enum 'MyEnum' value 'ONE' clashes with 'one'
@@ -1 +1 @@
-tests/qapi-schema/enum-max-member.json:3: Enum 'MyEnum' member 'max' clashes with '(automatic)'
+tests/qapi-schema/enum-max-member.json:3: Enum 'MyEnum' value 'max' clashes with '(automatic MAX)'
deleted file mode 100644
@@ -1 +0,0 @@
-tests/qapi-schema/union-bad-branch.json:6: Union 'MyUnion' member 'ONE' clashes with 'one'
deleted file mode 100644
@@ -1 +0,0 @@
-1
deleted file mode 100644
@@ -1,8 +0,0 @@
-# we reject normal unions where branches would collide in C
-{ 'struct': 'One',
- 'data': { 'string': 'str' } }
-{ 'struct': 'Two',
- 'data': { 'number': 'int' } }
-{ 'union': 'MyUnion',
- 'data': { 'one': 'One',
- 'ONE': 'Two' } }
deleted file mode 100644
@@ -1 +1 @@
-tests/qapi-schema/union-clash-branches.json:4: Union 'TestUnion' member 'a_b' clashes with 'a-b'
+tests/qapi-schema/union-clash-branches.json:4: 'a_b' (branch of TestUnion) collides with 'a-b' (branch of TestUnion)
@@ -1,5 +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').
+# would try to generate two members 'a_b').
{ 'union': 'TestUnion',
'data': { 'a-b': 'int', 'a_b': 'str' } }
@@ -1 +1 @@
-tests/qapi-schema/union-max.json:2: Union 'Union' member 'max' clashes with '(automatic)'
+tests/qapi-schema/union-max.json:2: Union 'Union' branch 'max' clashes with '(automatic MAX)'
Similar to the previous commit, move the detection of a collision in enum values from parse time to QAPISchemaEnumType.check(). This happens to also detect collisions in union branch names mapping to the same enum value, even when the names do not collide case-wise. So for a decent error message, we have to determine if the enum is implicit (and if so where the real collision lies). Testing this showed that the test union-bad-branch wasn't adding much: union-clash-branches exposes the error message when branches directly collide, and union-max exposes the error message when branches cause an enum collision (union-bad-branch basically causes an enum collision that would not be a C collision). Maybe we should require ALL names to be case-insensitively unique, but that would be the topic for a different patch. No change to generated code. Signed-off-by: Eric Blake <eblake@redhat.com> --- v9: rebase to earlier changes, update commit message, break out helper _describe() method v8: rebase to earlier changes; better comments v7: retitle and improve commit message; earlier subclass patches avoid problem with detecting 'kind' collision v6: new patch --- scripts/qapi.py | 41 ++++++++++++++++------------- tests/Makefile | 1 - tests/qapi-schema/enum-clash-member.err | 2 +- tests/qapi-schema/enum-max-member.err | 2 +- tests/qapi-schema/union-bad-branch.err | 1 - tests/qapi-schema/union-bad-branch.exit | 1 - tests/qapi-schema/union-bad-branch.json | 8 ------ tests/qapi-schema/union-bad-branch.out | 0 tests/qapi-schema/union-clash-branches.err | 2 +- tests/qapi-schema/union-clash-branches.json | 2 +- tests/qapi-schema/union-max.err | 2 +- 11 files changed, 28 insertions(+), 34 deletions(-) delete mode 100644 tests/qapi-schema/union-bad-branch.err delete mode 100644 tests/qapi-schema/union-bad-branch.exit delete mode 100644 tests/qapi-schema/union-bad-branch.json delete mode 100644 tests/qapi-schema/union-bad-branch.out