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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2378 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]