On 02/21/2014 01:13 AM, Markus Armbruster wrote: >>> I guess you move this into its own loop because when base types are used >>> before they're defined, or an enum type is used for a discriminator >>> before it's defined, then discriminator_find_enum_define() complains. >>> Correct? >>> >> Exactly, which allow enum define after usage in schema. > > Do we want to (have to?) support "use before define" in schemas? Eric, > what do you think?
Topological sorting is a nice goal; and unfortunately not possible in qapi because we already have recursive types. We already have to support use before define in schemas. But it still seems like a two-pass parse is sufficient - in pass one, read all types, but without paying attention to their contents; in pass two, resolve all types in the order that they are encountered. Enums are not recursive, so it will always possible to resolve an enum before resolving the base class of a union, even if the enum definition occurred later than the union type that is using the enum as its discriminator. Now, just because we have to support use-before-define of recursive types does not necessarily mean that we have to support use-before-define of enums. Since enums are inherently not recursive, it might be okay to state that any use of a discriminator must be after the enum has already been declared. But for consistency, I think supporting use-before-define is nicer; I also think there may come a day where the schema file is so large that it would pay to do a one-time sort and make all further insertions in alphabetical order (to make it easier to find a given type name) - and I do not want to enforce that enum types must sort lexicographically before any client using it as a discriminator. > > If yes, we should add suitable tests. Outside the scope of this series. Here, I agree - whether you enforce define-before-use for now, or plan on supporting use-before-define, you need a test of an enum after the union type to ensure that it either gives a sane error message or does the right action. I actually think adding such a test IS part of the scope of this series, as this is the series adding support for an enum discriminator in the first place. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature