[Qemu-devel] [PATCH v11 11/28] qapi: Check for qapi collisions of flat union branches

[Eric Blake]

Right now, our ad hoc parser ensures that we cannot have a
flat union that introduces any qapi member names that would
conflict with the non-variant qapi members already present
from the union's base type (see flat-union-clash-member.json).
We want QAPISchemaObjectType.check() to make the same check,
so we can later reduce some of the ad hoc checks.

We already ensure that all branches of a flat union are qapi
structs with no variants, at which point those members appear
in the same JSON object as all non-variant members.  And we
already have a map 'seen' of all non-variant members.  Still
missing, until now, was a call to variant.type.check() during
Variants.check() (to populate variant.type.members), and a
new Variants.check_clash() which clones the seen map for each
variant before checking that variant's members for clashes.

Note that cloning 'seen' inside Variants.check_clash()
resembles the one we just removed from Variants.check(); the
difference here is that we are now checking for clashes
among the qapi members of the variant type, rather than for
a single clash with the variant tag name itself.

In general, a non-empy type used as a branch of a flat union
cannot also be the base type of the flat union, so even though
we are adding a call to variant.type.check() in order to
populate variant.type.members, this is merely a case of
gaining topological sorting of how types are visited (and
type.check() is already set up to allow multiple calls due to
base types).

For simple unions, the same code happens to work by design,
because of our synthesized wrapper classes (however, the
wrapper has a single member 'data' which will never collide
with the one non-variant member 'type', so it doesn't really
matter).

There is no impact to alternates, which intentionally do not
need to call variants.check_clash() (there, at most one of
the variant's branches will be an ObjectType, and even if one
exists, we are not inlining the qapi members of that object
into a parent object, the way we do for unions).  However,
since variants can have non-object branches, it did mean that
the addition of v.type.check() in Variants.check() had to be
conditional on visiting an object type.

No change to generated code.

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

---
v11: keep type.check() in check(), add comment to alternate
v10: create new Variants.check_clash() rather than piggybacking
on .check()
v9: new patch, split off from v8 7/17
---
 scripts/qapi.py | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index c6cb17b..b2d071f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -992,6 +992,7 @@ class QAPISchemaObjectType(QAPISchemaType):
         if self.variants:
             self.variants.check(schema, seen)
             assert self.variants.tag_member in self.members
+            self.variants.check_clash(schema, seen)

     def is_implicit(self):
         # See QAPISchema._make_implicit_object_type()
@@ -1056,6 +1057,18 @@ class QAPISchemaObjectTypeVariants(object):
         assert isinstance(self.tag_member.type, QAPISchemaEnumType)
         for v in self.variants:
             v.check(schema, self.tag_member.type)
+            if isinstance(v.type, QAPISchemaObjectType):
+                v.type.check(schema)
+
+    def check_clash(self, schema, seen):
+        for v in self.variants:
+            # Reset seen map for each variant, since qapi names from one
+            # branch do not affect another branch
+            vseen = dict(seen)
+            assert isinstance(v.type, QAPISchemaObjectType)
+            assert not v.type.variants       # not implemented
+            for m in v.type.members:
+                m.check_clash(vseen)


 class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
@@ -1086,6 +1099,8 @@ class QAPISchemaAlternateType(QAPISchemaType):

     def check(self, schema):
         self.variants.tag_member.check(schema)
+        # Not calling self.variants.check_clash(), because there's nothing
+        # to clash with
         self.variants.check(schema, {})

     def json_type(self):
-- 
2.4.3