From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37492) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zrtpq-0003A4-36 for qemu-devel@nongnu.org; Thu, 29 Oct 2015 16:30:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zrtpo-0005ab-NI for qemu-devel@nongnu.org; Thu, 29 Oct 2015 16:30:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:56638) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zrtpo-0005aD-Ec for qemu-devel@nongnu.org; Thu, 29 Oct 2015 16:30:08 -0400 From: Eric Blake Date: Thu, 29 Oct 2015 14:29:58 -0600 Message-Id: <1446150601-6141-3-git-send-email-eblake@redhat.com> In-Reply-To: <1446150601-6141-1-git-send-email-eblake@redhat.com> References: <1446150601-6141-1-git-send-email-eblake@redhat.com> Subject: [Qemu-devel] [PATCH v9 2/5] qapi: Detect collisions in C member names List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: armbru@redhat.com, Michael Roth 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 a previously-broken test, and the resulting error message demonstrates the utility of the .describe() method added previously. No change to generated code. Signed-off-by: Eric Blake --- 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 | 43 +++++++++++++++++++--------------- 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, 29 insertions(+), 29 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index 2877e44..00e8452 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -976,19 +976,20 @@ class QAPISchemaObjectType(QAPISchemaType): seen = OrderedDict() if self._base_name: self.base = schema.lookup_type(self._base_name) - self.base.check_qmp(schema, seen) + self.base.check_qmp(schema, self.info, seen) for m in self.local_members: - m.check(schema, seen) + m.check(schema, self.info, seen) if self.variants: - self.variants.check(schema, seen) + self.variants.check(schema, self.info, seen) self.members = seen.values() # Check that this type does not introduce QMP collisions into seen - def check_qmp(self, schema, seen): + # info is the location providing seen, which is not necessarily self.info + def check_qmp(self, schema, info, seen): self.check(schema) assert not self.variants # not implemented for m in self.members: - m.check(schema, seen) + m.check(schema, info, seen) def is_implicit(self): # See QAPISchema._make_implicit_object_type() @@ -1029,15 +1030,19 @@ class QAPISchemaObjectTypeMember(object): assert not self.owner self.owner = name - def check(self, schema, seen): + def check(self, schema, info, seen): # seen is a map of names we must not collide with (either QMP # names, when called by ObjectType, or case names, when called # by Variant). This method is safe to call over multiple 'seen'. assert self.owner - assert self.name not in seen self.type = schema.lookup_type(self._type_name) assert self.type - seen[self.name] = self + 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 c_type(self): return self.type.c_type() @@ -1084,21 +1089,21 @@ class QAPISchemaObjectTypeVariants(object): for v in self.variants: v.set_owner(name) - def check(self, schema, seen): + def check(self, schema, info, seen): if self.tag_name: # flat union - self.tag_member = seen[self.tag_name] - assert self.tag_member + self.tag_member = seen[c_name(self.tag_name)] + assert self.tag_name == self.tag_member.name elif seen: # simple union assert self.tag_member in seen.itervalues() else: # alternate - self.tag_member.check(schema, seen) + self.tag_member.check(schema, info, seen) if not isinstance(self.tag_member, QAPISchemaAlternateTypeTag): assert isinstance(self.tag_member.type, QAPISchemaEnumType) cases = OrderedDict() for v in self.variants: # Reset seen array for each variant, since QMP names from one # branch do not affect another branch, nor add to all_members - v.check(schema, self.tag_member.type, dict(seen), cases) + v.check(schema, info, self.tag_member.type, dict(seen), cases) class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): @@ -1107,13 +1112,13 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): def __init__(self, name, typ): QAPISchemaObjectTypeMember.__init__(self, name, typ, False) - def check(self, schema, tag_type, seen, cases): + def check(self, schema, info, tag_type, seen, cases): # cases is case names we must not collide with - QAPISchemaObjectTypeMember.check(self, schema, cases) + QAPISchemaObjectTypeMember.check(self, schema, info, cases) if tag_type: # seen is QMP names our members must not collide with assert self.name in tag_type.values - self.type.check_qmp(schema, seen) + self.type.check_qmp(schema, info, seen) # This function exists to support ugly simple union special cases # TODO get rid of them, and drop the function @@ -1135,7 +1140,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' @@ -1148,10 +1153,10 @@ class QAPISchemaAlternateTypeTag(QAPISchemaObjectTypeMember): def __init__(self): QAPISchemaObjectTypeMember.__init__(self, 'type', '', False) - def check(self, schema, seen): + def check(self, schema, info, seen): assert self.owner assert len(seen) == 0 - seen[self.name] = self + seen[c_name(self.name)] = self def c_type(self): return 'qtype_code' 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