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 <ebl...@redhat.com> --- 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
