Kevin Wolf <kw...@redhat.com> writes: > Am 27.01.2021 um 13:51 hat Markus Armbruster geschrieben: >> Kevin Wolf <kw...@redhat.com> writes: >> >> > This adds functions to the Visitor interface that can be used to define >> > aliases and alias scopes. >> > >> > Signed-off-by: Kevin Wolf <kw...@redhat.com> >> > --- >> > include/qapi/visitor-impl.h | 12 ++++++++++++ >> > include/qapi/visitor.h | 37 +++++++++++++++++++++++++++++++++++++ >> > qapi/qapi-visit-core.c | 21 +++++++++++++++++++++ >> > 3 files changed, 70 insertions(+) >> > >> > diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h >> > index 7362c043be..e30da2599c 100644 >> > --- a/include/qapi/visitor-impl.h >> > +++ b/include/qapi/visitor-impl.h >> > @@ -113,6 +113,18 @@ struct Visitor >> > The core takes care of the return type in the public interface. */ >> > void (*optional)(Visitor *v, const char *name, bool *present); >> > >> > + /* >> > + * Optional; intended for input visitors. If not given, aliases are >> > + * ignored. >> > + */ >> > + void (*define_alias)(Visitor *v, const char *alias, const char >> > **source); >> > + >> > + /* Must be set if define_alias is set */ >> > + void (*start_alias_scope)(Visitor *v); >> > + >> > + /* Must be set if define_alias is set */ >> > + void (*end_alias_scope)(Visitor *v); >> > + >> > /* Must be set */ >> > VisitorType type; >> > >> > diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h >> > index ebc19ede7f..9bdc0ee03d 100644 >> > --- a/include/qapi/visitor.h >> > +++ b/include/qapi/visitor.h >> > @@ -459,6 +459,43 @@ void visit_end_alternate(Visitor *v, void **obj); >> > */ >> > bool visit_optional(Visitor *v, const char *name, bool *present); >> > >> > +/* >> > + * Defines a new alias rule. >> > + * >> > + * If @alias is non-NULL, the member named @alias in the external >> > + * representation of the current struct is defined as an alias for the >> >> Terminology: the big comment uses "object". See also the FIXME in >> visit_start_struct()'s contract. > > Ok. Maybe the FIXME should be resolved to avoid this kind of problem.
True. Checking... the churn outside tests/ looks quite tolerable. Feel free to stick a suitable patch into v2. >> > + * member described by @source. >> > + * >> > + * If @alias is NULL, all members of the struct described by @source are >> > + * considered to have alias members with the same key in the current >> > + * struct. >> >> Define "the current struct". I believe it's the object being visited. > > Yes. > >> What happens if we're currently visiting something other than an object, >> i.e. the root of a tree, or a list? > > Then you (= the generated code) shouldn't call the function. Aliases > only make sense for objects (because everything else doesn't have keys). > > If you call it anyway, it depends on how you visit the elements of the > list. Currently, I think they are always visited with a NULL name. In > this case the alias should just never apply, but it looks like > propagate_aliases() might actually crash because it doesn't check for > NULL names. > > We don't have such callers and I don't think we want to have them, so > I'm not sure if we want to fix anything, and if we do, if the fix should > be tolerating and effectively ignoring such alias definitions or if we > should explicitly assert that the name is non-NULL. I'm like putting "no nonsense" into the contract, then enforce it with an assertion if practical. >> > + * >> > + * @source is a NULL-terminated array of names that describe the path to >> > + * a member, starting from the currently visited struct. >> >> I'm afraid "describe the path to a member" is too vague. How? >> >> I figure this is what you have in mind: >> >> cur = the currently visited object >> for s in source: >> cur = the member of cur denoted by s >> >> When @cur is indeed an object, then "the member denoted by @s" makes >> sense: you must pass a name when visiting object members, and whatever >> is visited with name @s is the member denoted by @s. >> >> "Must pass a name" is documented in the big comment: >> >> * The @name parameter of visit_type_FOO() describes the relation >> * between this QAPI value and its parent container. When visiting >> * the root of a tree, @name is ignored; when visiting a member of an >> * object, @name is the key associated with the value; when visiting a >> * member of a list, @name is NULL; and when visiting the member of an >> * alternate, @name should equal the name used for visiting the >> * alternate. >> >> But what if @cur is a list? I guess that makes no sense. Say so >> explicitly, please. > > Yes, everything but the last element in the path must be an object. Got it, thanks.
