[V7,07/11] qapi script: support pre-defined enum type as discriminator in union

By default, any union will automatically generate a enum type as
"[UnionName]Kind" in C code, and it is duplicated when the discriminator
is specified as a pre-defined enum type in schema. After this patch,
the pre-defined enum type will be really used as the switch case
condition in generated C code, if discriminator is an enum field.

Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
---
 docs/qapi-code-gen.txt |    8 ++++++--
 scripts/qapi-types.py  |   20 ++++++++++++++++----
 scripts/qapi-visit.py  |   27 ++++++++++++++++++++-------
 scripts/qapi.py        |   13 ++++++++++++-
 4 files changed, 54 insertions(+), 14 deletions(-)

Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

> By default, any union will automatically generate a enum type as
> "[UnionName]Kind" in C code, and it is duplicated when the discriminator
> is specified as a pre-defined enum type in schema. After this patch,
> the pre-defined enum type will be really used as the switch case
> condition in generated C code, if discriminator is an enum field.
>
> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
> ---
>  docs/qapi-code-gen.txt |    8 ++++++--
>  scripts/qapi-types.py  |   20 ++++++++++++++++----
>  scripts/qapi-visit.py  |   27 ++++++++++++++++++++-------
>  scripts/qapi.py        |   13 ++++++++++++-
>  4 files changed, 54 insertions(+), 14 deletions(-)
>
> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
> index 0728f36..a2e7921 100644
> --- a/docs/qapi-code-gen.txt
> +++ b/docs/qapi-code-gen.txt
> @@ -123,11 +123,15 @@ And it looks like this on the wire:
>  
>  Flat union types avoid the nesting on the wire. They are used whenever a
>  specific field of the base type is declared as the discriminator ('type' is
> -then no longer generated). The discriminator must always be a string field.
> +then no longer generated). The discriminator can be a string field or a
> +predefined enum field. If it is a string field, a hidden enum type will be
> +generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
> +will be done to verify the correctness. It is recommended to use an enum field.
>  The above example can then be modified as follows:
>  
> + { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
>   { 'type': 'BlockdevCommonOptions',
> -   'data': { 'driver': 'str', 'readonly': 'bool' } }
> +   'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
>   { 'union': 'BlockdevOptions',
>     'base': 'BlockdevCommonOptions',
>     'discriminator': 'driver',
> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> index 656a9a0..4098c60 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -201,14 +201,22 @@ def generate_union(expr):
>      base = expr.get('base')
>      discriminator = expr.get('discriminator')
>  
> +    expr_elem = {'expr': expr}
> +    enum_define = discriminator_find_enum_define(expr_elem)

expr_elem has no fp, line.  What if discriminator_find_enum_define
throws a QAPIExprError?

More of the same below.

> +    if enum_define:
> +        discriminator_type_name = enum_define['enum_name']
> +    else:
> +        discriminator_type_name = '%sKind' % (name)
> +
>      ret = mcgen('''
>  struct %(name)s
>  {
> -    %(name)sKind kind;
> +    %(discriminator_type_name)s kind;
>      union {
>          void *data;
>  ''',
> -                name=name)
> +                name=name,
> +                discriminator_type_name=discriminator_type_name)
>  
>      for key in typeinfo:
>          ret += mcgen('''
> @@ -389,8 +397,12 @@ for expr in exprs:
>          fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
>      elif expr.has_key('union'):
>          ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
> -        ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
> -        fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys()))
> +        expr_elem = {'expr': expr}
> +        enum_define = discriminator_find_enum_define(expr_elem)
> +        if not enum_define:
> +            ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
> +            fdef.write(generate_enum_lookup('%sKind' % expr['union'],
> +                                            expr['data'].keys()))

Generate the implicit enum only when we don't have an explicit enum
discriminator.  Good.

>          if expr.get('discriminator') == {}:
>              fdef.write(generate_anon_union_qtypes(expr))
>      else:
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 87e6df7..08685a7 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -260,10 +260,17 @@ def generate_visit_union(expr):
>          assert not base
>          return generate_visit_anon_union(name, members)
>  
> -    # There will always be a discriminator in the C switch code, by default it
> -    # is an enum type generated silently as "'%sKind' % (name)"
> -    ret = generate_visit_enum('%sKind' % name, members.keys())
> -    discriminator_type_name = '%sKind' % (name)
> +    expr_elem = {'expr': expr}
> +    enum_define = discriminator_find_enum_define(expr_elem)
> +    if enum_define:
> +        # Use the predefined enum type as discriminator
> +        ret = ""
> +        discriminator_type_name = enum_define['enum_name']
> +    else:
> +        # There will always be a discriminator in the C switch code, by default it
> +        # is an enum type generated silently as "'%sKind' % (name)"
> +        ret = generate_visit_enum('%sKind' % name, members.keys())
> +        discriminator_type_name = '%sKind' % (name)

Generate the visit of the discriminator only when we don't have an
explicit enum discriminator (which gets visited elsewhere already).
Good.

>  
>      if base:
>          base_fields = find_struct(base)['data']
> @@ -303,11 +310,12 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
>      else:
>          desc_type = discriminator
>      ret += mcgen('''
> -        visit_type_%(name)sKind(m, &(*obj)->kind, "%(type)s", &err);
> +        visit_type_%(discriminator_type_name)s(m, &(*obj)->kind, "%(type)s", &err);

Another long line due to very long identifier.

>          if (!err) {
>              switch ((*obj)->kind) {
>  ''',
> -                 name=name, type=desc_type)
> +                 discriminator_type_name=discriminator_type_name,
> +                 type=desc_type)
>  
>      for key in members:
>          if not discriminator:
> @@ -519,7 +527,12 @@ for expr in exprs:
>          ret += generate_visit_list(expr['union'], expr['data'])
>          fdef.write(ret)
>  
> -        ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
> +        expr_elem = {'expr': expr}
> +        enum_define = discriminator_find_enum_define(expr_elem)
> +        ret = ""
> +        if not enum_define:
> +            ret = generate_decl_enum('%sKind' % expr['union'],
> +                                     expr['data'].keys())

Generate the visitor for the implicit enum only when we don't have an
explicit enum discriminator (which has its own visitor already).  Good.

>          ret += generate_declaration(expr['union'], expr['data'])
>          fdecl.write(ret)
>      elif expr.has_key('enum'):
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 130dced..2a5eb59 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -250,11 +250,22 @@ def parse_schema(fp):
>              add_enum(expr['enum'], expr['data'])
>          elif expr.has_key('union'):
>              add_union(expr)
> -            add_enum('%sKind' % expr['union'])
>          elif expr.has_key('type'):
>              add_struct(expr)
>          exprs.append(expr)
>  
> +    # Try again for hidden UnionKind enum
> +    for expr_elem in schema.exprs:
> +        expr = expr_elem['expr']
> +        if expr.has_key('union'):
> +            try:
> +                enum_define = discriminator_find_enum_define(expr_elem)
> +            except QAPIExprError, e:
> +                print >>sys.stderr, e
> +                exit(1)
> +            if not enum_define:
> +                add_enum('%sKind' % expr['union'])
> +
>      try:
>          check_exprs(schema)
>      except QAPIExprError, e:

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?

于 2014/2/21 0:38, Markus Armbruster 写道:
> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
> 
>> By default, any union will automatically generate a enum type as
>> "[UnionName]Kind" in C code, and it is duplicated when the discriminator
>> is specified as a pre-defined enum type in schema. After this patch,
>> the pre-defined enum type will be really used as the switch case
>> condition in generated C code, if discriminator is an enum field.
>>
>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>> ---
>>   docs/qapi-code-gen.txt |    8 ++++++--
>>   scripts/qapi-types.py  |   20 ++++++++++++++++----
>>   scripts/qapi-visit.py  |   27 ++++++++++++++++++++-------
>>   scripts/qapi.py        |   13 ++++++++++++-
>>   4 files changed, 54 insertions(+), 14 deletions(-)
>>
>> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
>> index 0728f36..a2e7921 100644
>> --- a/docs/qapi-code-gen.txt
>> +++ b/docs/qapi-code-gen.txt
>> @@ -123,11 +123,15 @@ And it looks like this on the wire:
>>   
>>   Flat union types avoid the nesting on the wire. They are used whenever a
>>   specific field of the base type is declared as the discriminator ('type' is
>> -then no longer generated). The discriminator must always be a string field.
>> +then no longer generated). The discriminator can be a string field or a
>> +predefined enum field. If it is a string field, a hidden enum type will be
>> +generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
>> +will be done to verify the correctness. It is recommended to use an enum field.
>>   The above example can then be modified as follows:
>>   
>> + { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
>>    { 'type': 'BlockdevCommonOptions',
>> -   'data': { 'driver': 'str', 'readonly': 'bool' } }
>> +   'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
>>    { 'union': 'BlockdevOptions',
>>      'base': 'BlockdevCommonOptions',
>>      'discriminator': 'driver',
>> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
>> index 656a9a0..4098c60 100644
>> --- a/scripts/qapi-types.py
>> +++ b/scripts/qapi-types.py
>> @@ -201,14 +201,22 @@ def generate_union(expr):
>>       base = expr.get('base')
>>       discriminator = expr.get('discriminator')
>>   
>> +    expr_elem = {'expr': expr}
>> +    enum_define = discriminator_find_enum_define(expr_elem)
> 
> expr_elem has no fp, line.  What if discriminator_find_enum_define
> throws a QAPIExprError?
> 
  It shouldn't happen, since all error check happens in parse_schema().

> More of the same below.
> 
>> +    if enum_define:
>> +        discriminator_type_name = enum_define['enum_name']
>> +    else:
>> +        discriminator_type_name = '%sKind' % (name)
>> +
>>       ret = mcgen('''
>>   struct %(name)s
>>   {
>> -    %(name)sKind kind;
>> +    %(discriminator_type_name)s kind;
>>       union {
>>           void *data;
>>   ''',
>> -                name=name)
>> +                name=name,
>> +                discriminator_type_name=discriminator_type_name)
>>   
>>       for key in typeinfo:
>>           ret += mcgen('''
>> @@ -389,8 +397,12 @@ for expr in exprs:
>>           fdef.write(generate_enum_lookup(expr['enum'], expr['data']))
>>       elif expr.has_key('union'):
>>           ret += generate_fwd_struct(expr['union'], expr['data']) + "\n"
>> -        ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
>> -        fdef.write(generate_enum_lookup('%sKind' % expr['union'], expr['data'].keys()))
>> +        expr_elem = {'expr': expr}
>> +        enum_define = discriminator_find_enum_define(expr_elem)
>> +        if not enum_define:
>> +            ret += generate_enum('%sKind' % expr['union'], expr['data'].keys())
>> +            fdef.write(generate_enum_lookup('%sKind' % expr['union'],
>> +                                            expr['data'].keys()))
> 
> Generate the implicit enum only when we don't have an explicit enum
> discriminator.  Good.
> 
>>           if expr.get('discriminator') == {}:
>>               fdef.write(generate_anon_union_qtypes(expr))
>>       else:
>> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
>> index 87e6df7..08685a7 100644
>> --- a/scripts/qapi-visit.py
>> +++ b/scripts/qapi-visit.py
>> @@ -260,10 +260,17 @@ def generate_visit_union(expr):
>>           assert not base
>>           return generate_visit_anon_union(name, members)
>>   
>> -    # There will always be a discriminator in the C switch code, by default it
>> -    # is an enum type generated silently as "'%sKind' % (name)"
>> -    ret = generate_visit_enum('%sKind' % name, members.keys())
>> -    discriminator_type_name = '%sKind' % (name)
>> +    expr_elem = {'expr': expr}
>> +    enum_define = discriminator_find_enum_define(expr_elem)
>> +    if enum_define:
>> +        # Use the predefined enum type as discriminator
>> +        ret = ""
>> +        discriminator_type_name = enum_define['enum_name']
>> +    else:
>> +        # There will always be a discriminator in the C switch code, by default it
>> +        # is an enum type generated silently as "'%sKind' % (name)"
>> +        ret = generate_visit_enum('%sKind' % name, members.keys())
>> +        discriminator_type_name = '%sKind' % (name)
> 
> Generate the visit of the discriminator only when we don't have an
> explicit enum discriminator (which gets visited elsewhere already).
> Good.
> 
>>   
>>       if base:
>>           base_fields = find_struct(base)['data']
>> @@ -303,11 +310,12 @@ void visit_type_%(name)s(Visitor *m, %(name)s ** obj, const char *name, Error **
>>       else:
>>           desc_type = discriminator
>>       ret += mcgen('''
>> -        visit_type_%(name)sKind(m, &(*obj)->kind, "%(type)s", &err);
>> +        visit_type_%(discriminator_type_name)s(m, &(*obj)->kind, "%(type)s", &err);
> 
> Another long line due to very long identifier.
> 
  Will improve.

>>           if (!err) {
>>               switch ((*obj)->kind) {
>>   ''',
>> -                 name=name, type=desc_type)
>> +                 discriminator_type_name=discriminator_type_name,
>> +                 type=desc_type)
>>   
>>       for key in members:
>>           if not discriminator:
>> @@ -519,7 +527,12 @@ for expr in exprs:
>>           ret += generate_visit_list(expr['union'], expr['data'])
>>           fdef.write(ret)
>>   
>> -        ret = generate_decl_enum('%sKind' % expr['union'], expr['data'].keys())
>> +        expr_elem = {'expr': expr}
>> +        enum_define = discriminator_find_enum_define(expr_elem)
>> +        ret = ""
>> +        if not enum_define:
>> +            ret = generate_decl_enum('%sKind' % expr['union'],
>> +                                     expr['data'].keys())
> 
> Generate the visitor for the implicit enum only when we don't have an
> explicit enum discriminator (which has its own visitor already).  Good.
> 
>>           ret += generate_declaration(expr['union'], expr['data'])
>>           fdecl.write(ret)
>>       elif expr.has_key('enum'):
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index 130dced..2a5eb59 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -250,11 +250,22 @@ def parse_schema(fp):
>>               add_enum(expr['enum'], expr['data'])
>>           elif expr.has_key('union'):
>>               add_union(expr)
>> -            add_enum('%sKind' % expr['union'])
>>           elif expr.has_key('type'):
>>               add_struct(expr)
>>           exprs.append(expr)
>>   
>> +    # Try again for hidden UnionKind enum
>> +    for expr_elem in schema.exprs:
>> +        expr = expr_elem['expr']
>> +        if expr.has_key('union'):
>> +            try:
>> +                enum_define = discriminator_find_enum_define(expr_elem)
>> +            except QAPIExprError, e:
>> +                print >>sys.stderr, e
>> +                exit(1)
>> +            if not enum_define:
>> +                add_enum('%sKind' % expr['union'])
>> +
>>       try:
>>           check_exprs(schema)
>>       except QAPIExprError, e:
> 
> 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.

Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:

> 于 2014/2/21 0:38, Markus Armbruster 写道:
>> Wenchao Xia <xiawenc@linux.vnet.ibm.com> writes:
>> 
>>> By default, any union will automatically generate a enum type as
>>> "[UnionName]Kind" in C code, and it is duplicated when the discriminator
>>> is specified as a pre-defined enum type in schema. After this patch,
>>> the pre-defined enum type will be really used as the switch case
>>> condition in generated C code, if discriminator is an enum field.
>>>
>>> Signed-off-by: Wenchao Xia <xiawenc@linux.vnet.ibm.com>
>>> ---
>>>   docs/qapi-code-gen.txt |    8 ++++++--
>>>   scripts/qapi-types.py  |   20 ++++++++++++++++----
>>>   scripts/qapi-visit.py  |   27 ++++++++++++++++++++-------
>>>   scripts/qapi.py        |   13 ++++++++++++-
>>>   4 files changed, 54 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt
>>> index 0728f36..a2e7921 100644
>>> --- a/docs/qapi-code-gen.txt
>>> +++ b/docs/qapi-code-gen.txt
>>> @@ -123,11 +123,15 @@ And it looks like this on the wire:
>>>   
>>>   Flat union types avoid the nesting on the wire. They are used whenever a
>>>   specific field of the base type is declared as the discriminator ('type' is
>>> -then no longer generated). The discriminator must always be a string field.
>>> +then no longer generated). The discriminator can be a string field or a
>>> +predefined enum field. If it is a string field, a hidden enum type will be
>>> +generated as "[UNION_NAME]Kind". If it is an enum field, a compile time check
>>> +will be done to verify the correctness. It is recommended to use an enum field.
>>>   The above example can then be modified as follows:
>>>   
>>> + { 'enum': 'BlockdevDriver', 'data': [ 'raw', 'qcow2' ] }
>>>    { 'type': 'BlockdevCommonOptions',
>>> -   'data': { 'driver': 'str', 'readonly': 'bool' } }
>>> +   'data': { 'driver': 'BlockdevDriver', 'readonly': 'bool' } }
>>>    { 'union': 'BlockdevOptions',
>>>      'base': 'BlockdevCommonOptions',
>>>      'discriminator': 'driver',
>>> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
>>> index 656a9a0..4098c60 100644
>>> --- a/scripts/qapi-types.py
>>> +++ b/scripts/qapi-types.py
>>> @@ -201,14 +201,22 @@ def generate_union(expr):
>>>       base = expr.get('base')
>>>       discriminator = expr.get('discriminator')
>>>   
>>> +    expr_elem = {'expr': expr}
>>> +    enum_define = discriminator_find_enum_define(expr_elem)
>> 
>> expr_elem has no fp, line.  What if discriminator_find_enum_define
>> throws a QAPIExprError?
>> 
>   It shouldn't happen, since all error check happens in parse_schema().

Impossible errors often mean the abstractions aren't quite right.  But
this series is progress, and I don't want to delay it by demanding
perfection.  We can improve on it in tree if we want.

>> More of the same below.
[...]
>>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>>> index 130dced..2a5eb59 100644
>>> --- a/scripts/qapi.py
>>> +++ b/scripts/qapi.py
>>> @@ -250,11 +250,22 @@ def parse_schema(fp):
>>>               add_enum(expr['enum'], expr['data'])
>>>           elif expr.has_key('union'):
>>>               add_union(expr)
>>> -            add_enum('%sKind' % expr['union'])
>>>           elif expr.has_key('type'):
>>>               add_struct(expr)
>>>           exprs.append(expr)
>>>   
>>> +    # Try again for hidden UnionKind enum
>>> +    for expr_elem in schema.exprs:
>>> +        expr = expr_elem['expr']
>>> +        if expr.has_key('union'):
>>> +            try:
>>> +                enum_define = discriminator_find_enum_define(expr_elem)
>>> +            except QAPIExprError, e:
>>> +                print >>sys.stderr, e
>>> +                exit(1)
>>> +            if not enum_define:
>>> +                add_enum('%sKind' % expr['union'])
>>> +
>>>       try:
>>>           check_exprs(schema)
>>>       except QAPIExprError, e:
>> 
>> 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?

If yes, we should add suitable tests.  Outside the scope of this series.

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> 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...