Eric Blake <[email protected]> writes: > 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.
Color me convinced. >> 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. Wenchao, if it's not too much trouble...
