Message ID | 1449033659-25497-12-git-send-email-eblake@redhat.com |
---|---|
State | New |
Headers | show |
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 --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
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(-)