From patchwork Wed Nov 11 06:51:18 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 542815 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 4F369140761 for ; Wed, 11 Nov 2015 18:03:23 +1100 (AEDT) Received: from localhost ([::1]:38303 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwPRA-0002rV-UE for incoming@patchwork.ozlabs.org; Wed, 11 Nov 2015 02:03:20 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41450) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwPG6-0006KU-28 for qemu-devel@nongnu.org; Wed, 11 Nov 2015 01:52:13 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZwPFt-0005df-A7 for qemu-devel@nongnu.org; Wed, 11 Nov 2015 01:51:53 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53478) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZwPFt-0005dS-2W for qemu-devel@nongnu.org; Wed, 11 Nov 2015 01:51:41 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id B81458E24E; Wed, 11 Nov 2015 06:51:40 +0000 (UTC) Received: from red.redhat.com (ovpn-113-115.phx2.redhat.com [10.3.113.115]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id tAB6pW31007377; Wed, 11 Nov 2015 01:51:40 -0500 From: Eric Blake To: qemu-devel@nongnu.org Date: Tue, 10 Nov 2015 23:51:18 -0700 Message-Id: <1447224690-9743-17-git-send-email-eblake@redhat.com> In-Reply-To: <1447224690-9743-1-git-send-email-eblake@redhat.com> References: <1447224690-9743-1-git-send-email-eblake@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: armbru@redhat.com, Michael Roth Subject: [Qemu-devel] [PATCH v11 16/28] qapi: Detect collisions in C member names X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 --- v11: rebase to earlier changes, use 'cname' local variable, shorter message in test file 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 | 5 ++--- tests/qapi-schema/args-name-clash.out | 6 ------ 5 files changed, 23 insertions(+), 22 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index 79038a8..b068141 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -977,20 +977,23 @@ class QAPISchemaObjectType(QAPISchemaType): self.base = schema.lookup_type(self._base_name) assert isinstance(self.base, QAPISchemaObjectType) self.base.check(schema) - 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): 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() @@ -1036,10 +1039,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): + cname = c_name(self.name) + if cname in seen: + raise QAPIExprError(info, + "%s collides with %s" + % (self.describe(), seen[cname].describe())) + seen[cname] = self def _pretty_owner(self): owner = self.owner @@ -1079,7 +1085,8 @@ class QAPISchemaObjectTypeVariants(object): def check(self, schema, seen): if not self.tag_member: # 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 assert isinstance(self.tag_member.type, QAPISchemaEnumType) for v in self.variants: v.check(schema) @@ -1087,12 +1094,12 @@ class QAPISchemaObjectTypeVariants(object): if isinstance(v.type, QAPISchemaObjectType): v.type.check(schema) - 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 assert isinstance(v.type, QAPISchemaObjectType) - 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..d953e8d 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:4: 'a_b' (parameter of oops) collides with 'a-b' (parameter 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..61423cb 100644 --- a/tests/qapi-schema/args-name-clash.json +++ b/tests/qapi-schema/args-name-clash.json @@ -1,5 +1,4 @@ # 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). { '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