Eric Blake <ebl...@redhat.com> writes: > On 03/08/2016 03:12 AM, Markus Armbruster wrote: >> Eric Blake <ebl...@redhat.com> writes: >> >>> We are getting closer to the point where we could use one union >>> as the base or variant type within another union type (as long >>> as there are no collisions between any possible combination of >>> member names allowed across all discriminator choices). But >>> until we get to that point, it is worth asserting that variants >>> are not present in places where we are not prepared to handle >>> them: base types must still be plain structs, and anywhere we >>> explode a struct into a parameter list (events and command >>> marshalling), we don't support variants in that explosion. >>> >>> Signed-off-by: Eric Blake <ebl...@redhat.com> >>> > >>> +++ b/scripts/qapi.py >>> @@ -960,6 +960,7 @@ class QAPISchemaObjectType(QAPISchemaType): >>> assert isinstance(self.base, QAPISchemaObjectType) >>> self.base.check(schema) >>> self.base.check_clash(schema, self.info, seen) >>> + assert not self.base.variants >> >> I'd move this two lines up, so it's next to the isinstance. >> >> Assertions in .check() are place-holders for semantic checks that >> haven't been moved from the old semantic analysis to the classes. >> Whenever we add one, we should double-check the old semantic analysis >> catches whatever we assert. For object types, that's check_struct() and >> check_union(). Both check_type() the base with allow_metas=['struct']), >> so we're good. >> >> Inconsistency: you add the check for base, but not for variants. >> >> On closer look, adding it for either is actually redundant, because >> se.f.base.check_clash() already asserts it, with a nice "not >> implemented" comment. >> >> If we think asserting twice is useful for base, then it's useful for >> variants, too. But I think asserting once suffices. > > So basically, we can drop this hunk, right?
Yes.
