Re: [Qemu-devel] [PATCH v7 04/15] qapi: Hide tag_name data member of variants

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> Clean up the only remaining external use of the tag_name field of
> QAPISchemaObjectTypeVariants, by explicitly listing the generated
> 'type' tag for all variants in the testsuite.  Then we can mark the
> tag_name field as private by adding a leading underscore to prevent
> any further use.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v7: no change
> v6: rebase to earlier changes
> ---
>  scripts/qapi.py                        |  8 ++++----
>  tests/qapi-schema/qapi-schema-test.out | 10 ++++++++++
>  tests/qapi-schema/test-qapi.py         |  3 +--
>  3 files changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 82d96e2..3554ab1 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1086,7 +1086,7 @@ class QAPISchemaObjectTypeVariants(object):
>          assert len(variants) > 0
>          for v in variants:
>              assert isinstance(v, QAPISchemaObjectTypeVariant)
> -        self.tag_name = tag_name
> +        self._tag_name = tag_name
>          self.tag_member = tag_member
>          self.variants = variants
>
> @@ -1096,8 +1096,8 @@ class QAPISchemaObjectTypeVariants(object):
>
>      def check(self, schema, seen):
>          if not self.tag_member:    # flat union
> -            self.tag_member = seen[c_name(self.tag_name)]
> -            assert self.tag_name == self.tag_member.name
> +            self.tag_member = seen[c_name(self._tag_name)]
> +            assert self._tag_name == self.tag_member.name
>          assert isinstance(self.tag_member.type, QAPISchemaEnumType)
>          for v in self.variants:
>              v.check(schema)
> @@ -1127,7 +1127,7 @@ class QAPISchemaAlternateType(QAPISchemaType):
>      def __init__(self, name, info, variants):
>          QAPISchemaType.__init__(self, name, info)
>          assert isinstance(variants, QAPISchemaObjectTypeVariants)
> -        assert not variants.tag_name
> +        assert variants.tag_member
>          variants.set_owner(name)
>          variants.tag_member.set_owner(self.name)
>          self.variants = variants
> diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
> index 19cd214..f34ecc7 100644
> --- a/tests/qapi-schema/qapi-schema-test.out
> +++ b/tests/qapi-schema/qapi-schema-test.out
> @@ -1,19 +1,25 @@
>  alternate AltIntNum
> +    tag type
>      case i: int
>      case n: number
>  alternate AltNumInt
> +    tag type
>      case n: number
>      case i: int
>  alternate AltNumStr
> +    tag type
>      case n: number
>      case s: str
>  alternate AltStrBool
> +    tag type
>      case s: str
>      case b: bool
>  alternate AltStrInt
> +    tag type
>      case s: str
>      case i: int
>  alternate AltStrNum
> +    tag type
>      case s: str
>      case n: number
>  event EVENT_A None
> @@ -50,6 +56,7 @@ object UserDefA
>      member boolean: bool optional=False
>      member a_b: int optional=True
>  alternate UserDefAlternate
> +    tag type
>      case udfu: UserDefFlatUnion
>      case s: str
>      case i: int
> @@ -72,6 +79,7 @@ object UserDefFlatUnion2
>      case value2: UserDefB
>  object UserDefNativeListUnion
>      member type: UserDefNativeListUnionKind optional=False
> +    tag type
>      case integer: q_obj_intList-wrapper
>      case s8: q_obj_int8List-wrapper
>      case s16: q_obj_int16List-wrapper
> @@ -117,6 +125,7 @@ object WrapAlternate
>      member alt: UserDefAlternate optional=False
>  event __ORG.QEMU_X-EVENT __org.qemu_x-Struct
>  alternate __org.qemu_x-Alt
> +    tag type
>      case __org.qemu_x-branch: str
>      case b: __org.qemu_x-Base
>  object __org.qemu_x-Base
> @@ -130,6 +139,7 @@ object __org.qemu_x-Struct2
>      member array: __org.qemu_x-Union1List optional=False
>  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']
>  object __org.qemu_x-Union2

The implicit "tag type" is now shown.  You can still recognize simple
unions by their -wrapper types.  Okay.

> diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
> index ccd1704..65ca19a 100644
> --- a/tests/qapi-schema/test-qapi.py
> +++ b/tests/qapi-schema/test-qapi.py
> @@ -47,8 +47,7 @@ class QAPISchemaTestVisitor(QAPISchemaVisitor):
>      @staticmethod
>      def _print_variants(variants):
>          if variants:
> -            if variants.tag_name:
> -                print '    tag %s' % variants.tag_name
> +            print '    tag %s' % variants.tag_member.name
>              for v in variants.variants:
>                  print '    case %s: %s' % (v.name, v.type.name)