diff mbox

[v8.5,2/4] qapi: Check for QMP collisions of flat union branches

Message ID 1446504092-29008-3-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake Nov. 2, 2015, 10:41 p.m. UTC
Right now, our ad hoc parser ensures that we cannot have a
flat union that introduces any QMP member names that would
conflict with the non-variant QMP members already present
from the union's base type (see flat-union-clash-member.json).
However, we want QAPISchema*.check() to make the same check,
so we can later reduce some of the ad hoc checks.

Basically, all branches of a flat union must be qapi structs
with no variants, at which point those members appear in the
same JSON object as all non-variant members.  We already have
a map 'seen' of all non-variant members passed in to
QAPISchemaObjectTypeVariants.check(), which we clone for each
particular variant (since the members of one variant do not
clash with another); all that is additionally needed is to
actually check the each member of the variant type do not add
any collisions.

For simple unions, the same code happens to work (however, our
synthesized wrapper type has a single member 'data' which will
never collide with the one non-variant member 'type').

But for alternates, we do NOT want to check the type members
for adding collisions (an alternate has no parent JSON object
that is merging in member names, the way a flat union does), so
we have to pass in an extra flag to distinguish whether we are
working with a union or an alternate.  The flag is temporary; a
later patch will rework how alternates are laid out by creating
a new subclass of QAPISchemaObjectTypeMember, and detecting the
use of this subclass for variants.tag_member will serve the
same purpose.

Note that an early proposal [1] for what eventually became
commit ac88219a had QAPISchemaObjectTypeVariant.check() ensure
that the variant type was complete, although it was later
removed [2]; the checks added here happen to match what that
earlier attempt meant to do.

[1] https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg00394.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2015-08/msg00450.html

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

---
v9: new patch, split off from v8 7/17
---
 scripts/qapi.py | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)
diff mbox

Patch

diff --git a/scripts/qapi.py b/scripts/qapi.py
index fff4adb..3cf051f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1047,7 +1047,8 @@  class QAPISchemaObjectTypeVariants(object):
         self.tag_member = tag_member
         self.variants = variants

-    def check(self, schema, seen):
+    # TODO drop 'union' param once tag_member is sufficient to spot alternates
+    def check(self, schema, seen, union=True):
         if self.tag_name:    # flat union
             self.tag_member = seen[self.tag_name]
             assert self.tag_member
@@ -1055,17 +1056,29 @@  class QAPISchemaObjectTypeVariants(object):
             assert self.tag_member in seen.itervalues()
         assert isinstance(self.tag_member.type, QAPISchemaEnumType)
         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
+            v.check(schema, self.tag_member.type, dict(seen), 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' param once tag_type is sufficient to spot alternates
+    def check(self, schema, tag_type, seen, union):
+        QAPISchemaObjectTypeMember.check(self, schema, dict(seen))
         assert self.name in tag_type.values
+        if union:
+            # 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

     # This function exists to support ugly simple union special cases
     # TODO get rid of them, and drop the function
@@ -1088,7 +1101,7 @@  class QAPISchemaAlternateType(QAPISchemaType):
     def check(self, schema):
         seen = {}
         self.variants.tag_member.check(schema, seen)
-        self.variants.check(schema, seen)
+        self.variants.check(schema, seen, False)

     def json_type(self):
         return 'value'