[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/2] qapi: add location comment for generated
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] qapi: add location comment for generated types |
Date: |
Thu, 6 Jul 2017 05:11:53 -0700 |
Hi
On Thu, Jul 6, 2017 at 10:43 AM Markus Armbruster <address@hidden> wrote:
> Marc-André Lureau <address@hidden> writes:
>
> > This may help to find where the origin of the type was declared in the
> > json (when greping isn't easy enough).
> >
> > Generates the following kind of C comment before types:
> >
> > /* /home/elmarco/src/qemu/qapi/introspect.json:94 */
> > typedef struct SchemaInfo SchemaInfo;
>
> Can we do relative file names? I.e. just qapi/introspect.json.
>
We could, but relative to $srcdir or $builddir? I think absolute path
avoids some potential confusion.
>
> Would such comments be useful for things other than typedefs?
>
Certainly it could, however I just needed it for typedef, didn't bother
adding it for the rest.
>
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> > scripts/qapi.py | 12 +++++++++---
> > scripts/qapi-event.py | 2 +-
> > scripts/qapi-types.py | 18 +++++++++---------
> > 3 files changed, 19 insertions(+), 13 deletions(-)
> >
> > diff --git a/scripts/qapi.py b/scripts/qapi.py
> > index d37a6157e0..7f9935eec0 100644
> > --- a/scripts/qapi.py
> > +++ b/scripts/qapi.py
> > @@ -1852,15 +1852,21 @@ const char *const %(c_name)s_lookup[] = {
> > return ret
> >
> >
> > -def gen_enum(name, values, prefix=None):
> > +def build_src_loc_comment(info):
> > + if info:
> > + return '\n/* %s:%d */' % (info['file'], info['line'])
>
> Leading instead of trailing newline, hmm. Let's see how it works out.
>
> > + else:
> > + return ''
>
> Let's drop the else: line.
>
ok
>
> > +
> > +def gen_enum(info, name, values, prefix=None):
>
> Make that name, info, values, ... for consistency with other functions
> taking both name and info. More of the same below.
>
>
ok
> > # append automatically generated _MAX value
> > enum_values = values + ['_MAX']
> >
> > ret = mcgen('''
> > -
> > +%(comment)s
>
> Loses the blank line when not info.
>
> > typedef enum %(c_name)s {
> > ''',
> > - c_name=c_name(name))
> > + c_name=c_name(name), comment=build_src_loc_comment(
> info))
> >
> > i = 0
> > for value in enum_values:
> > diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
> > index bcbef1035f..cf5e282011 100644
> > --- a/scripts/qapi-event.py
> > +++ b/scripts/qapi-event.py
> > @@ -159,7 +159,7 @@ class QAPISchemaGenEventVisitor(QAPISchemaVisitor):
> > self._event_names = []
> >
> > def visit_end(self):
> > - self.decl += gen_enum(event_enum_name, self._event_names)
> > + self.decl += gen_enum(None, event_enum_name, self._event_names)
> > self.defn += gen_enum_lookup(event_enum_name,
> self._event_names)
> > self._event_names = None
> >
> > diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
> > index b45e7b5634..9c8a3e277b 100644
> > --- a/scripts/qapi-types.py
> > +++ b/scripts/qapi-types.py
> > @@ -19,12 +19,12 @@ from qapi import *
> > objects_seen = set()
> >
> >
> > -def gen_fwd_object_or_array(name):
> > +def gen_fwd_object_or_array(info, name):
> > return mcgen('''
> > -
> > +%(comment)s
>
> Likewise.
>
> I think we should drop the newline from build_src_loc_comment()'s value,
> and keep the blank line before its two uses.
>
>
And you get an extra empty line if info is None. I don't mind. Other option
is to just add \n in the else case in build_src_loc_comment()
> > typedef struct %(c_name)s %(c_name)s;
> > ''',
> > - c_name=c_name(name))
> > + c_name=c_name(name), comment=build_src_loc_comment(
> info))
> >
> >
> > def gen_array(name, element_type):
> > @@ -199,22 +199,22 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
> > # Special case for our lone builtin enum type
> > # TODO use something cleaner than existence of info
> > if not info:
> > - self._btin += gen_enum(name, values, prefix)
> > + self._btin += gen_enum(None, name, values, prefix)
> > if do_builtins:
> > self.defn += gen_enum_lookup(name, values, prefix)
> > else:
> > - self._fwdecl += gen_enum(name, values, prefix)
> > + self._fwdecl += gen_enum(info, name, values, prefix)
> > self.defn += gen_enum_lookup(name, values, prefix)
> >
> > def visit_array_type(self, name, info, element_type):
> > if isinstance(element_type, QAPISchemaBuiltinType):
> > - self._btin += gen_fwd_object_or_array(name)
> > + self._btin += gen_fwd_object_or_array(None, name)
> > self._btin += gen_array(name, element_type)
> > self._btin += gen_type_cleanup_decl(name)
> > if do_builtins:
> > self.defn += gen_type_cleanup(name)
> > else:
> > - self._fwdecl += gen_fwd_object_or_array(name)
> > + self._fwdecl += gen_fwd_object_or_array(info, name)
> > self.decl += gen_array(name, element_type)
> > self._gen_type_cleanup(name)
> >
> > @@ -222,7 +222,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
> > # Nothing to do for the special empty builtin
> > if name == 'q_empty':
> > return
> > - self._fwdecl += gen_fwd_object_or_array(name)
> > + self._fwdecl += gen_fwd_object_or_array(info, name)
> > self.decl += gen_object(name, base, members, variants)
> > if base and not base.is_implicit():
> > self.decl += gen_upcast(name, base)
> > @@ -233,7 +233,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
> > self._gen_type_cleanup(name)
> >
> > def visit_alternate_type(self, name, info, variants):
> > - self._fwdecl += gen_fwd_object_or_array(name)
> > + self._fwdecl += gen_fwd_object_or_array(info, name)
> > self.decl += gen_object(name, None, [variants.tag_member],
> variants)
> > self._gen_type_cleanup(name)
>
> No comment gets generated before built-in types such as QType and
> anyList. That's okay, but perhaps the commit message could be a bit
> more precise.
>
>
ok