Detect attempts to declare two object members that would result
in the same C member name, by keying the 'seen' dictionary off
of the C name rather than the qapi name. It also requires passing
info through the check_clash() methods.
This addresses a TODO and fixes the previously-broken
args-name-clash test. The resulting error message demonstrates
the utility of the .describe() method added previously. No change
to generated code.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v10 (now in subset C): rebase to latest; update commit message
v9 (now in subset D): rebase to earlier changes, now only one test
affected
v8: rebase to earlier changes
v7: split out error reporting prep and member.c_name() addition
v6: rebase to earlier testsuite and info improvements
---
scripts/qapi.py | 31 +++++++++++++++++++------------
tests/qapi-schema/args-name-clash.err | 1 +
tests/qapi-schema/args-name-clash.exit | 2 +-
tests/qapi-schema/args-name-clash.json | 6 +++---
tests/qapi-schema/args-name-clash.out | 6 ------
5 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index b3af973..3a359cb 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -975,21 +975,24 @@ class QAPISchemaObjectType(QAPISchemaType):
seen = OrderedDict()
if self._base_name:
self.base = schema.lookup_type(self._base_name)
- self.base.check_clash(schema, seen)
+ self.base.check_clash(schema, self.info, seen)
for m in self.local_members:
m.check(schema)
- m.check_clash(seen)
+ m.check_clash(self.info, seen)
self.members = seen.values()
if self.variants:
self.variants.check(schema, seen)
assert self.variants.tag_member in self.members
- self.variants.check_clash(schema, seen)
+ self.variants.check_clash(schema, self.info, seen)
- def check_clash(self, schema, seen):
+ # Check that the members of this type do not cause duplicate JSON fields,
+ # and update seen to track the members seen so far. Report any errors
+ # on behalf of info, which is not necessarily self.info
+ def check_clash(self, schema, info, seen):
self.check(schema)
assert not self.variants # not implemented
for m in self.members:
- m.check_clash(seen)
+ m.check_clash(info, seen)
def is_implicit(self):
# See QAPISchema._make_implicit_object_type()
@@ -1035,10 +1038,13 @@ class QAPISchemaObjectTypeMember(object):
self.type = schema.lookup_type(self._type_name)
assert self.type
- def check_clash(self, seen):
- # TODO change key of seen from QAPI name to C name
- assert self.name not in seen
- seen[self.name] = self
+ def check_clash(self, info, seen):
+ name = c_name(self.name)
+ if name in seen:
+ raise QAPIExprError(info,
+ "%s collides with %s"
+ % (self.describe(), seen[name].describe()))
+ seen[name] = self
def _pretty_owner(self):
# See QAPISchema._make_implicit_object_type() - reverse the
@@ -1081,18 +1087,19 @@ class QAPISchemaObjectTypeVariants(object):
def check(self, schema, seen):
if self.tag_name: # flat union
- self.tag_member = seen[self.tag_name]
+ self.tag_member = seen[c_name(self.tag_name)]
+ assert self.tag_name == self.tag_member.name
tag_type = self.tag_member.type
assert isinstance(tag_type, QAPISchemaEnumType)
for v in self.variants:
v.check(schema)
assert v.name in tag_type.values
- def check_clash(self, schema, seen):
+ def check_clash(self, schema, info, seen):
for v in self.variants:
# Reset seen map for each variant, since qapi names from one
# branch do not affect another branch
- v.type.check_clash(schema, dict(seen))
+ v.type.check_clash(schema, info, dict(seen))
class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
diff --git a/tests/qapi-schema/args-name-clash.err
b/tests/qapi-schema/args-name-clash.err
index e69de29..2735217 100644
--- a/tests/qapi-schema/args-name-clash.err
+++ b/tests/qapi-schema/args-name-clash.err
@@ -0,0 +1 @@
+tests/qapi-schema/args-name-clash.json:5: 'a_b' (argument of oops) collides
with 'a-b' (argument of oops)
diff --git a/tests/qapi-schema/args-name-clash.exit
b/tests/qapi-schema/args-name-clash.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/args-name-clash.exit
+++ b/tests/qapi-schema/args-name-clash.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/args-name-clash.json
b/tests/qapi-schema/args-name-clash.json
index 9e8f889..3fe4ea5 100644
--- a/tests/qapi-schema/args-name-clash.json
+++ b/tests/qapi-schema/args-name-clash.json
@@ -1,5 +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.
+# Reject members that clash when mapped to C names (we would have two 'a_b'
+# members). It would also be possible to munge the C names to avoid the
+# collision, but unlikely to be worth the effort.
{ '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
index 9b2f6e4..e69de29 100644
--- a/tests/qapi-schema/args-name-clash.out
+++ b/tests/qapi-schema/args-name-clash.out
@@ -1,6 +0,0 @@
-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
--
2.4.3
