Re: [Qemu-devel] [PATCH for-4.0 v7 08/27] qapi: add a dictionary form with 'name' key for enum members

[Markus Armbruster <armbru@redhat.com>]

One more thing...

Markus Armbruster <armbru@redhat.com> writes:

> Marc-André Lureau <marcandre.lureau@redhat.com> writes:
>
>> Desugar the enum NAME form to { 'name': NAME }. This will allow to add
>> new enum members, such as 'if' in the following patch.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  scripts/qapi/common.py                        | 49 ++++++++++++++++---
>>  tests/Makefile.include                        |  3 +-
>>  tests/qapi-schema/enum-bad-member.err         |  1 +
>>  ...-dict-member.exit => enum-bad-member.exit} |  0
>>  tests/qapi-schema/enum-bad-member.json        |  2 +
>>  ...um-dict-member.out => enum-bad-member.out} |  0
>>  .../qapi-schema/enum-dict-member-unknown.err  |  2 +
>>  .../qapi-schema/enum-dict-member-unknown.exit |  1 +
>>  .../qapi-schema/enum-dict-member-unknown.json |  2 +
>>  .../qapi-schema/enum-dict-member-unknown.out  |  0
>>  tests/qapi-schema/enum-dict-member.err        |  1 -
>>  tests/qapi-schema/enum-dict-member.json       |  2 -
>>  12 files changed, 52 insertions(+), 11 deletions(-)
>>  create mode 100644 tests/qapi-schema/enum-bad-member.err
>>  rename tests/qapi-schema/{enum-dict-member.exit => enum-bad-member.exit} (100%)
>>  create mode 100644 tests/qapi-schema/enum-bad-member.json
>>  rename tests/qapi-schema/{enum-dict-member.out => enum-bad-member.out} (100%)
>>  create mode 100644 tests/qapi-schema/enum-dict-member-unknown.err
>>  create mode 100644 tests/qapi-schema/enum-dict-member-unknown.exit
>>  create mode 100644 tests/qapi-schema/enum-dict-member-unknown.json
>>  create mode 100644 tests/qapi-schema/enum-dict-member-unknown.out
>>  delete mode 100644 tests/qapi-schema/enum-dict-member.err
>>  delete mode 100644 tests/qapi-schema/enum-dict-member.json
>>
>> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
[...]
>> @@ -847,18 +851,32 @@ def check_alternate(expr, info):
>>              types_seen[qt] = key
>>  
>>  
>> +def normalize_enum(expr):
>> +    members = expr['data']
>> +
>> +    # translate short member form to dict form
>> +    expr['data'] = [m if isinstance(m, dict) else {'name': m} for m in members]
>> +
>> +
>>  def check_enum(expr, info):
>>      name = expr['enum']
>> -    members = expr.get('data')
>> +    members = expr['data']
>>      prefix = expr.get('prefix')
>>  
>> -    if not isinstance(members, list):
>> -        raise QAPISemError(info,
>> -                           "Enum '%s' requires an array for 'data'" % name)
>>      if prefix is not None and not isinstance(prefix, str):
>>          raise QAPISemError(info,
>>                             "Enum '%s' requires a string for 'prefix'" % name)
>> +
>> +    if not isinstance(members, list):
>> +        raise QAPISemError(info,
>> +                           "Enum '%s' requires an array for 'data'" % name)
>> +
>
> Did you move this check down intentionally?
>
>>      for member in members:
>> +        if isinstance(member, dict):
>> +            source = "dictionary member of enum '%s'" % name
>> +            check_known_keys(info, source, member, ['name'], [])
>> +            member = member['name']
>> +
>>          check_name(info, "Member of enum '%s'" % name, member,
>>                     enum_member=True)
>
> Again, slightly more complicated than v6 because it now runs before
> normalization.  We'll revisit this at [*] below.

If we keep it this way, then let's at least avoid assigning to the loop
control variable:

                member_name = member['name']
            else:
                member_name = member
            check_name(info, "Member of enum '%s'" % name, member_name,
                       enum_member=True)

[...]