Re: [Qemu-devel] [PATCH V7 07/11] qapi script: support pre-defined enum type as discriminator in union

[Markus Armbruster]

Eric Blake <address@hidden> 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...