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. Rather than making all the check() methods have to pass around additional information, it is easier to have each member track the name of the type that owns it in the first place. The new member.owner field is set when registering the members and variants arrays with an object or variant type. We 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' (member of foo arguments)". To make the human-readable name of implicit types work without duplicating efforts, the name of implicit types is tweaked after the ':obj-' prefix, so that we can just trim off the prefix. This required updates to the testsuite. No change to generated code. Signed-off-by: Eric Blake <ebl...@redhat.com> --- 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 | 41 ++++++++++++--- tests/qapi-schema/args-member-array.out | 4 +- tests/qapi-schema/args-name-clash.out | 4 +- tests/qapi-schema/ident-with-escape.out | 4 +- tests/qapi-schema/qapi-schema-test.out | 88 ++++++++++++++++----------------- tests/qapi-schema/union-clash-data.out | 4 +- 6 files changed, 87 insertions(+), 58 deletions(-) diff --git a/scripts/qapi.py b/scripts/qapi.py index e49f72b..11ffc49 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -961,8 +961,16 @@ 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)) + assert not m.owner + m.owner = name + if variants is not None: + assert isinstance(variants, QAPISchemaObjectTypeVariants) + if variants.tag_member: + assert not variants.tag_member.owner + variants.tag_member.owner = name + for v in variants.variants: + assert not v.owner + v.owner = name self._base_name = base self.base = None self.local_members = local_members @@ -1023,8 +1031,10 @@ class QAPISchemaObjectTypeMember(object): self._type_name = typ self.type = None self.optional = optional + self.owner = None # will be filled by owner's init 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 @@ -1034,6 +1044,15 @@ class QAPISchemaObjectTypeMember(object): def c_name(self): return c_name(self.name) + def describe(self): + source = self.owner + if source.startswith(':obj-'): + source = source[5:] + return "'%s' (%s of %s)" % (self.name, self._describe(), source) + + def _describe(self): + return 'member' + # TODO Drop this class once we no longer have the 'type'/'kind' mismatch class QAPISchemaObjectTypeUnionTag(QAPISchemaObjectTypeMember): @@ -1042,6 +1061,9 @@ class QAPISchemaObjectTypeUnionTag(QAPISchemaObjectTypeMember): assert self.type.is_implicit(QAPISchemaEnumType) return 'kind' + def describe(self): + return "'kind' (implicit tag of %s)" % self.owner + class QAPISchemaObjectTypeVariants(object): def __init__(self, tag_name, tag_member, variants): @@ -1086,12 +1108,19 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): return self.type.members[0].type return None + def _describe(self): + return 'branch' + class QAPISchemaAlternateType(QAPISchemaType): def __init__(self, name, info, variants): QAPISchemaType.__init__(self, name, info) assert isinstance(variants, QAPISchemaObjectTypeVariants) - assert not variants.tag_name + assert variants.tag_member and not variants.tag_member.owner + variants.tag_member.owner = name + for v in variants.variants: + assert not v.owner + v.owner = name self.variants = variants def check(self, schema): @@ -1222,7 +1251,7 @@ class QAPISchema(object): def _make_implicit_object_type(self, name, info, role, members): if not members: return None - name = ':obj-%s-%s' % (name, role) + name = ':obj-%s %s' % (name, role) if not self.lookup_entity(name, QAPISchemaObjectType): self._def_entity(QAPISchemaObjectType(name, info, None, members, None)) @@ -1312,7 +1341,7 @@ class QAPISchema(object): success_response = expr.get('success-response', True) if isinstance(data, OrderedDict): data = self._make_implicit_object_type( - name, info, 'arg', self._make_members(data, info)) + name, info, 'arguments', self._make_members(data, info)) if isinstance(rets, list): assert len(rets) == 1 rets = self._make_array_type(rets[0], info) @@ -1324,7 +1353,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/args-member-array.out b/tests/qapi-schema/args-member-array.out index b3b92df..c822309 100644 --- a/tests/qapi-schema/args-member-array.out +++ b/tests/qapi-schema/args-member-array.out @@ -1,9 +1,9 @@ object :empty -object :obj-okay-arg +object :obj-okay arguments member member1: intList optional=False member member2: defList optional=False enum abc ['a', 'b', 'c'] object def member array: abcList optional=False -command okay :obj-okay-arg -> None +command okay :obj-okay arguments -> None gen=True success_response=True diff --git a/tests/qapi-schema/args-name-clash.out b/tests/qapi-schema/args-name-clash.out index 9b2f6e4..0e986b6 100644 --- a/tests/qapi-schema/args-name-clash.out +++ b/tests/qapi-schema/args-name-clash.out @@ -1,6 +1,6 @@ object :empty -object :obj-oops-arg +object :obj-oops arguments member a-b: str optional=False member a_b: str optional=False -command oops :obj-oops-arg -> None +command oops :obj-oops arguments -> None gen=True success_response=True diff --git a/tests/qapi-schema/ident-with-escape.out b/tests/qapi-schema/ident-with-escape.out index f4542b1..4544777 100644 --- a/tests/qapi-schema/ident-with-escape.out +++ b/tests/qapi-schema/ident-with-escape.out @@ -1,5 +1,5 @@ object :empty -object :obj-fooA-arg +object :obj-fooA arguments member bar1: str optional=False -command fooA :obj-fooA-arg -> None +command fooA :obj-fooA arguments -> None gen=True success_response=True diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index 2a8c82e..c666481 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -1,56 +1,56 @@ 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 member enum3: EnumOne optional=True -object :obj-__org.qemu_x-command-arg +object :obj-__org.qemu_x-command arguments member a: __org.qemu_x-EnumList optional=False member b: __org.qemu_x-StructList optional=False member c: __org.qemu_x-Union2 optional=False member d: __org.qemu_x-Alt optional=False -object :obj-anyList-wrapper +object :obj-anyList wrapper member data: anyList optional=False -object :obj-boolList-wrapper +object :obj-boolList wrapper member data: boolList optional=False -object :obj-guest-sync-arg +object :obj-guest-sync arguments member arg: any optional=False -object :obj-int16List-wrapper +object :obj-int16List wrapper member data: int16List optional=False -object :obj-int32List-wrapper +object :obj-int32List wrapper member data: int32List optional=False -object :obj-int64List-wrapper +object :obj-int64List wrapper member data: int64List optional=False -object :obj-int8List-wrapper +object :obj-int8List wrapper member data: int8List optional=False -object :obj-intList-wrapper +object :obj-intList wrapper member data: intList optional=False -object :obj-numberList-wrapper +object :obj-numberList wrapper member data: numberList optional=False -object :obj-sizeList-wrapper +object :obj-sizeList wrapper member data: sizeList optional=False -object :obj-str-wrapper +object :obj-str wrapper member data: str optional=False -object :obj-strList-wrapper +object :obj-strList wrapper member data: strList optional=False -object :obj-uint16List-wrapper +object :obj-uint16List wrapper member data: uint16List optional=False -object :obj-uint32List-wrapper +object :obj-uint32List wrapper member data: uint32List optional=False -object :obj-uint64List-wrapper +object :obj-uint64List wrapper member data: uint64List optional=False -object :obj-uint8List-wrapper +object :obj-uint8List wrapper member data: uint8List optional=False -object :obj-user_def_cmd1-arg +object :obj-user_def_cmd1 arguments member ud1a: UserDefOne optional=False -object :obj-user_def_cmd2-arg +object :obj-user_def_cmd2 arguments member ud1a: UserDefOne optional=False member ud1b: UserDefOne optional=True -object :obj-user_def_cmd3-arg +object :obj-user_def_cmd3 arguments member a: int optional=False member b: int optional=True alternate AltIntNum @@ -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 @@ -123,20 +123,20 @@ object UserDefFlatUnion2 case value2: UserDefB case value3: UserDefA object UserDefNativeListUnion - case integer: :obj-intList-wrapper - case s8: :obj-int8List-wrapper - case s16: :obj-int16List-wrapper - case s32: :obj-int32List-wrapper - case s64: :obj-int64List-wrapper - case u8: :obj-uint8List-wrapper - case u16: :obj-uint16List-wrapper - case u32: :obj-uint32List-wrapper - case u64: :obj-uint64List-wrapper - case number: :obj-numberList-wrapper - case boolean: :obj-boolList-wrapper - case string: :obj-strList-wrapper - case sizes: :obj-sizeList-wrapper - case any: :obj-anyList-wrapper + case integer: :obj-intList wrapper + case s8: :obj-int8List wrapper + case s16: :obj-int16List wrapper + case s32: :obj-int32List wrapper + case s64: :obj-int64List wrapper + case u8: :obj-uint8List wrapper + case u16: :obj-uint16List wrapper + case u32: :obj-uint32List wrapper + case u64: :obj-uint64List wrapper + case number: :obj-numberList wrapper + case boolean: :obj-boolList wrapper + case string: :obj-strList wrapper + case sizes: :obj-sizeList wrapper + case any: :obj-anyList wrapper enum UserDefNativeListUnionKind ['integer', 's8', 's16', 's32', 's64', 'u8', 'u16', 'u32', 'u64', 'number', 'boolean', 'string', 'sizes', 'any'] object UserDefOne base UserDefZero @@ -178,21 +178,21 @@ object __org.qemu_x-Struct object __org.qemu_x-Struct2 member array: __org.qemu_x-Union1List optional=False object __org.qemu_x-Union1 - case __org.qemu_x-branch: :obj-str-wrapper + case __org.qemu_x-branch: :obj-str wrapper enum __org.qemu_x-Union1Kind ['__org.qemu_x-branch'] object __org.qemu_x-Union2 base __org.qemu_x-Base tag __org.qemu_x-member1 case __org.qemu_x-value: __org.qemu_x-Struct2 -command __org.qemu_x-command :obj-__org.qemu_x-command-arg -> __org.qemu_x-Union1 +command __org.qemu_x-command :obj-__org.qemu_x-command arguments -> __org.qemu_x-Union1 gen=True success_response=True -command guest-sync :obj-guest-sync-arg -> any +command guest-sync :obj-guest-sync arguments -> any gen=True success_response=True command user_def_cmd None -> None gen=True success_response=True -command user_def_cmd1 :obj-user_def_cmd1-arg -> None +command user_def_cmd1 :obj-user_def_cmd1 arguments -> None gen=True success_response=True -command user_def_cmd2 :obj-user_def_cmd2-arg -> UserDefTwo +command user_def_cmd2 :obj-user_def_cmd2 arguments -> UserDefTwo gen=True success_response=True -command user_def_cmd3 :obj-user_def_cmd3-arg -> int +command user_def_cmd3 :obj-user_def_cmd3 arguments -> int gen=True success_response=True diff --git a/tests/qapi-schema/union-clash-data.out b/tests/qapi-schema/union-clash-data.out index 6277239..4c5903f 100644 --- a/tests/qapi-schema/union-clash-data.out +++ b/tests/qapi-schema/union-clash-data.out @@ -1,6 +1,6 @@ object :empty -object :obj-int-wrapper +object :obj-int wrapper member data: int optional=False object TestUnion - case data: :obj-int-wrapper + case data: :obj-int wrapper enum TestUnionKind ['data'] -- 2.4.3