Re: [Qemu-devel] [PATCH v13 11/14] qapi: Populate info['name'] for each entity

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@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.  Add a new info['name'] field
> to track this information, as it will be handy in future commits
> for better error messages.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>  scripts/qapi.py | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index d1239c2..ff3fccb 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
> @@ -1296,7 +1296,7 @@ class QAPISchema(object):
>          return name
>
>      def _def_enum_type(self, expr, info):
> -        name = expr['enum']
> +        info['name'] = 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']
> +        info['name'] = 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']
> +        info['name'] = name = expr['union']
>          data = expr['data']
>          base = expr.get('base')
>          tag_name = expr.get('discriminator')
> @@ -1359,7 +1359,7 @@ class QAPISchema(object):
>                                                                variants)))
>
>      def _def_alternate_type(self, expr, info):
> -        name = expr['alternate']
> +        info['name'] = name = expr['alternate']
>          data = expr['data']
>          variants = [self._make_variant(key, value)
>                      for (key, value) in data.iteritems()]
> @@ -1371,7 +1371,7 @@ class QAPISchema(object):
>                                                                   variants)))
>
>      def _def_command(self, expr, info):
> -        name = expr['command']
> +        info['name'] = name = expr['command']
>          data = expr.get('data')
>          rets = expr.get('returns')
>          gen = expr.get('gen', True)
> @@ -1386,7 +1386,7 @@ class QAPISchema(object):
>                                             success_response))
>
>      def _def_event(self, expr, info):
> -        name = expr['event']
> +        info['name'] = name = expr['event']
>          data = expr.get('data')
>          if isinstance(data, OrderedDict):
>              data = self._make_implicit_object_type(

There's no single place where we can do that?

info is created in QAPISchemaParser.__init__()'s loop:

            expr_info = {'file': fname, 'line': self.line,
                         'parent': self.incl_info}

We obviously don't know the name there.

It's stored in QAPISchemaParser.exprs together with the expression
dictionary:

                expr_elem = {'expr': expr,
                             'info': expr_info}
                self.exprs.append(expr_elem)

QAPISchema.__init__() filters it through check_exprs()

            self.exprs = check_exprs(QAPISchemaParser(open(fname, "r")).exprs)

but check_exprs() should go away.

That leaves _def_exprs().  It currently can't, because the _def_FOO()
don't return anything, unlike the _make_BAR().

Your solution might be the simplest one...  Let's see how much use the
later commits get out of it.

However, I'd prefer

        name = info['name'] = expr['event']

because that makes it easier to see the definition of name, which is the
one that gets used further down in the functions.