Re: [Qemu-devel] [PATCH V7 01/11] qapi script: remember enum values

[Markus Armbruster]

Wenchao Xia <address@hidden> writes:

> Later other scripts will need to check the enum values.
>
> Signed-off-by: Wenchao Xia <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> ---
>  scripts/qapi.py                        |   18 ++++++++++++++----
>  tests/qapi-schema/comments.out         |    2 +-
>  tests/qapi-schema/qapi-schema-test.out |   10 +++++-----
>  3 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index f3c2a20..bd81f06 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -169,7 +169,7 @@ def parse_schema(fp):
>  
>      for expr in schema.exprs:
>          if expr.has_key('enum'):
> -            add_enum(expr['enum'])
> +            add_enum(expr['enum'], expr['data'])

This is an explicitly defined enum.

>          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'])

This is an implicitly defined enum.

> @@ -289,13 +289,23 @@ def find_union(name):
>              return union
>      return None
>  
> -def add_enum(name):
> +def add_enum(name, enum_values = None):
>      global enum_types
> -    enum_types.append(name)
> +    enum_types.append({"enum_name": name, "enum_values": enum_values})

You remember enum values only for the explicitly defined enums.  Let's
see how that works out later in this series.  In any case, mentioning it
in the commit message wouldn't hurt :)

> +
> +def find_enum(name):
> +    global enum_types
> +    for enum in enum_types:
> +        if enum['enum_name'] == name:
> +            return enum
> +    return None
>  
>  def is_enum(name):
>      global enum_types
> -    return (name in enum_types)
> +    for enum in enum_types:
> +        if enum['enum_name'] == name:
> +            return True
> +    return False

Duplicates find_enum()'s loop.  Consider simplifying to

   def is_enum(name):
       return find_enum(name) != None

>  
>  def c_type(name):
>      if name == 'str':
> diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
> index e3bd904..4ce3dcf 100644
> --- a/tests/qapi-schema/comments.out
> +++ b/tests/qapi-schema/comments.out
> @@ -1,3 +1,3 @@
>  [OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
> -['Status']
> +[{'enum_name': 'Status', 'enum_values': ['good', 'bad', 'ugly']}]
>  []
> diff --git a/tests/qapi-schema/qapi-schema-test.out 
> b/tests/qapi-schema/qapi-schema-test.out
> index 89b53d4..01685d4 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -15,11 +15,11 @@
>   OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 
> 'UserDefOne'), ('*ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')]),
>   OrderedDict([('command', 'user_def_cmd3'), ('data', OrderedDict([('a', 
> 'int'), ('*b', 'int')])), ('returns', 'int')]),
>   OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', 
> ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), 
> ('*u64x', 'uint64')]))])]
> -['EnumOne',
> - 'UserDefUnionKind',
> - 'UserDefFlatUnionKind',
> - 'UserDefAnonUnionKind',
> - 'UserDefNativeListUnionKind']
> +[{'enum_name': 'EnumOne', 'enum_values': ['value1', 'value2', 'value3']},

Explicitly defined enum with values.

> + {'enum_name': 'UserDefUnionKind', 'enum_values': None},
> + {'enum_name': 'UserDefFlatUnionKind', 'enum_values': None},
> + {'enum_name': 'UserDefAnonUnionKind', 'enum_values': None},
> + {'enum_name': 'UserDefNativeListUnionKind', 'enum_values': None}]

Four implicitly defined enums without values.

>  [OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 
> 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 
> 'EnumOne')]))]),
>   OrderedDict([('type', 'UserDefZero'), ('data', OrderedDict([('integer', 
> 'int')]))]),
>   OrderedDict([('type', 'UserDefOne'), ('base', 'UserDefZero'), ('data', 
> OrderedDict([('string', 'str'), ('*enum1', 'EnumOne')]))]),