Eric Blake <[email protected]> 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 QAPISchemaObjectType, > and use that function where needed. (Done at the ObjectType > level, since we already know all builtins and arrays are > implicit, no commands or events are implicit, and we don't > have any differences in generated code for regular vs. > implicit enums.) > > Signed-off-by: Eric Blake <[email protected]> > > --- > 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 | 3 ++- > scripts/qapi-visit.py | 3 ++- > scripts/qapi.py | 10 +++++++--- > 3 files changed, 11 insertions(+), 5 deletions(-) > > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py > index c5b71b0..6bac5b3 100644 > --- a/scripts/qapi-types.py > +++ b/scripts/qapi-types.py > @@ -234,7 +234,8 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor): > self._btin = None > > def visit_predicate(self, entity): > - return not isinstance(entity, QAPISchemaObjectType) or entity.info > + return not (isinstance(entity, QAPISchemaObjectType) and > + entity.is_implicit())
Aha, now the left hand side becomes necessary to guard the .is_implicit(). It stays superfluous if you make .is_implicit() a QAPISchemaEntity method. > > 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 0f47614..2957c85 100644 > --- a/scripts/qapi-visit.py > +++ b/scripts/qapi-visit.py > @@ -334,7 +334,8 @@ class QAPISchemaGenVisitVisitor(QAPISchemaVisitor): > self._btin = None > > def visit_predicate(self, entity): > - return not isinstance(entity, QAPISchemaObjectType) or entity.info > + return not (isinstance(entity, QAPISchemaObjectType) and > + entity.is_implicit()) > > 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 7d359c8..8123ab3 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -970,12 +970,15 @@ class QAPISchemaObjectType(QAPISchemaType): > self.variants.check(schema, members, seen) > self.members = members > > + def is_implicit(self): > + return self.name[0] == ':' > + If this test is here to stay, perhaps add a comment pointing to _make_implicit_object_type(). > 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): > @@ -1043,7 +1046,8 @@ 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 isinstance(self.type, > + QAPISchemaObjectType) and self.type.is_implicit(): I figure you break this line in the argument list to avoid a backslash for line continuation. I know PEP8 doesn't like them, but I like line breaks away from top level operators even less. I feel it should be broken after the and operator, even though that'll require wrapping the condition in parenthesis. > assert len(self.type.members) == 1 > assert not self.type.variants > return self.type.members[0].type
