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. Doing this was made
easier by adding a member.c_name() helper function.
As this is the first error raised within the QAPISchema*.check()
methods, we also have to pass 'info' through the call stack, and
fix the overall 'try' to display errors detected during the
check() phase.
This fixes a couple of previously-broken tests, and the
resulting error messages demonstrate the utility of the
.describe() method added previously. No change to generated
code.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
v6: rebase to earlier testsuite and info improvements
---
scripts/qapi.py | 46 ++++++++++++++++----------
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 ----
tests/qapi-schema/flat-union-clash-branch.err | 1 +
tests/qapi-schema/flat-union-clash-branch.exit | 2 +-
tests/qapi-schema/flat-union-clash-branch.json | 9 ++---
tests/qapi-schema/flat-union-clash-branch.out | 14 --------
9 files changed, 40 insertions(+), 47 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 880de94..1acf02b 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -103,6 +103,7 @@ class QAPISchemaError(Exception):
class QAPIExprError(Exception):
def __init__(self, expr_info, msg):
Exception.__init__(self)
+ assert expr_info
self.info = expr_info
self.msg = msg
@@ -964,11 +965,12 @@ class QAPISchemaObjectType(QAPISchemaType):
members = []
seen = {}
for m in members:
- seen[m.name] = m
+ assert m.c_name() not in seen
+ seen[m.c_name()] = m
for m in self.local_members:
- m.check(schema, members, seen)
+ m.check(schema, self.info, members, seen)
if self.variants:
- self.variants.check(schema, members, seen)
+ self.variants.check(schema, self.info, members, seen)
self.members = members
def is_implicit(self):
@@ -1004,12 +1006,19 @@ class QAPISchemaObjectTypeMember(object):
self.optional = optional
self._owner = owner
- def check(self, schema, all_members, seen):
- assert self.name not in seen
+ def check(self, schema, info, all_members, seen):
+ if self.c_name() in seen:
+ raise QAPIExprError(info,
+ "%s collides with %s"
+ % (self.describe(),
+ seen[self.c_name()].describe()))
self.type = schema.lookup_type(self._type_name)
assert self.type
all_members.append(self)
- seen[self.name] = self
+ seen[self.c_name()] = self
+
+ def c_name(self):
+ return c_name(self.name)
def describe(self):
return "'%s' (member of %s)" % (self.name, self._owner)
@@ -1028,23 +1037,24 @@ class QAPISchemaObjectTypeVariants(object):
self.tag_member = tag_member
self.variants = variants
- def check(self, schema, members, seen):
+ def check(self, schema, info, members, seen):
if self.tag_name:
- self.tag_member = seen[self.tag_name]
+ self.tag_member = seen[c_name(self.tag_name)]
+ assert self.tag_name == self.tag_member.name
else:
- self.tag_member.check(schema, members, seen)
+ self.tag_member.check(schema, info, members, seen)
assert isinstance(self.tag_member.type, QAPISchemaEnumType)
for v in self.variants:
vseen = dict(seen)
- v.check(schema, self.tag_member.type, vseen)
+ v.check(schema, info, self.tag_member.type, vseen)
class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
def __init__(self, name, typ, owner):
QAPISchemaObjectTypeMember.__init__(self, name, typ, False, owner)
- def check(self, schema, tag_type, seen):
- QAPISchemaObjectTypeMember.check(self, schema, [], seen)
+ def check(self, schema, info, tag_type, seen):
+ QAPISchemaObjectTypeMember.check(self, schema, info, [], seen)
assert self.name in tag_type.values
def describe(self):
@@ -1069,7 +1079,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
self.variants = variants
def check(self, schema):
- self.variants.check(schema, [], {})
+ self.variants.check(schema, self.info, [], {})
def json_type(self):
return 'value'
@@ -1128,14 +1138,14 @@ class QAPISchema(object):
def __init__(self, fname):
try:
self.exprs = check_exprs(QAPISchemaParser(open(fname, "r")).exprs)
+ self._entity_dict = {}
+ self._def_predefineds()
+ QAPISchema.predefined_initialized = True
+ self._def_exprs()
+ self.check()
except (QAPISchemaError, QAPIExprError), err:
print >>sys.stderr, err
exit(1)
- self._entity_dict = {}
- self._def_predefineds()
- QAPISchema.predefined_initialized = True
- self._def_exprs()
- self.check()
def _def_entity(self, ent):
assert ent.name not in self._entity_dict
diff --git a/tests/qapi-schema/args-name-clash.err
b/tests/qapi-schema/args-name-clash.err
index e69de29..66f609c 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' (member of oops arguments)
collides with 'a-b' (member of oops arguments)
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
diff --git a/tests/qapi-schema/flat-union-clash-branch.err
b/tests/qapi-schema/flat-union-clash-branch.err
index e69de29..0190d79 100644
--- a/tests/qapi-schema/flat-union-clash-branch.err
+++ b/tests/qapi-schema/flat-union-clash-branch.err
@@ -0,0 +1 @@
+tests/qapi-schema/flat-union-clash-branch.json:15: 'c-d' (branch of TestUnion)
collides with 'c_d' (member of Base)
diff --git a/tests/qapi-schema/flat-union-clash-branch.exit
b/tests/qapi-schema/flat-union-clash-branch.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/flat-union-clash-branch.exit
+++ b/tests/qapi-schema/flat-union-clash-branch.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/flat-union-clash-branch.json
b/tests/qapi-schema/flat-union-clash-branch.json
index e593336..a6c302f 100644
--- a/tests/qapi-schema/flat-union-clash-branch.json
+++ b/tests/qapi-schema/flat-union-clash-branch.json
@@ -1,8 +1,9 @@
# 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.
+# Reject attempts to use a branch name that would clash with a non-variant
+# member, when mapped to C names (here, we would have two 'c_d' members,
+# one from the base member, the other from the branch name).
+# TODO: We could munge the generated branch name to avoid the collision and
+# allow this to compile.
{ 'enum': 'TestEnum',
'data': [ 'base', 'c-d' ] }
{ 'struct': 'Base',
diff --git a/tests/qapi-schema/flat-union-clash-branch.out
b/tests/qapi-schema/flat-union-clash-branch.out
index 8e0da73..e69de29 100644
--- a/tests/qapi-schema/flat-union-clash-branch.out
+++ b/tests/qapi-schema/flat-union-clash-branch.out
@@ -1,14 +0,0 @@
-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
--
2.4.3
