[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v8 14/18] qapi: Detect collisions in C member names
From: |
Eric Blake |
Subject: |
[Qemu-devel] [PATCH v8 14/18] qapi: Detect collisions in C member names |
Date: |
Mon, 12 Oct 2015 22:22:34 -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>
---
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 | 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 8b29e11..58c4bb3 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -978,11 +978,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):
@@ -1022,13 +1022,19 @@ class QAPISchemaObjectTypeMember(object):
assert not self.owner
self.owner = name
- 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)
@@ -1088,23 +1094,24 @@ class QAPISchemaObjectTypeVariants(object):
for v in self.variants:
v.set_owner(name)
- 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
@@ -1129,7 +1136,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..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
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 v8 03/18] qapi: Drop redundant alternate-good test, (continued)
- [Qemu-devel] [PATCH v8 03/18] qapi: Drop redundant alternate-good test, Eric Blake, 2015/10/13
- [Qemu-devel] [PATCH v8 04/18] qapi: Move empty-enum to compile-time test, Eric Blake, 2015/10/13
- [Qemu-devel] [PATCH v8 05/18] qapi: Drop redundant returns-int test, Eric Blake, 2015/10/13
- [Qemu-devel] [PATCH v8 01/18] qapi: Use predicate callback to determine visit filtering, Eric Blake, 2015/10/13
- [Qemu-devel] [PATCH v8 07/18] qapi: Don't use info as witness of implicit object type, Eric Blake, 2015/10/13
- [Qemu-devel] [PATCH v8 08/18] qapi: Lazy creation of array types, Eric Blake, 2015/10/13
- [Qemu-devel] [PATCH v8 14/18] qapi: Detect collisions in C member names,
Eric Blake <=
- [Qemu-devel] [PATCH v8 11/18] qapi: Simplify gen_struct_field(), Eric Blake, 2015/10/13
- [Qemu-devel] [PATCH v8 09/18] qapi: Create simple union type member earlier, Eric Blake, 2015/10/13
- [Qemu-devel] [PATCH v8 12/18] qapi: Track location that created an implicit type, Eric Blake, 2015/10/13
- [Qemu-devel] [PATCH v8 10/18] qapi: Move union tag quirks into subclass, Eric Blake, 2015/10/13