Am 17.02.2021 um 16:23 hat Markus Armbruster geschrieben: > Kevin Wolf <kw...@redhat.com> writes: > > > Introduce alias definitions for object types (structs and unions). This > > allows using the same QAPI type and visitor for many syntax variations > > that exist in the external representation, like between QMP and the > > command line. It also provides a new tool for evolving the schema while > > maintaining backwards compatibility during a deprecation period. > > > > Signed-off-by: Kevin Wolf <kw...@redhat.com> > [...] > > diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py > > index 22e62df901..e370485f6e 100644 > > --- a/scripts/qapi/visit.py > > +++ b/scripts/qapi/visit.py > > @@ -26,6 +26,7 @@ from .common import ( > > from .gen import QAPISchemaModularCVisitor, ifcontext > > from .schema import ( > > QAPISchema, > > + QAPISchemaAlias, > > QAPISchemaEnumMember, > > QAPISchemaEnumType, > > QAPISchemaFeature, > > @@ -60,7 +61,8 @@ bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s > > *obj, Error **errp); > > def gen_visit_object_members(name: str, > > base: Optional[QAPISchemaObjectType], > > members: List[QAPISchemaObjectTypeMember], > > - variants: Optional[QAPISchemaVariants]) -> > > str: > > + variants: Optional[QAPISchemaVariants], > > + aliases: List[QAPISchemaAlias]) -> str: > > ret = mcgen(''' > > > > bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error > > **errp) > > @@ -68,6 +70,24 @@ bool visit_type_%(c_name)s_members(Visitor *v, > > %(c_name)s *obj, Error **errp) > > ''', > > c_name=c_name(name)) > > > > + if aliases: > > + ret += mcgen(''' > > + visit_start_alias_scope(v); > > +''') > > + > > + for a in aliases: > > + if a.name: > > + name = '"%s"' % a.name > > + else: > > + name = "NULL" > > + > > + source = ", ".join('"%s"' % x for x in a.source) > > + > > + ret += mcgen(''' > > + visit_define_alias(v, %(name)s, (const char * []) { %(source)s, NULL > > }); > > +''', > > + name=name, source=source) > > + > > if base: > > ret += mcgen(''' > > if (!visit_type_%(c_type)s_members(v, (%(c_type)s *)obj, errp)) { > > @@ -133,6 +153,11 @@ bool visit_type_%(c_name)s_members(Visitor *v, > > %(c_name)s *obj, Error **errp) > > } > > ''') > > > > + if aliases: > > + ret += mcgen(''' > > + visit_end_alias_scope(v); > > +''') > > + > > ret += mcgen(''' > > return true; > > } > > The visit_type_FOO_members() are primarily helpers for the > visit_type_FOO(). But they also get called > > * by visit_type_BAR_members() when > > - struct or union BAR has 'base': 'FOO' > - union or alternate BAR a variant 'FOO' > > * by qmp_marshal_CMD() when > > - CMD has 'boxed': true, 'data': 'FOO' > > Have you considered these cases? > > How's the test coverage?
What is the difference between these cases? The visiting should work the same, no matter from where it was started. I did consider the struct base/union variant case and this is why I introduced visit_start/end_alias_scope so that aliases wouldn't leak to the outer level. Now that I'm trying to think of a test case, this probably only protects against weird corner cases: The input object is the same anyway, so I guess the only way for this to make a difference is when the base struct defines an alias for a member that it doesn't even have (so the alias would remain unused when applied to the base struct independently), but that exists in the struct in which it is embedded. I hope adding a test case that checks that this is an error should be easy. Would the right place be tests/test-qobject-input-visitor.c? Can you think of any other specific differences that need to be tested? Kevin