[Qemu-devel] [PATCH v9 19/27] 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 QAPISchema*.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 passed
in to QAPISchemaObjectTypeVariants.check(), which we clone for
each particular variant (since the members of one variant do
not clash with another).  So all that is additionally needed
is to actually check the each member of the variant type do
not add any collisions.

In general, a 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).

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 branch on whether seen is empty to distinguish whether we
are checking a union or an alternate.

No change to generated code.

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

---
v9: new patch, split off from v8 7/17
---
 scripts/qapi.py | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/scripts/qapi.py b/scripts/qapi.py
index a20abda..3054628 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -1057,8 +1057,9 @@ 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 qapi names from one
+            # branch do not affect another branch
+            v.check(schema, self.tag_member.type, dict(seen))


 class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
@@ -1068,6 +1069,14 @@ class 
QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember):
     def check(self, schema, tag_type, seen):
         QAPISchemaObjectTypeMember.check(self, schema)
         assert self.name in tag_type.values
+        if seen:
+            # This variant is used within a union; ensure each qapi member
+            # field does not collide with the union's non-variant members.
+            assert isinstance(self.type, QAPISchemaObjectType)
+            assert not self.type.variants       # not implemented
+            self.type.check(schema)
+            for m in self.type.members:
+                m.check_clash(seen)

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