[Qemu-devel] [PATCH v7 09/14] qapi: Track owner of each object member

[Eric Blake]

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 <address@hidden>

---
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