Re: [Qemu-devel] [PATCH v6 08/27] qapi: change enum visitor and gen_enum* to take QAPISchemaMember

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> This will allow to add and access more properties associated with enum
> values/members, like the associated 'if' condition. We may want to
> have a specialized type QAPISchemaEnumMember, for now this will do.

Well, we can add all we want without this patch.  The patch is about
enabling access in visit_enum_type() and ...

> While at it, also modify gen_enum() and gen_enum_lookup() for the
> same reason.

... these two helpers.  Worthwhile if later patches need such access.
We'll see.

> Suggested-by: Markus Armbruster <address@hidden>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  scripts/qapi/common.py         | 22 +++++++++++-----------
>  scripts/qapi/doc.py            |  2 +-
>  scripts/qapi/events.py         |  2 +-
>  scripts/qapi/introspect.py     |  5 +++--
>  scripts/qapi/types.py          |  6 +++---
>  scripts/qapi/visit.py          |  2 +-
>  tests/qapi-schema/test-qapi.py |  4 ++--
>  7 files changed, 22 insertions(+), 21 deletions(-)
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 7020b88abc..a353670079 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -1063,7 +1063,7 @@ class QAPISchemaVisitor(object):
>      def visit_builtin_type(self, name, info, json_type):
>          pass
>  
> -    def visit_enum_type(self, name, info, ifcond, values, prefix):
> +    def visit_enum_type(self, name, info, ifcond, members, prefix):
>          pass
>  
>      def visit_array_type(self, name, info, ifcond, element_type):
> @@ -1193,7 +1193,7 @@ class QAPISchemaEnumType(QAPISchemaType):
>  
>      def visit(self, visitor):
>          visitor.visit_enum_type(self.name, self.info, self.ifcond,
> -                                self.member_names(), self.prefix)
> +                                self.members, self.prefix)
>  
>  

Just two uses of QAPISchemaType.member_names() left.  See also below.

>  class QAPISchemaArrayType(QAPISchemaType):
> @@ -2012,19 +2012,19 @@ def _wrap_ifcond(ifcond, before, after):
>      return out
>  
>  
> -def gen_enum_lookup(name, values, prefix=None):
> +def gen_enum_lookup(name, members, prefix=None):
>      ret = mcgen('''
>  
>  const QEnumLookup %(c_name)s_lookup = {
>      .array = (const char *const[]) {
>  ''',
>                  c_name=c_name(name))
> -    for value in values:
> -        index = c_enum_const(name, value, prefix)
> +    for m in members:
> +        index = c_enum_const(name, m.name, prefix)
>          ret += mcgen('''
> -        [%(index)s] = "%(value)s",
> +        [%(index)s] = "%(name)s",
>  ''',
> -                     index=index, value=value)
> +                     index=index, name=m.name)
>  
>      ret += mcgen('''
>      },
> @@ -2035,9 +2035,9 @@ const QEnumLookup %(c_name)s_lookup = {
>      return ret
>  
>  
> -def gen_enum(name, values, prefix=None):
> +def gen_enum(name, members, prefix=None):
>      # append automatically generated _MAX value
> -    enum_values = values + ['_MAX']
> +    enum_members = members + [QAPISchemaMember('_MAX')]
>  
>      ret = mcgen('''
>  
> @@ -2045,11 +2045,11 @@ typedef enum %(c_name)s {
>  ''',
>                  c_name=c_name(name))
>  
> -    for value in enum_values:
> +    for m in enum_members:
>          ret += mcgen('''
>      %(c_enum)s,
>  ''',
> -                     c_enum=c_enum_const(name, value, prefix))
> +                     c_enum=c_enum_const(name, m.name, prefix))
>  
>      ret += mcgen('''
>  } %(c_name)s;
> diff --git a/scripts/qapi/doc.py b/scripts/qapi/doc.py
> index 987fd3c943..76cb186ff9 100755
> --- a/scripts/qapi/doc.py
> +++ b/scripts/qapi/doc.py
> @@ -206,7 +206,7 @@ class 
> QAPISchemaGenDocVisitor(qapi.common.QAPISchemaVisitor):
>      def write(self, output_dir):
>          self._gen.write(output_dir, self._prefix + 'qapi-doc.texi')
>  
> -    def visit_enum_type(self, name, info, ifcond, values, prefix):
> +    def visit_enum_type(self, name, info, ifcond, members, prefix):
>          doc = self.cur_doc
>          self._gen.add(TYPE_FMT(type='Enum',
>                                 name=doc.symbol,
> diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
> index 764ef177ab..ea4dac6a05 100644
> --- a/scripts/qapi/events.py
> +++ b/scripts/qapi/events.py
> @@ -189,7 +189,7 @@ class 
> QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor):
>              self._genh.add(gen_event_send_decl(name, arg_type, boxed))
>              self._genc.add(gen_event_send(name, arg_type, boxed,
>                                            self._enum_name))
> -        self._event_names.append(name)
> +        self._event_names.append(QAPISchemaMember(name))
>  
>  

Rename ._event_names to ._event_enum_members?

May want to rename .enum_name to ._event_enum_name then.

>  def gen_events(schema, output_dir, prefix):
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index 71d4a779ce..3f1ca99f6d 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -160,8 +160,9 @@ const QLitObject %(c_name)s = %(c_string)s;
>      def visit_builtin_type(self, name, info, json_type):
>          self._gen_qlit(name, 'builtin', {'json-type': json_type}, [])
>  
> -    def visit_enum_type(self, name, info, ifcond, values, prefix):
> -        self._gen_qlit(name, 'enum', {'values': values}, ifcond)
> +    def visit_enum_type(self, name, info, ifcond, members, prefix):
> +        self._gen_qlit(name, 'enum',
> +                       {'values': [m.name for m in members]}, ifcond)

Can't use QAPISchemaEnumType.member_names() here, since we don't have
the QAPISchemaEnumType, only its .members.

>  
>      def visit_array_type(self, name, info, ifcond, element_type):
>          element = self._use_type(element_type)
> diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
> index 91f87d0b8f..2d4a70f810 100644
> --- a/scripts/qapi/types.py
> +++ b/scripts/qapi/types.py
> @@ -213,10 +213,10 @@ class 
> QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
>          self._genh.add(gen_type_cleanup_decl(name))
>          self._genc.add(gen_type_cleanup(name))
>  
> -    def visit_enum_type(self, name, info, ifcond, values, prefix):
> +    def visit_enum_type(self, name, info, ifcond, members, prefix):
>          with ifcontext(ifcond, self._genh, self._genc):
> -            self._genh.preamble_add(gen_enum(name, values, prefix))
> -            self._genc.add(gen_enum_lookup(name, values, prefix))
> +            self._genh.preamble_add(gen_enum(name, members, prefix))
> +            self._genc.add(gen_enum_lookup(name, members, prefix))
>  
>      def visit_array_type(self, name, info, ifcond, element_type):
>          with ifcontext(ifcond, self._genh, self._genc):
> diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
> index 460cf12989..24f85a2e85 100644
> --- a/scripts/qapi/visit.py
> +++ b/scripts/qapi/visit.py
> @@ -310,7 +310,7 @@ class 
> QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
>  ''',
>                                        types=types))
>  
> -    def visit_enum_type(self, name, info, ifcond, values, prefix):
> +    def visit_enum_type(self, name, info, ifcond, members, prefix):
>          with ifcontext(ifcond, self._genh, self._genc):
>              self._genh.add(gen_visit_decl(name, scalar=True))
>              self._genc.add(gen_visit_enum(name))
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index cea21c773a..27f776693e 100644
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -23,8 +23,8 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
>      def visit_include(self, name, info):
>          print('include %s' % name)
>  
> -    def visit_enum_type(self, name, info, ifcond, values, prefix):
> -        print('enum %s %s' % (name, values))
> +    def visit_enum_type(self, name, info, ifcond, members, prefix):
> +        print('enum %s %s' % (name, [m.name for m in members]))
>          if prefix:
>              print('    prefix %s' % prefix)
>          self._print_if(ifcond)

Likewise.

Perhaps we can get rid of .member_names().  Not this patch's business,
of course.

Assuming later patches make use of this:
Reviewed-by: Markus Armbruster <address@hidden>