diff mbox

[v14,11/15] qapi: Track enum values by QAPISchemaMember, not string

Message ID 1449033659-25497-12-git-send-email-eblake@redhat.com
State New
Headers show

Commit Message

Eric Blake Dec. 2, 2015, 5:20 a.m. UTC
Rather than using just an array of strings, make enum.values be
an array of the new QAPISchemaMember type, and add a helper
member_names() method to get back at the original list of names.
Likewise, creating an enum requires wrapping strings, via a new
QAPISchema._make_enum_members() method.  The benefit of wrapping
enum members in a QAPISchemaMember Python object is that we now
share the existing code for C name clash detection (although the
code is not yet active until a later commit removes the earlier
ad hoc parser checks).

In a related change, the QAPISchemaMember._pretty_owner() method
needs to learn about one more implicit type name: the generated
enum associated with a simple union.

In the interest of keeping the changes of this patch local to one
file, the visitor interface still passes just a list of names
rather than the full list of QAPISchemaMember instances.  We may
want to revisit this in the future, if the consistency with
visit_object_type() is worth it.

Signed-off-by: Eric Blake <eblake@redhat.com>

---
v14: Add .member_names() and ._make_enum_members(), cross-reference
comments, improve commit message
v13: new patch
---
 scripts/qapi.py | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

Comments

Markus Armbruster Dec. 2, 2015, 3:52 p.m. UTC | #1
Eric Blake <eblake@redhat.com> writes:

> Rather than using just an array of strings, make enum.values be
> an array of the new QAPISchemaMember type, and add a helper
> member_names() method to get back at the original list of names.
> Likewise, creating an enum requires wrapping strings, via a new
> QAPISchema._make_enum_members() method.  The benefit of wrapping
> enum members in a QAPISchemaMember Python object is that we now
> share the existing code for C name clash detection (although the
> code is not yet active until a later commit removes the earlier
> ad hoc parser checks).
>
> In a related change, the QAPISchemaMember._pretty_owner() method
> needs to learn about one more implicit type name: the generated
> enum associated with a simple union.
>
> In the interest of keeping the changes of this patch local to one
> file, the visitor interface still passes just a list of names
> rather than the full list of QAPISchemaMember instances.  We may
> want to revisit this in the future, if the consistency with
> visit_object_type() is worth it.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
>
> ---
> v14: Add .member_names() and ._make_enum_members(), cross-reference
> comments, improve commit message
> v13: new patch
> ---
>  scripts/qapi.py | 37 ++++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 11 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 2748464..8fad7c8 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -901,13 +901,16 @@ class QAPISchemaEnumType(QAPISchemaType):
>      def __init__(self, name, info, values, prefix):
>          QAPISchemaType.__init__(self, name, info)
>          for v in values:
> -            assert isinstance(v, str)
> +            assert isinstance(v, QAPISchemaMember)
> +            v.set_owner(name)
>          assert prefix is None or isinstance(prefix, str)
>          self.values = values
>          self.prefix = prefix
>
>      def check(self, schema):
> -        assert len(set(self.values)) == len(self.values)
> +        seen = {}
> +        for v in self.values:
> +            v.check_clash(self.info, seen)
>
>      def is_implicit(self):
>          # See QAPISchema._make_implicit_enum_type()
> @@ -916,8 +919,11 @@ class QAPISchemaEnumType(QAPISchemaType):
>      def c_type(self, is_param=False):
>          return c_name(self.name)
>
> +    def member_names(self):
> +        return [v.name for v in self.values]
> +
>      def c_null(self):
> -        return c_enum_const(self.name, (self.values + ['_MAX'])[0],
> +        return c_enum_const(self.name, (self.member_names() + ['_MAX'])[0],
>                              self.prefix)
>
>      def json_type(self):
> @@ -925,7 +931,7 @@ class QAPISchemaEnumType(QAPISchemaType):
>
>      def visit(self, visitor):
>          visitor.visit_enum_type(self.name, self.info,
> -                                self.values, self.prefix)
> +                                self.member_names(), self.prefix)
>
>
>  class QAPISchemaArrayType(QAPISchemaType):
> @@ -1049,6 +1055,9 @@ class QAPISchemaMember(object):
>              else:
>                  assert owner.endswith('-wrapper')
>                  return '(branch of %s)' % owner[:-8]
> +        if owner.endswith('Kind'):
> +            # See QAPISchema._make_implicit_enum_type()
> +            return '(branch of %s)' % owner[:-4]
>          return '(%s of %s)' % (self.role, owner)
>
>      def describe(self):
> @@ -1099,7 +1108,7 @@ class QAPISchemaObjectTypeVariants(object):
>              # Union names must match enum values; alternate names are
>              # checked separately. Use 'seen' to tell the two apart.
>              if seen:
> -                assert v.name in self.tag_member.type.values
> +                assert v.name in self.tag_member.type.member_names()
>                  assert isinstance(v.type, QAPISchemaObjectType)
>                  v.type.check(schema)
>
> @@ -1257,15 +1266,20 @@ class QAPISchema(object):
>          self.the_empty_object_type = QAPISchemaObjectType(':empty', None, None,
>                                                            [], None)
>          self._def_entity(self.the_empty_object_type)
> -        self._def_entity(QAPISchemaEnumType('QType', None,
> -                                            ['none', 'qnull', 'qint',
> -                                             'qstring', 'qdict', 'qlist',
> -                                             'qfloat', 'qbool'],
> +        qtype_values = self._make_enum_members('none', 'qnull', 'qint',
> +                                               'qstring', 'qdict', 'qlist',
> +                                               'qfloat', 'qbool')
> +        self._def_entity(QAPISchemaEnumType('QType', None, qtype_values,
>                                              'QTYPE'))
>
> +    def _make_enum_members(self, *values):
> +        return [QAPISchemaMember(v) for v in values]

Sure a var-positional parameter makes sense here?  Two of three callers
want to pass a list and have to splice it with a *expression.  Only the
call above actually profits from the var-positional, and it could easily
keep passing a list.

> +
>      def _make_implicit_enum_type(self, name, info, values):
> +        # See also QAPISchemaObjectTypeMember._pretty_owner()
>          name = name + 'Kind'   # Use namespace reserved by add_name()
> -        self._def_entity(QAPISchemaEnumType(name, info, values, None))
> +        self._def_entity(QAPISchemaEnumType(
> +            name, info, self._make_enum_members(*values), None))
>          return name
>
>      def _make_array_type(self, element_type, info):
> @@ -1288,7 +1302,8 @@ class QAPISchema(object):
>          name = expr['enum']
>          data = expr['data']
>          prefix = expr.get('prefix')
> -        self._def_entity(QAPISchemaEnumType(name, info, data, prefix))
> +        self._def_entity(QAPISchemaEnumType(
> +            name, info, self._make_enum_members(*data), prefix))
>
>      def _make_member(self, name, typ, info):
>          optional = False
diff mbox

Patch

diff --git a/scripts/qapi.py b/scripts/qapi.py
index 2748464..8fad7c8 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -901,13 +901,16 @@  class QAPISchemaEnumType(QAPISchemaType):
     def __init__(self, name, info, values, prefix):
         QAPISchemaType.__init__(self, name, info)
         for v in values:
-            assert isinstance(v, str)
+            assert isinstance(v, QAPISchemaMember)
+            v.set_owner(name)
         assert prefix is None or isinstance(prefix, str)
         self.values = values
         self.prefix = prefix

     def check(self, schema):
-        assert len(set(self.values)) == len(self.values)
+        seen = {}
+        for v in self.values:
+            v.check_clash(self.info, seen)

     def is_implicit(self):
         # See QAPISchema._make_implicit_enum_type()
@@ -916,8 +919,11 @@  class QAPISchemaEnumType(QAPISchemaType):
     def c_type(self, is_param=False):
         return c_name(self.name)

+    def member_names(self):
+        return [v.name for v in self.values]
+
     def c_null(self):
-        return c_enum_const(self.name, (self.values + ['_MAX'])[0],
+        return c_enum_const(self.name, (self.member_names() + ['_MAX'])[0],
                             self.prefix)

     def json_type(self):
@@ -925,7 +931,7 @@  class QAPISchemaEnumType(QAPISchemaType):

     def visit(self, visitor):
         visitor.visit_enum_type(self.name, self.info,
-                                self.values, self.prefix)
+                                self.member_names(), self.prefix)


 class QAPISchemaArrayType(QAPISchemaType):
@@ -1049,6 +1055,9 @@  class QAPISchemaMember(object):
             else:
                 assert owner.endswith('-wrapper')
                 return '(branch of %s)' % owner[:-8]
+        if owner.endswith('Kind'):
+            # See QAPISchema._make_implicit_enum_type()
+            return '(branch of %s)' % owner[:-4]
         return '(%s of %s)' % (self.role, owner)

     def describe(self):
@@ -1099,7 +1108,7 @@  class QAPISchemaObjectTypeVariants(object):
             # Union names must match enum values; alternate names are
             # checked separately. Use 'seen' to tell the two apart.
             if seen:
-                assert v.name in self.tag_member.type.values
+                assert v.name in self.tag_member.type.member_names()
                 assert isinstance(v.type, QAPISchemaObjectType)
                 v.type.check(schema)

@@ -1257,15 +1266,20 @@  class QAPISchema(object):
         self.the_empty_object_type = QAPISchemaObjectType(':empty', None, None,
                                                           [], None)
         self._def_entity(self.the_empty_object_type)
-        self._def_entity(QAPISchemaEnumType('QType', None,
-                                            ['none', 'qnull', 'qint',
-                                             'qstring', 'qdict', 'qlist',
-                                             'qfloat', 'qbool'],
+        qtype_values = self._make_enum_members('none', 'qnull', 'qint',
+                                               'qstring', 'qdict', 'qlist',
+                                               'qfloat', 'qbool')
+        self._def_entity(QAPISchemaEnumType('QType', None, qtype_values,
                                             'QTYPE'))

+    def _make_enum_members(self, *values):
+        return [QAPISchemaMember(v) for v in values]
+
     def _make_implicit_enum_type(self, name, info, values):
+        # See also QAPISchemaObjectTypeMember._pretty_owner()
         name = name + 'Kind'   # Use namespace reserved by add_name()
-        self._def_entity(QAPISchemaEnumType(name, info, values, None))
+        self._def_entity(QAPISchemaEnumType(
+            name, info, self._make_enum_members(*values), None))
         return name

     def _make_array_type(self, element_type, info):
@@ -1288,7 +1302,8 @@  class QAPISchema(object):
         name = expr['enum']
         data = expr['data']
         prefix = expr.get('prefix')
-        self._def_entity(QAPISchemaEnumType(name, info, data, prefix))
+        self._def_entity(QAPISchemaEnumType(
+            name, info, self._make_enum_members(*data), prefix))

     def _make_member(self, name, typ, info):
         optional = False