Eric Blake <ebl...@redhat.com> writes: > Every non-implicit entity is associated with an 'info' > dictionary, but it is not easy to reverse-engineer the name of > the top-most entity associated with that 'info'. Our use of > 'case_whitelist' (added in commit 893e1f2) is thus currently > tied to the owner of a member instead; but as the previous patch > showed with CpuInfo, this requires whitelist exceptions to know > how an implicit name will be generated.
Why is that a problem? > While we have a ._pretty_owner() that maps from implicit names > back to a human readable phrase, that produces more than just a > plain top-level entity name. What's more, the current use of > ._pretty_owner() is via .check_clash(), which can be called on > the same member object more than once (once through the > standalone type, and a second time when used as a base class of > a derived tpye); if a clash is only introduced in the derived > class, using ._pretty_owner() to report the error on behalf of > the base class named in member.owner seems wrong. Therefore, > we need a new mechanism. Now I'm confused. Are you fixing suboptimal error messages? If yes, can you show the message before & after the patch? > Add a new info['name'] field to track the information we need, > allowing us to change 'case_whitelist' to use only names present > in the qapi files. > > Unfortunately, there is no one good place to add the mapping: > at the point 'info' is created in QAPISchemaParser.__init__(), > we don't know the name. Info is then stored into > QAPISchemaParser.exprs, which then then gets fed to > QAPISchema.__init__() via the global check_exprs(), but we want > check_exprs() to go away. And QAPISchema._def_exprs() sees > every entity, except that the various _def_FOO() helpers don't > return anything. So we have to modify all of the _def_FOO() > methods. > > Signed-off-by: Eric Blake <ebl...@redhat.com> > > --- > v4: Change series again :) > [previously posted in subset F]: > v6: sink later in series, and rework commit message > [previously posted in subset D]: > v14: rearrange assignment, improve commit message > v13: new patch > --- > scripts/qapi.py | 16 ++++++++-------- > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/scripts/qapi.py b/scripts/qapi.py > index ebcd207..6c991c3 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -62,8 +62,8 @@ returns_whitelist = [ > # Whitelist of entities allowed to violate case conventions > case_whitelist = [ > # From QMP: > - ':obj-CpuInfo-base', # CPU, visible through query-cpu > 'ACPISlotType', # DIMM, visible through query-acpi-ospm-status > + 'CpuInfo', # CPU and PC, visible through query-cpu > 'CpuInfoMIPS', # PC, visible through query-cpu > 'CpuInfoTricore', # PC, visible through query-cpu > 'QapiErrorClass', # all members, visible through errors > @@ -1035,7 +1035,7 @@ class QAPISchemaMember(object): > > def check_clash(self, info, seen): > cname = c_name(self.name) > - if cname.lower() != cname and self.owner not in case_whitelist: > + if cname.lower() != cname and info['name'] not in case_whitelist: > raise QAPIExprError(info, > "%s should not use uppercase" % > self.describe()) > if cname in seen: Doesn't look like you're touching the error message: QAPIExprError() doesn't use info['name']. > @@ -1296,7 +1296,7 @@ class QAPISchema(object): > return name > > def _def_enum_type(self, expr, info): > - name = expr['enum'] > + name = info['name'] = expr['enum'] > data = expr['data'] > prefix = expr.get('prefix') > self._def_entity(QAPISchemaEnumType( > @@ -1317,7 +1317,7 @@ class QAPISchema(object): > for (key, value) in data.iteritems()] > > def _def_struct_type(self, expr, info): > - name = expr['struct'] > + name = info['name'] = expr['struct'] > base = expr.get('base') > data = expr['data'] > self._def_entity(QAPISchemaObjectType(name, info, base, > @@ -1336,7 +1336,7 @@ class QAPISchema(object): > return QAPISchemaObjectTypeVariant(case, typ) > > def _def_union_type(self, expr, info): > - name = expr['union'] > + name = info['name'] = expr['union'] > data = expr['data'] > base = expr.get('base') > tag_name = expr.get('discriminator') > @@ -1362,7 +1362,7 @@ class QAPISchema(object): > variants))) > > def _def_alternate_type(self, expr, info): > - name = expr['alternate'] > + name = info['name'] = expr['alternate'] > data = expr['data'] > variants = [self._make_variant(key, value) > for (key, value) in data.iteritems()] > @@ -1374,7 +1374,7 @@ class QAPISchema(object): > variants))) > > def _def_command(self, expr, info): > - name = expr['command'] > + name = info['name'] = expr['command'] > data = expr.get('data') > rets = expr.get('returns') > gen = expr.get('gen', True) > @@ -1389,7 +1389,7 @@ class QAPISchema(object): > success_response)) > > def _def_event(self, expr, info): > - name = expr['event'] > + name = info['name'] = expr['event'] > data = expr.get('data') > if isinstance(data, OrderedDict): > data = self._make_implicit_object_type(