Re: [Qemu-devel] [PATCH v8 06/22] qapi: add a dictionary form with 'name' key for enum members

[Marc-André Lureau]

On Thu, Dec 13, 2018 at 6:32 PM Markus Armbruster <address@hidden> wrote:
>
> One little thing...
>
> Marc-André Lureau <address@hidden> 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 <address@hidden>
> > ---
> [...]
> > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> > index f205805751..bb928d2f7d 100644
> > --- a/scripts/qapi/common.py
> > +++ b/scripts/qapi/common.py
> [...]
> > @@ -847,19 +851,29 @@ def check_alternate(expr, info):
> >              types_seen[qt] = key
> >
> >
> > +def normalize_enum(expr):
> > +    if isinstance(expr['data'], list):
> > +        expr['data'] = [m if isinstance(m, dict) else {'name': m}
> > +                        for m in expr['data']]
> > +
> > +
> >  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)
> > +
>
> The move of the "requires an array" check looks accidental.  I'll back
> it out unless you tell me it's needed.

ack

>
> >      for member in members:
> > -        check_name(info, "Member of enum '%s'" % name, member,
> > +        source = "dictionary member of enum '%s'" % name
> > +        check_known_keys(info, source, member, ['name'], [])
> > +        check_name(info, "Member of enum '%s'" % name, member['name'],
> >                     enum_member=True)
> >
> >
> [...]