On 03/08/2016 09:23 AM, Markus Armbruster wrote: > Eric Blake <ebl...@redhat.com> writes: > >> Rather than requiring all flat unions to explicitly create >> a separate base struct, we can allow the qapi schema to specify >> the common members via an inline dictionary. This is similar to >> how commands can specify an inline anonymous type for its 'data'. >> We already have several struct types that only exist to serve as >> a single flat union's base; the next commit will clean them up. >>
> > I think you also > > * fixed a missing allow_optional=True in check_union() More on that below. > > * fixed a missing "non-optional" in qapi-code-gen.txt (mention in commit > message? separate patch?) > > * renamed readonly to read-only in an example in qapi-code-gen.txt to be > closer to the code (separate patch?) Could separate those two cleanups if it helps. >> @@ -560,9 +562,10 @@ def check_union(expr, expr_info): >> >> # Else, it's a flat union. >> else: >> - # The object must have a string member 'base'. >> + # The object must have a string or dictionary 'base'. >> check_type(expr_info, "'base' for union '%s'" % name, >> - base, allow_metas=['struct']) >> + base, allow_dict=True, allow_optional=True, >> + allow_metas=['struct']) > > This is where you added allow_optional=True. allow_optional only matters if allow_dict is True. We have places where we allow a dict but do not allow optionals (namely, simple unions); but where we are creating an anonymous type, we want to allow optionals at the same time we allow a dict. I think this is just a case where the commit message needs to be beefed up. >> +A flat union definition avoids nesting on the wire, and specifies a >> +set of common members that occur in all variants of the union. The >> +'base' key must specifiy either a type name (the type must be a >> +struct, not a union), or a dictionary representing an anonymous type. >> +All branches of the union must be complex types, and the top-level >> +members of the union dictionary on the wire will be combination of >> +members from both the base type and the appropriate branch type (when >> +merging two dictionaries, there must be no keys in common). The >> +'discriminator' member must be the name of a non-optional enum-typed > > This is where you supplied the missing "non-optional". > >> +member of the base struct. >> > > And below, you rename readonly to read-only. They touch the same paragraph, but I can separate them if it would make this patch feel cleaner. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
