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? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
