Eric Blake <ebl...@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 <ebl...@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
