Re: [Qemu-devel] [PATCH v3 18/50] qapi: change enum visitor 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.
>
> Suggested-by: Markus Armbruster <address@hidden>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  scripts/qapi.py                          | 40 +++++++++++++-------------
>  scripts/qapi-event.py                    |  2 +-
>  scripts/qapi-introspect.py               |  5 ++--
>  scripts/qapi-types.py                    | 10 +++----
>  scripts/qapi-visit.py                    |  2 +-
>  scripts/qapi2texi.py                     |  2 +-
>  tests/qapi-schema/comments.out           | 14 +++++++--
>  tests/qapi-schema/doc-good.out           | 17 +++++++++--
>  tests/qapi-schema/empty.out              |  9 +++++-
>  tests/qapi-schema/event-case.out         |  9 +++++-
>  tests/qapi-schema/ident-with-escape.out  |  9 +++++-
>  tests/qapi-schema/include-relpath.out    | 14 +++++++--
>  tests/qapi-schema/include-repetition.out | 14 +++++++--
>  tests/qapi-schema/include-simple.out     | 14 +++++++--
>  tests/qapi-schema/indented-expr.out      |  9 +++++-
>  tests/qapi-schema/qapi-schema-test.out   | 49 
> ++++++++++++++++++++++++++------
>  tests/qapi-schema/test-qapi.py           | 17 +++++++----
>  17 files changed, 177 insertions(+), 59 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 074ee221a1..386a577a59 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1039,7 +1039,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):
> @@ -1127,21 +1127,21 @@ class QAPISchemaBuiltinType(QAPISchemaType):
>  
>  
>  class QAPISchemaEnumType(QAPISchemaType):
> -    def __init__(self, name, info, doc, ifcond, values, prefix):
> +    def __init__(self, name, info, doc, ifcond, members, prefix):
>          QAPISchemaType.__init__(self, name, info, doc, ifcond)
> -        for v in values:
> -            assert isinstance(v, QAPISchemaMember)
> -            v.set_owner(name)
> +        for m in members:
> +            assert isinstance(m, QAPISchemaMember)
> +            m.set_owner(name)
>          assert prefix is None or isinstance(prefix, str)
> -        self.values = values
> +        self.members = members
>          self.prefix = prefix
>  
>      def check(self, schema):
>          seen = {}
> -        for v in self.values:
> -            v.check_clash(self.info, seen)
> +        for m in self.members:
> +            m.check_clash(self.info, seen)
>              if self.doc:
> -                self.doc.connect_member(v)
> +                self.doc.connect_member(m)
>  
>      def is_implicit(self):
>          # See QAPISchema._make_implicit_enum_type() and ._def_predefineds()
> @@ -1151,14 +1151,14 @@ class QAPISchemaEnumType(QAPISchemaType):
>          return c_name(self.name)
>  
>      def member_names(self):
> -        return [v.name for v in self.values]
> +        return [m.name for m in self.members]
>  
>      def json_type(self):
>          return 'string'
>  
>      def visit(self, visitor):
>          visitor.visit_enum_type(self.name, self.info, self.ifcond,
> -                                self.member_names(), self.prefix)
> +                                self.members, self.prefix)
>  
>  
>  class QAPISchemaArrayType(QAPISchemaType):
> @@ -1952,19 +1952,19 @@ def ifcond_decorator(func):
>      return func_wrapper
>  
>  
> -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('''
>      },
> @@ -1975,9 +1975,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('''
>  
> @@ -1985,11 +1985,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-event.py b/scripts/qapi-event.py
> index bef301dfe9..38f4264817 100644
> --- a/scripts/qapi-event.py
> +++ b/scripts/qapi-event.py
> @@ -168,7 +168,7 @@ class QAPISchemaGenEventVisitor(QAPISchemaVisitor):
>      def visit_event(self, name, info, ifcond, arg_type, boxed):
>          self.decl += gen_event_send_decl(name, arg_type, boxed)
>          self.defn += gen_event_send(name, arg_type, boxed)
> -        self._event_names.append(name)
> +        self._event_names.append(QAPISchemaMember(name))
>  
>  
>  (input_file, output_dir, do_c, do_h, prefix, dummy) = parse_command_line()
> diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
> index 69d9afc792..32a58cf879 100644
> --- a/scripts/qapi-introspect.py
> +++ b/scripts/qapi-introspect.py
> @@ -151,8 +151,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}, None)
>  
> -    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)
>  
>      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 789e89ff59..75c1823e44 100644
> --- a/scripts/qapi-types.py
> +++ b/scripts/qapi-types.py
> @@ -203,16 +203,16 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>          self.defn += gen_type_cleanup(name)
>  
>      @ifcond_decorator
> -    def visit_enum_type(self, name, info, ifcond, values, prefix):
> +    def visit_enum_type(self, name, info, ifcond, members, prefix):
>          # Special case for our lone builtin enum type
>          # TODO use something cleaner than existence of info
>          if not info:
> -            self._btin += gen_enum(name, values, prefix)
> +            self._btin += gen_enum(name, members, prefix)
>              if do_builtins:
> -                self.defn += gen_enum_lookup(name, values, prefix)
> +                self.defn += gen_enum_lookup(name, members, prefix)
>          else:
> -            self._fwdecl += gen_enum(name, values, prefix)
> -            self.defn += gen_enum_lookup(name, values, prefix)
> +            self._fwdecl += gen_enum(name, members, prefix)
> +            self.defn += gen_enum_lookup(name, members, prefix)
>  
>      @ifcond_decorator
>      def visit_array_type(self, name, info, ifcond, element_type):
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 4b0e005437..7e816ae98e 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -284,7 +284,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor):
>          self._btin = None
>  
>      @ifcond_decorator
> -    def visit_enum_type(self, name, info, ifcond, values, prefix):
> +    def visit_enum_type(self, name, info, ifcond, members, prefix):
>          # Special case for our lone builtin enum type
>          # TODO use something cleaner than existence of info
>          if not info:
> diff --git a/scripts/qapi2texi.py b/scripts/qapi2texi.py
> index cf63cb0006..e72e7cfe0b 100755
> --- a/scripts/qapi2texi.py
> +++ b/scripts/qapi2texi.py
> @@ -207,7 +207,7 @@ class QAPISchemaGenDocVisitor(qapi.QAPISchemaVisitor):
>      def visit_begin(self, schema):
>          self.out = ''
>  
> -    def visit_enum_type(self, name, info, ifcond, values, prefix):
> +    def visit_enum_type(self, name, info, ifcond, members, prefix):
>          doc = self.cur_doc
>          if self.out:
>              self.out += '\n'
> diff --git a/tests/qapi-schema/comments.out b/tests/qapi-schema/comments.out
> index 17e652535c..17b493ec24 100644
> --- a/tests/qapi-schema/comments.out
> +++ b/tests/qapi-schema/comments.out
> @@ -1,4 +1,14 @@
> -enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> +enum QType
>      prefix QTYPE
> -enum Status ['good', 'bad', 'ugly']
> +    member none:
> +    member qnull:
> +    member qnum:
> +    member qstring:
> +    member qdict:
> +    member qlist:
> +    member qbool:
> +enum Status
> +    member good:
> +    member bad:
> +    member ugly:
>  object q_empty
> diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
> index 63ca25a8b9..0de06ce345 100644
> --- a/tests/qapi-schema/doc-good.out
> +++ b/tests/qapi-schema/doc-good.out
> @@ -1,19 +1,30 @@
>  object Base
>      member base1: Enum optional=False
> -enum Enum ['one', 'two']
> +enum Enum
> +    member one:
> +    member two:
>  object Object
>      base Base
>      tag base1
>      case one: Variant1
>      case two: Variant2
> -enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> +enum QType
>      prefix QTYPE
> +    member none:
> +    member qnull:
> +    member qnum:
> +    member qstring:
> +    member qdict:
> +    member qlist:
> +    member qbool:
>  object SugaredUnion
>      member type: SugaredUnionKind optional=False
>      tag type
>      case one: q_obj_Variant1-wrapper
>      case two: q_obj_Variant2-wrapper
> -enum SugaredUnionKind ['one', 'two']
> +enum SugaredUnionKind
> +    member one:
> +    member two:
>  object Variant1
>      member var1: str optional=False
>  object Variant2
> diff --git a/tests/qapi-schema/empty.out b/tests/qapi-schema/empty.out
> index 40b886ddae..9859251087 100644
> --- a/tests/qapi-schema/empty.out
> +++ b/tests/qapi-schema/empty.out
> @@ -1,3 +1,10 @@
> -enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> +enum QType
>      prefix QTYPE
> +    member none:
> +    member qnull:
> +    member qnum:
> +    member qstring:
> +    member qdict:
> +    member qlist:
> +    member qbool:
>  object q_empty
> diff --git a/tests/qapi-schema/event-case.out 
> b/tests/qapi-schema/event-case.out
> index 313c0fe7be..4dccc8f61e 100644
> --- a/tests/qapi-schema/event-case.out
> +++ b/tests/qapi-schema/event-case.out
> @@ -1,5 +1,12 @@
> -enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> +enum QType
>      prefix QTYPE
> +    member none:
> +    member qnull:
> +    member qnum:
> +    member qstring:
> +    member qdict:
> +    member qlist:
> +    member qbool:
>  event oops None
>     boxed=False
>  object q_empty
> diff --git a/tests/qapi-schema/ident-with-escape.out 
> b/tests/qapi-schema/ident-with-escape.out
> index b5637cb2e0..4d17bc6783 100644
> --- a/tests/qapi-schema/ident-with-escape.out
> +++ b/tests/qapi-schema/ident-with-escape.out
> @@ -1,5 +1,12 @@
> -enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> +enum QType
>      prefix QTYPE
> +    member none:
> +    member qnull:
> +    member qnum:
> +    member qstring:
> +    member qdict:
> +    member qlist:
> +    member qbool:
>  command fooA q_obj_fooA-arg -> None
>     gen=True success_response=True boxed=False
>  object q_empty
> diff --git a/tests/qapi-schema/include-relpath.out 
> b/tests/qapi-schema/include-relpath.out
> index 17e652535c..17b493ec24 100644
> --- a/tests/qapi-schema/include-relpath.out
> +++ b/tests/qapi-schema/include-relpath.out
> @@ -1,4 +1,14 @@
> -enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> +enum QType
>      prefix QTYPE
> -enum Status ['good', 'bad', 'ugly']
> +    member none:
> +    member qnull:
> +    member qnum:
> +    member qstring:
> +    member qdict:
> +    member qlist:
> +    member qbool:
> +enum Status
> +    member good:
> +    member bad:
> +    member ugly:
>  object q_empty
> diff --git a/tests/qapi-schema/include-repetition.out 
> b/tests/qapi-schema/include-repetition.out
> index 17e652535c..17b493ec24 100644
> --- a/tests/qapi-schema/include-repetition.out
> +++ b/tests/qapi-schema/include-repetition.out
> @@ -1,4 +1,14 @@
> -enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> +enum QType
>      prefix QTYPE
> -enum Status ['good', 'bad', 'ugly']
> +    member none:
> +    member qnull:
> +    member qnum:
> +    member qstring:
> +    member qdict:
> +    member qlist:
> +    member qbool:
> +enum Status
> +    member good:
> +    member bad:
> +    member ugly:
>  object q_empty
> diff --git a/tests/qapi-schema/include-simple.out 
> b/tests/qapi-schema/include-simple.out
> index 17e652535c..17b493ec24 100644
> --- a/tests/qapi-schema/include-simple.out
> +++ b/tests/qapi-schema/include-simple.out
> @@ -1,4 +1,14 @@
> -enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> +enum QType
>      prefix QTYPE
> -enum Status ['good', 'bad', 'ugly']
> +    member none:
> +    member qnull:
> +    member qnum:
> +    member qstring:
> +    member qdict:
> +    member qlist:
> +    member qbool:
> +enum Status
> +    member good:
> +    member bad:
> +    member ugly:
>  object q_empty
> diff --git a/tests/qapi-schema/indented-expr.out 
> b/tests/qapi-schema/indented-expr.out
> index 586795f44d..8bdc016e55 100644
> --- a/tests/qapi-schema/indented-expr.out
> +++ b/tests/qapi-schema/indented-expr.out
> @@ -1,5 +1,12 @@
> -enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> +enum QType
>      prefix QTYPE
> +    member none:
> +    member qnull:
> +    member qnum:
> +    member qstring:
> +    member qdict:
> +    member qlist:
> +    member qbool:
>  command eins None -> None
>     gen=True success_response=True boxed=False
>  object q_empty
> diff --git a/tests/qapi-schema/qapi-schema-test.out 
> b/tests/qapi-schema/qapi-schema-test.out
> index fc5fd25f1b..9a7cafc269 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -33,7 +33,10 @@ event EVENT_F UserDefAlternate
>  object Empty1
>  object Empty2
>      base Empty1
> -enum EnumOne ['value1', 'value2', 'value3']
> +enum EnumOne
> +    member value1:
> +    member value2:
> +    member value3:
>  object EventStructOne
>      member struct1: UserDefOne optional=False
>      member string: str optional=False
> @@ -42,16 +45,25 @@ object ForceArrays
>      member unused1: UserDefOneList optional=False
>      member unused2: UserDefTwoList optional=False
>      member unused3: TestStructList optional=False
> -enum MyEnum []
> +enum MyEnum
>  object NestedEnumsOne
>      member enum1: EnumOne optional=False
>      member enum2: EnumOne optional=True
>      member enum3: EnumOne optional=False
>      member enum4: EnumOne optional=True
> -enum QEnumTwo ['value1', 'value2']
> +enum QEnumTwo
>      prefix QENUM_TWO
> -enum QType ['none', 'qnull', 'qnum', 'qstring', 'qdict', 'qlist', 'qbool']
> +    member value1:
> +    member value2:
> +enum QType
>      prefix QTYPE
> +    member none:
> +    member qnull:
> +    member qnum:
> +    member qstring:
> +    member qdict:
> +    member qlist:
> +    member qbool:
>  alternate TestIfAlternate
>      tag type
>      case foo: int
> @@ -60,7 +72,9 @@ alternate TestIfAlternate
>  command TestIfCmd q_obj_TestIfCmd-arg -> None
>     gen=True success_response=True boxed=False
>      if defined(TEST_IF_CMD) && defined(TEST_IF_STRUCT)
> -enum TestIfEnum ['foo', 'bar']
> +enum TestIfEnum
> +    member foo:
> +    member bar:
>      if defined(TEST_IF_ENUM)
>  event TestIfEvent q_obj_TestIfEvent-arg
>     boxed=False
> @@ -73,7 +87,8 @@ object TestIfUnion
>      tag type
>      case foo: q_obj_TestStruct-wrapper
>      if defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)
> -enum TestIfUnionKind ['foo']
> +enum TestIfUnionKind
> +    member foo:
>      if defined(TEST_IF_UNION) && defined(TEST_IF_STRUCT)
>  object TestStruct
>      member integer: int optional=False
> @@ -122,7 +137,21 @@ object UserDefNativeListUnion
>      case string: q_obj_strList-wrapper
>      case sizes: q_obj_sizeList-wrapper
>      case any: q_obj_anyList-wrapper
> -enum UserDefNativeListUnionKind ['integer', 's8', 's16', 's32', 's64', 'u8', 
> 'u16', 'u32', 'u64', 'number', 'boolean', 'string', 'sizes', 'any']
> +enum UserDefNativeListUnionKind
> +    member integer:
> +    member s8:
> +    member s16:
> +    member s32:
> +    member s64:
> +    member u8:
> +    member u16:
> +    member u32:
> +    member u64:
> +    member number:
> +    member boolean:
> +    member string:
> +    member sizes:
> +    member any:
>  object UserDefOne
>      base UserDefZero
>      member string: str optional=False
> @@ -159,7 +188,8 @@ alternate __org.qemu_x-Alt
>      case b: __org.qemu_x-Base
>  object __org.qemu_x-Base
>      member __org.qemu_x-member1: __org.qemu_x-Enum optional=False
> -enum __org.qemu_x-Enum ['__org.qemu_x-value']
> +enum __org.qemu_x-Enum
> +    member __org.qemu_x-value:
>  object __org.qemu_x-Struct
>      base __org.qemu_x-Base
>      member __org.qemu_x-member2: str optional=False
> @@ -170,7 +200,8 @@ object __org.qemu_x-Union1
>      member type: __org.qemu_x-Union1Kind optional=False
>      tag type
>      case __org.qemu_x-branch: q_obj_str-wrapper
> -enum __org.qemu_x-Union1Kind ['__org.qemu_x-branch']
> +enum __org.qemu_x-Union1Kind
> +    member __org.qemu_x-branch:
>  object __org.qemu_x-Union2
>      base __org.qemu_x-Base
>      tag __org.qemu_x-member1
> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index 8627f978af..67c6c1ecef 100644
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -17,19 +17,18 @@ import sys
>  
>  
>  class QAPISchemaTestVisitor(QAPISchemaVisitor):
> -    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' % name
>          if prefix:
>              print '    prefix %s' % prefix
> +        self._print_members(members)
>          self._print_if(ifcond)
>  
>      def visit_object_type(self, name, info, ifcond, base, members, variants):
>          print 'object %s' % name
>          if base:
>              print '    base %s' % base.name
> -        for m in members:
> -            print '    member %s: %s optional=%s' % \
> -                (m.name, m.type.name, m.optional)
> +        self._print_members(members)
>          self._print_variants(variants)
>          self._print_if(ifcond)
>  
> @@ -51,6 +50,14 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
>          print '   boxed=%s' % boxed
>          self._print_if(ifcond)
>  
> +    @staticmethod
> +    def _print_members(members):
> +        for m in members:
> +            print '    member %s:' % m.name,
> +            if isinstance(m, QAPISchemaObjectTypeMember):
> +                print '%s optional=%s' % (m.type.name, m.optional),
> +            print
> +
>      @staticmethod
>      def _print_variants(variants):
>          if variants:

The change advertized in the commit message is hard to see among all the
other stuff going on.  I think this patch should be split into three
parts:

1. Rename QAPISchemaEnumType.values and related variables to members.
   Makes sense ever since commit 93bda4dd4 changed .values from list of
   string to list of QAPISchemaMember.  Obvious no-op.

2. Change the visit_enum_type().

3. Change test-qapi.py.