[Qemu-devel] [PATCH v8.5 4/4] qapi: Consolidate collision detection code

[Eric Blake <eblake@redhat.com>]

Rather than having three separate places populate the seen map,
it is easier to just factor out a subset of Member.check()
that does this as a new method check_collision(), and have the
remaining places in ObjectType.check() and Variant.check()
call into it.  This likewise means a new helper method
ObjectType.check_collision().  Later patches can then change
the handling of the seen array in just one place, when moving
away from ad hoc parser tests.

Note that there was some discrepancy in the existing code on
whether name or c_name(name) could not previously be in the
seen map; a future patch will clean this up to consistently
populate the map via c_name().

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v9: new patch, split off from v8 7/17; name change from
ObjectType.check_qmp() to check_collision(), and new method
Member.check_collision(). I'm open to naming suggestions.
---
 scripts/qapi.py | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 10bf16f..b519d30 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -981,18 +981,21 @@ class QAPISchemaObjectType(QAPISchemaType):
         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)
-            for m in self.base.members:
-                assert c_name(m.name) not in seen
-                seen[m.name] = m
+            self.base.check_collision(schema, seen)
         for m in self.local_members:
             m.check(schema, seen)
         if self.variants:
             self.variants.check(schema, seen)
         self.members = seen.values()

+    # Check that the members of this type do not cause duplicate JSON fields,
+    # and update seen to track the members seen so far
+    def check_collision(self, schema, seen):
+        assert not self.variants       # not implemented
+        self.check(schema)
+        for m in self.members:
+            m.check_collision(seen)
+
     def is_implicit(self):
         # See QAPISchema._make_implicit_object_type()
         return self.name[0] == ':'
@@ -1026,9 +1029,16 @@ class QAPISchemaObjectTypeMember(object):
         self.optional = optional

     def check(self, schema, seen):
-        assert self.name not in seen
         self.type = schema.lookup_type(self._type_name)
+        self.check_collision(seen)
+
+    # Check that this member does not collide with anything in seen (the
+    # set of non-variant members when called from QAPISchemaObjectType,
+    # or the set of tag values when called from QAPISchemaObjectTypeVariant),
+    # and update seen accordingly.
+    def check_collision(self, seen):
         assert self.type
+        assert self.name not in seen
         seen[self.name] = self


@@ -1074,12 +1084,7 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
             # If this variant is used within a union, then each member
             # field must avoid collisions with the non-variant members
             # already present in the union.
-            assert isinstance(self.type, QAPISchemaObjectType)
-            assert not self.type.variants       # not implemented
-            self.type.check(schema)
-            for m in self.type.members:
-                assert c_name(m.name) not in seen
-                seen[m.name] = m
+            self.type.check_collision(schema, seen)

     # This function exists to support ugly simple union special cases
     # TODO get rid of them, and drop the function
-- 
2.4.3