From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44601) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zlr6w-00026M-4e for qemu-devel@nongnu.org; Tue, 13 Oct 2015 00:22:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zlr6t-0005nE-Nr for qemu-devel@nongnu.org; Tue, 13 Oct 2015 00:22:50 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58613) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zlr6t-0005mZ-ED for qemu-devel@nongnu.org; Tue, 13 Oct 2015 00:22:47 -0400 From: Eric Blake Date: Mon, 12 Oct 2015 22:22:33 -0600 Message-Id: <1444710158-8723-14-git-send-email-eblake@redhat.com> In-Reply-To: <1444710158-8723-1-git-send-email-eblake@redhat.com> References: <1444710158-8723-1-git-send-email-eblake@redhat.com> Subject: [Qemu-devel] [PATCH v8 13/18] qapi: Track owner of each object member List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: armbru@redhat.com, Michael Roth Future commits will migrate semantic checking away from parsing and over to the various QAPISchema*.check() methods. But to report an error message about an incorrect semantic use of a member of an object type, it helps to know which type, command, or event owns the member. In particular, when a member is inherited from a base type, it is desirable to associate the member name with the base type (and not the type calling member.check()). Rather than packing additional information into the seen array passed to each member.check() (as in seen[m.name] = {'member':m, 'owner':type}), it is easier to have each member track the name of the owner type in the first place (keeping things simpler with the existing seen[m.name] = m). The new member.owner field is set via a new set_owner() function, called when registering the members and variants arrays with an object or variant type. Track only a name, and not the actual type object, to avoid creating a circular python reference chain. The source information is intended for human consumption in error messages, and a new describe() method is added to access the resulting information. For example, given the qapi: { 'command': 'foo', 'data': { 'string': 'str' } } an implementation of visit_command() that calls arg_type.members[0].describe() will see "'string' (argument of foo)". To make the human-readable name of implicit types work without duplicating efforts, the describe() method has to reverse the name of implicit types. No change to generated code. Signed-off-by: Eric Blake --- v8: don't munge implicit type names [except for event data], and instead make describe() create nicer messages. Add set_owner(), and use field 'role' instead of method _describe() v7: total rewrite: rework implicit object names, assign owner when initializing owner type rather than when creating member python object v6: rebase on new lazy array creation and simple union 'type' motion; tweak commit message --- scripts/qapi.py | 48 +++++++++++++++++++++++++++++++--- tests/qapi-schema/qapi-schema-test.out | 8 +++--- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index c9ce9ee..8b29e11 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -952,8 +952,10 @@ class QAPISchemaObjectType(QAPISchemaType): assert base is None or isinstance(base, str) for m in local_members: assert isinstance(m, QAPISchemaObjectTypeMember) - assert (variants is None or - isinstance(variants, QAPISchemaObjectTypeVariants)) + m.set_owner(name) + if variants is not None: + assert isinstance(variants, QAPISchemaObjectTypeVariants) + variants.set_owner(name) self._base_name = base self.base = None self.local_members = local_members @@ -1014,8 +1016,14 @@ class QAPISchemaObjectTypeMember(object): self._type_name = typ self.type = None self.optional = optional + self.owner = None + + def set_owner(self, name): + assert not self.owner + self.owner = name def check(self, schema, all_members, seen): + assert self.owner assert self.name not in seen self.type = schema.lookup_type(self._type_name) assert self.type @@ -1025,6 +1033,25 @@ class QAPISchemaObjectTypeMember(object): def c_name(self): return c_name(self.name) + def describe(self): + owner = self.owner + # See QAPISchema._make_implicit_object_type() - reverse the + # mapping there to create a nice human-readable description + if owner.startswith(':obj-'): + owner = owner[5:] + if owner.endswith('-arg'): + source = '(argument of %s)' % owner[:4] + elif owner.endswith('-data'): + source = '(data of %s)' % owner[:5] + else: + assert owner.endswith('-wrapper') + source = '(branch of %s)' % owner[:8] + else: + source = '(%s of %s)' % (self.role, owner) + return "'%s' %s" % (self.name, source) + + role = 'member' + # TODO Drop this class once we no longer have the 'type'/'kind' mismatch class QAPISchemaObjectTypeUnionTag(QAPISchemaObjectTypeMember): @@ -1034,6 +1061,11 @@ class QAPISchemaObjectTypeUnionTag(QAPISchemaObjectTypeMember): assert self.type.is_implicit() return 'kind' + def describe(self): + # Must override superclass describe() because self.name is 'type' + assert self.owner[0] != ':' + return "'kind' (implicit tag of %s)" % self.owner + class QAPISchemaObjectTypeVariants(object): def __init__(self, tag_name, tag_member, variants): @@ -1050,6 +1082,12 @@ class QAPISchemaObjectTypeVariants(object): self.tag_member = tag_member self.variants = variants + def set_owner(self, name): + if self.tag_member: + self.tag_member.set_owner(name) + for v in self.variants: + v.set_owner(name) + def check(self, schema, members, seen): if self.tag_name: self.tag_member = seen[self.tag_name] @@ -1079,12 +1117,15 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): return self.type.members[0].type return None + role = 'branch' + class QAPISchemaAlternateType(QAPISchemaType): def __init__(self, name, info, variants): QAPISchemaType.__init__(self, name, info) assert isinstance(variants, QAPISchemaObjectTypeVariants) assert not variants.tag_name + variants.set_owner(name) self.variants = variants def check(self, schema): @@ -1216,6 +1257,7 @@ class QAPISchema(object): def _make_implicit_object_type(self, name, info, role, members): if not members: return None + # See also QAPISchemaObjectTypeMember.describe() name = ':obj-%s-%s' % (name, role) if not self.lookup_entity(name, QAPISchemaObjectType): self._def_entity(QAPISchemaObjectType(name, info, None, @@ -1318,7 +1360,7 @@ class QAPISchema(object): data = expr.get('data') if isinstance(data, OrderedDict): data = self._make_implicit_object_type( - name, info, 'arg', self._make_members(data, info)) + name, info, 'data', self._make_members(data, info)) self._def_entity(QAPISchemaEvent(name, info, data)) def _def_exprs(self): diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index a6c80e0..d837475 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -1,9 +1,9 @@ object :empty -object :obj-EVENT_C-arg +object :obj-EVENT_C-data member a: int optional=True member b: UserDefOne optional=True member c: str optional=False -object :obj-EVENT_D-arg +object :obj-EVENT_D-data member a: EventStructOne optional=False member b: str optional=False member c: str optional=True @@ -79,8 +79,8 @@ alternate AltStrNum enum AltStrNumKind ['s', 'n'] event EVENT_A None event EVENT_B None -event EVENT_C :obj-EVENT_C-arg -event EVENT_D :obj-EVENT_D-arg +event EVENT_C :obj-EVENT_C-data +event EVENT_D :obj-EVENT_D-data enum EnumOne ['value1', 'value2', 'value3'] object EventStructOne member struct1: UserDefOne optional=False -- 2.4.3