Message ID | 1443930073-19359-5-git-send-email-eblake@redhat.com |
---|---|
State | New |
Headers | show |
Eric Blake <eblake@redhat.com> writes: > A future patch will enable error reporting from the various > QAPISchema*.check() methods. But to report an error related > to an implicit type, we'll need to associate a location with > the type (the same location as the top-level entity that is > causing the creation of the implicit type), and once we do > that, keying off of whether foo.info exists is no longer a > viable way to determine if foo is an implicit type. > > Instead, add an is_implicit() method to QAPISchemaEntity, with > an internal helper overridden for ObjectType and EnumType, and > use that function where needed. > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v7: declare at the Entity level, with an optional argument for > filtering by sub-class > v6: split 11/46 into pieces; don't rename _info yet; rework atop > nicer filtering mechanism, including no need to change visitor > signature > --- > scripts/qapi-types.py | 2 +- > scripts/qapi-visit.py | 2 +- > scripts/qapi.py | 24 +++++++++++++++++++++--- > 3 files changed, 23 insertions(+), 5 deletions(-) > > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py > index 2a29c6e..227ea5c 100644 > --- a/scripts/qapi-types.py > +++ b/scripts/qapi-types.py > @@ -235,7 +235,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor): > > def visit_needed(self, entity): > # Visit everything except implicit objects > - return not isinstance(entity, QAPISchemaObjectType) or entity.info > + return not entity.is_implicit(QAPISchemaObjectType) The alternative is something like return not (isinstance(entity, QAPISchemaObjectType) and entity.is_implicit()) Trades a more verbose "is this an implicit object type" check for a simpler is_implicit(). Shorter overall, and feels better to me. But if you feel strongly about your way of doing it, I can live with it. > > def _gen_type_cleanup(self, name): > self.decl += gen_type_cleanup_decl(name) > diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py > index 9fc79a7..56b8390 100644 > --- a/scripts/qapi-visit.py > +++ b/scripts/qapi-visit.py > @@ -335,7 +335,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor): > > def visit_needed(self, entity): > # Visit everything except implicit objects > - return not isinstance(entity, QAPISchemaObjectType) or entity.info > + return not entity.is_implicit(QAPISchemaObjectType) > > def visit_enum_type(self, name, info, values, prefix): > self.decl += gen_visit_decl(name, scalar=True) > diff --git a/scripts/qapi.py b/scripts/qapi.py > index 4573599..19cc78e 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -801,6 +801,16 @@ class QAPISchemaEntity(object): > def check(self, schema): > pass > > + # Return True if this entity is implicit > + # If @typ, only return True if entity is also an instance of that type > + def is_implicit(self, typ=None): > + if typ and not isinstance(self, typ): > + return False > + return self._is_implicit() > + > + def _is_implicit(self): > + return not self.info > + > def visit(self, visitor): > pass > > @@ -903,6 +913,10 @@ class QAPISchemaEnumType(QAPISchemaType): > def check(self, schema): > assert len(set(self.values)) == len(self.values) > > + def _is_implicit(self): > + # See QAPISchema._make_implicit_enum_type() > + return self.name[-4:] == 'Kind' > + > def c_type(self, is_param=False): > return c_name(self.name) > Might want to add a comment to _make_implicit_enum_type() that justifies name + 'Kind' by pointing out add_name() rejects a 'Kind' suffix. > @@ -973,12 +987,16 @@ class QAPISchemaObjectType(QAPISchemaType): > self.variants.check(schema, members, seen) > self.members = members > > + def _is_implicit(self): > + # See QAPISchema._make_implicit_object_type() > + return self.name[0] == ':' > + > def c_name(self): > - assert self.info > + assert not self.is_implicit() > return QAPISchemaType.c_name(self) > > def c_type(self, is_param=False): > - assert self.info > + assert not self.is_implicit() > return QAPISchemaType.c_type(self) > > def json_type(self): > @@ -1046,7 +1064,7 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): > # This function exists to support ugly simple union special cases > # TODO get rid of them, and drop the function > def simple_union_type(self): > - if isinstance(self.type, QAPISchemaObjectType) and not self.type.info: > + if self.type.is_implicit(QAPISchemaObjectType): > assert len(self.type.members) == 1 > assert not self.type.variants > return self.type.members[0].type
On 10/07/2015 10:27 AM, Markus Armbruster wrote: >> def visit_needed(self, entity): >> # Visit everything except implicit objects >> - return not isinstance(entity, QAPISchemaObjectType) or entity.info >> + return not entity.is_implicit(QAPISchemaObjectType) > > The alternative is something like > > return not (isinstance(entity, QAPISchemaObjectType) and > entity.is_implicit()) > > Trades a more verbose "is this an implicit object type" check for a > simpler is_implicit(). Shorter overall, and feels better to me. But if > you feel strongly about your way of doing it, I can live with it. I'm not strongly tied to the concise form enough to stall review, so v8 will use the longer explicit form.
diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py index 2a29c6e..227ea5c 100644 --- a/scripts/qapi-types.py +++ b/scripts/qapi-types.py @@ -235,7 +235,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor): def visit_needed(self, entity): # Visit everything except implicit objects - return not isinstance(entity, QAPISchemaObjectType) or entity.info + return not entity.is_implicit(QAPISchemaObjectType) def _gen_type_cleanup(self, name): self.decl += gen_type_cleanup_decl(name) diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py index 9fc79a7..56b8390 100644 --- a/scripts/qapi-visit.py +++ b/scripts/qapi-visit.py @@ -335,7 +335,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor): def visit_needed(self, entity): # Visit everything except implicit objects - return not isinstance(entity, QAPISchemaObjectType) or entity.info + return not entity.is_implicit(QAPISchemaObjectType) def visit_enum_type(self, name, info, values, prefix): self.decl += gen_visit_decl(name, scalar=True) diff --git a/scripts/qapi.py b/scripts/qapi.py index 4573599..19cc78e 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -801,6 +801,16 @@ class QAPISchemaEntity(object): def check(self, schema): pass + # Return True if this entity is implicit + # If @typ, only return True if entity is also an instance of that type + def is_implicit(self, typ=None): + if typ and not isinstance(self, typ): + return False + return self._is_implicit() + + def _is_implicit(self): + return not self.info + def visit(self, visitor): pass @@ -903,6 +913,10 @@ class QAPISchemaEnumType(QAPISchemaType): def check(self, schema): assert len(set(self.values)) == len(self.values) + def _is_implicit(self): + # See QAPISchema._make_implicit_enum_type() + return self.name[-4:] == 'Kind' + def c_type(self, is_param=False): return c_name(self.name) @@ -973,12 +987,16 @@ class QAPISchemaObjectType(QAPISchemaType): self.variants.check(schema, members, seen) self.members = members + def _is_implicit(self): + # See QAPISchema._make_implicit_object_type() + return self.name[0] == ':' + def c_name(self): - assert self.info + assert not self.is_implicit() return QAPISchemaType.c_name(self) def c_type(self, is_param=False): - assert self.info + assert not self.is_implicit() return QAPISchemaType.c_type(self) def json_type(self): @@ -1046,7 +1064,7 @@ class QAPISchemaObjectTypeVariant(QAPISchemaObjectTypeMember): # This function exists to support ugly simple union special cases # TODO get rid of them, and drop the function def simple_union_type(self): - if isinstance(self.type, QAPISchemaObjectType) and not self.type.info: + if self.type.is_implicit(QAPISchemaObjectType): assert len(self.type.members) == 1 assert not self.type.variants return self.type.members[0].type
A future patch will enable error reporting from the various QAPISchema*.check() methods. But to report an error related to an implicit type, we'll need to associate a location with the type (the same location as the top-level entity that is causing the creation of the implicit type), and once we do that, keying off of whether foo.info exists is no longer a viable way to determine if foo is an implicit type. Instead, add an is_implicit() method to QAPISchemaEntity, with an internal helper overridden for ObjectType and EnumType, and use that function where needed. Signed-off-by: Eric Blake <eblake@redhat.com> --- v7: declare at the Entity level, with an optional argument for filtering by sub-class v6: split 11/46 into pieces; don't rename _info yet; rework atop nicer filtering mechanism, including no need to change visitor signature --- scripts/qapi-types.py | 2 +- scripts/qapi-visit.py | 2 +- scripts/qapi.py | 24 +++++++++++++++++++++--- 3 files changed, 23 insertions(+), 5 deletions(-)