[Qemu-devel] [PATCH v8 07/17] qapi: Rework collision assertions

[Eric Blake]

Now that we have separate namespaces for QMP vs. tag values,
we can simplify how the QAPISchema*.check() methods check for
collisions.  Each QAPISchemaObjectTypeMember check() call is
given a single set of names it must not collide with; this set
is either the QMP names (when this member is used by an
ObjectType) or the case names (when this member is used by an
ObjectTypeVariants).  We no longer need an all_members
parameter, as it can be computed by seen.values().  When used
by a union, QAPISchemaObjectTypeVariant must also perform a
QMP collision check for each member of its corresponding type.

The new ObjectType.check_qmp() is an idempotent subset of
check(), and can be called multiple times over different seen
sets (useful, since the members of one type can be applied
into more than one other location via inheritance or flat
union variants).

The code needs a temporary hack of passing a 'union' flag
through Variants.check(), since we do not inline the branches
of an alternate type into a parent QMP object.  A later patch
will rework how alternates are laid out, by adding a new
subclass, and that will allow us to drop the extra parameter.

There are no changes to generated code.

Future patches will enhance testsuite coverage, improve error
message quality on actual collisions, and move collision
checks out of ad hoc parse code into the check() methods.

Signed-off-by: Eric Blake <address@hidden>

---
v8: rebase to earlier patches, defer positive test additions to
later in series
v7: new patch, although it is a much cleaner implementation of
what was attempted by subset B v8 15/18
https://lists.gnu.org/archive/html/qemu-devel/2015-10/msg03042.html
---
 scripts/qapi.py | 55 +++++++++++++++++++++++++++++++++----------------------
 1 file changed, 33 insertions(+), 22 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 84ac151..c571709 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -972,28 +972,28 @@ class QAPISchemaObjectType(QAPISchemaType):
         self.variants = variants
         self.members = None

+    # Finish construction, and validate that all members are usable
     def check(self, schema):
         assert self.members is not False        # not running in cycles
         if self.members:
             return
         self.members = False                    # mark as being checked
+        seen = OrderedDict()
         if self._base_name:
             self.base = schema.lookup_type(self._base_name)
-            assert isinstance(self.base, QAPISchemaObjectType)
-            assert not self.base.variants       # not implemented
-            self.base.check(schema)
-            members = list(self.base.members)
-        else:
-            members = []
-        seen = {}
-        for m in members:
-            assert c_name(m.name) not in seen
-            seen[m.name] = m
+            self.base.check_qmp(schema, seen)
         for m in self.local_members:
-            m.check(schema, members, seen)
+            m.check(schema, seen)
         if self.variants:
-            self.variants.check(schema, members, seen)
-        self.members = members
+            self.variants.check(schema, seen)
+        self.members = seen.values()
+
+    # Check that this type does not introduce QMP collisions into seen
+    def check_qmp(self, schema, seen):
+        self.check(schema)
+        assert not self.variants       # not implemented
+        for m in self.members:
+            m.check(schema, seen)

     def is_implicit(self):
         # See QAPISchema._make_implicit_object_type()
@@ -1027,11 +1027,13 @@ class QAPISchemaObjectTypeMember(object):
         self.type = None
         self.optional = optional

-    def check(self, schema, all_members, seen):
+    def check(self, schema, 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.name not in seen
         self.type = schema.lookup_type(self._type_name)
         assert self.type
-        all_members.append(self)
         seen[self.name] = self


@@ -1050,26 +1052,35 @@ class QAPISchemaObjectTypeVariants(object):
         self.tag_member = tag_member
         self.variants = variants

-    def check(self, schema, members, seen):
+    # TODO drop union once alternates can be distinguished by tag_member
+    def check(self, schema, seen, union=True):
         if self.tag_name:    # flat union
             self.tag_member = seen[self.tag_name]
+            assert self.tag_member
         elif seen:           # simple union
             assert self.tag_member in seen.itervalues()
         else:                # alternate
-            self.tag_member.check(schema, members, seen)
+            self.tag_member.check(schema, seen)
         assert isinstance(self.tag_member.type, QAPISchemaEnumType)
+        cases = OrderedDict()
         for v in self.variants:
-            vseen = dict(seen)
-            v.check(schema, self.tag_member.type, vseen)
+            # 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, union)


 class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
     def __init__(self, name, typ):
         QAPISchemaObjectTypeMember.__init__(self, name, typ, False)

-    def check(self, schema, tag_type, seen):
-        QAPISchemaObjectTypeMember.check(self, schema, [], seen)
+    # TODO drop union once alternates can be distinguished by tag_type
+    def check(self, schema, tag_type, seen, cases, union):
+        # cases is case names we must not collide with
+        QAPISchemaObjectTypeMember.check(self, schema, cases)
         assert self.name in tag_type.values
+        if union:
+            # seen is QMP names our members must not collide with
+            self.type.check_qmp(schema, seen)

     # This function exists to support ugly simple union special cases
     # TODO get rid of them, and drop the function
@@ -1090,7 +1101,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
         self.variants = variants

     def check(self, schema):
-        self.variants.check(schema, [], {})
+        self.variants.check(schema, {}, False)

     def json_type(self):
         return 'value'
-- 
2.4.3