Re: [Qemu-devel] [PATCH v2 29/54] qapi: add 'if' to enum members

[Markus Armbruster]

One more thing...

Markus Armbruster <address@hidden> writes:

> Marc-André Lureau <address@hidden> writes:
>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>> ---
>>  scripts/qapi.py                         | 33 
>> +++++++++++++++++++++++++++------
>>  tests/qapi-schema/enum-dict-member.err  |  1 -
>>  tests/qapi-schema/enum-dict-member.exit |  2 +-
>>  tests/qapi-schema/enum-dict-member.json |  3 +--
>>  tests/qapi-schema/enum-dict-member.out  |  4 ++++
>>  tests/qapi-schema/qapi-schema-test.json |  5 +++--
>>  tests/qapi-schema/qapi-schema-test.out  |  3 ++-
>>  tests/qapi-schema/test-qapi.py          |  4 +++-
>>  8 files changed, 41 insertions(+), 14 deletions(-)
>>
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index 9d075440d3..9c7c01c11d 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -729,6 +729,10 @@ def check_event(expr, info):
>>                 allow_metas=meta)
>>  
>>  
>> +def enum_get_values(expr):
>> +    return [e if isinstance(e, str) else e['name'] for e in expr['data']]
>> +
>> +
>
> An iterator would be more efficient, but this will do.
>
>>  def check_union(expr, info):
>>      name = expr['union']
>>      base = expr.get('base')
>> @@ -788,7 +792,7 @@ def check_union(expr, info):
>>          # If the discriminator names an enum type, then all members
>>          # of 'data' must also be members of the enum type.
>>          if enum_define:
>> -            if key not in enum_define['data']:
>> +            if key not in enum_get_values(enum_define):
>>                  raise QAPISemError(info,
>>                                     "Discriminator value '%s' is not found 
>> in "
>>                                     "enum '%s'"
>> @@ -796,7 +800,7 @@ def check_union(expr, info):
>>  
>>      # If discriminator is user-defined, ensure all values are covered
>>      if enum_define:
>> -        for value in enum_define['data']:
>> +        for value in enum_get_values(enum_define):
>>              if value not in members.keys():
>>                  raise QAPISemError(info, "Union '%s' data missing '%s' 
>> branch"
>>                                     % (name, value))
>> @@ -827,7 +831,7 @@ def check_alternate(expr, info):
>>          if qtype == 'QTYPE_QSTRING':
>>              enum_expr = enum_types.get(value)
>>              if enum_expr:
>> -                for v in enum_expr['data']:
>> +                for v in enum_get_values(enum_expr):
>>                      if v in ['on', 'off']:
>>                          conflicting.add('QTYPE_QBOOL')
>>                      if re.match(r'[-+0-9.]', v): # lazy, could be tightened
>> @@ -857,6 +861,12 @@ def check_enum(expr, info):
>>          raise QAPISemError(info,
>>                             "Enum '%s' requires a string for 'prefix'" % 
>> name)
>>      for member in members:
>> +        if isinstance(member, dict):
>> +            if not 'name' in member:
>> +                raise QAPISemError(info, "Dictionary member of enum '%s' 
>> must "
>> +                                   "have a 'name' key" % name)
>> +            check_if(member, info)

Where are dictionary keys other than 'name' and 'if' rejected?

>> +            member = member['name']
>>          check_name(info, "Member of enum '%s'" % name, member,
>>                     enum_member=True)
>>  
>
> Looks like you got all uses of enum's 'data' in the old semantic
> checker.
>
> Aside: work like this is why I dislike how the old semantic checker
> works directly on the parse trees represented as OrderedDict.
> Everything is a dictionary, and almost everything's key is 'data'.
>
[...]