[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v7 10/14] qapi: Detect collisions in C member names
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v7 10/14] qapi: Detect collisions in C member names |
Date: |
Sat, 3 Oct 2015 21:41:09 -0600 |
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 some of the check() methods.
This fixes two 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 <address@hidden>
---
v7: split out error reporting prep and member.c_name() addition
v6: rebase to earlier testsuite and info improvements
---
scripts/qapi.py | 33 ++++++++++++++++----------
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, 32 insertions(+), 42 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 11ffc49..30f1483 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -993,11 +993,11 @@ class QAPISchemaObjectType(QAPISchemaType):
seen = {}
for m in members:
assert m.c_name() not in seen
- seen[m.name] = m
+ 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):
@@ -1033,13 +1033,19 @@ class QAPISchemaObjectTypeMember(object):
self.optional = optional
self.owner = None # will be filled by owner's init
- def check(self, schema, all_members, seen):
+ def check(self, schema, info, all_members, seen):
assert self.owner
- assert self.name not in seen
self.type = schema.lookup_type(self._type_name)
assert self.type
+ # Check that there is no collision in generated C names (which
+ # also ensures no collisions in QMP names)
+ if self.c_name() in seen:
+ raise QAPIExprError(info,
+ "%s collides with %s"
+ % (self.describe(),
+ seen[self.c_name()].describe()))
all_members.append(self)
- seen[self.name] = self
+ seen[self.c_name()] = self
def c_name(self):
return c_name(self.name)
@@ -1080,23 +1086,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):
QAPISchemaObjectTypeMember.__init__(self, name, typ, False)
- 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
# This function exists to support ugly simple union special cases
@@ -1124,7 +1131,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'
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 0e986b6..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 arguments
- member a-b: str optional=False
- member a_b: str optional=False
-command oops :obj-oops arguments -> 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
- [Qemu-devel] [PATCH v7 09/14] qapi: Track owner of each object member, (continued)
- [Qemu-devel] [PATCH v7 09/14] qapi: Track owner of each object member, Eric Blake, 2015/10/08
- [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check(), Eric Blake, 2015/10/08
- Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check(), Markus Armbruster, 2015/10/12
- Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check(), Eric Blake, 2015/10/15
- Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check(), Eric Blake, 2015/10/13
- Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check(), Markus Armbruster, 2015/10/13
- Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check(), Eric Blake, 2015/10/13
- Re: [Qemu-devel] [PATCH v7 11/14] qapi: Move duplicate member checks to schema check(), Markus Armbruster, 2015/10/13
[Qemu-devel] [PATCH v7 10/14] qapi: Detect collisions in C member names,
Eric Blake <=
[Qemu-devel] [PATCH v7 14/14] qapi: Detect base class loops, Eric Blake, 2015/10/08