All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Subject: [Qemu-devel] [PULL 6/6] qapi: Add comments to aid debugging generated introspection
Date: Tue, 28 Aug 2018 21:10:48 +0200	[thread overview]
Message-ID: <20180828191048.29806-7-armbru@redhat.com> (raw)
In-Reply-To: <20180828191048.29806-1-armbru@redhat.com>

From: Eric Blake <eblake@redhat.com>

We consciously chose in commit 1a9a507b to hide QAPI type names
from the introspection output on the wire, but added a command
line option -u to unmask the type name when doing a debug build.
The unmask option still remains useful to some other forms of
automated analysis, so it will not be removed; however, when it
is not in use, the generated .c file can be hard to read.  At
the time when we first introduced masking, the generated file
consisted only of a monolithic C string, so there was no clean
way to inject any comments.

Later, in commit 7d0f982b, we switched the generation to output
a QLit object, in part to make it easier for future addition of
conditional compilation.  In fact, commit d626b6c1 took advantage
of this by passing a tuple instead of a bare object for encoding
the output of conditionals.  By extending that tuple, we can now
interject strategic comments.

For now, type name debug aid comments are only output once per
meta-type, rather than at all uses of the number used to encode
the type within the introspection data.  But this is still a lot
more convenient than having to regenerate the file with the
unmask operation temporarily turned on - merely search the
generated file for '"NNN" =' to learn the corresponding source
name and associated definition of type NNN.

The generated qapi-introspect.c changes only with the addition
of comments, such as:

| @@ -14755,6 +15240,7 @@
|          { "name", QLIT_QSTR("[485]"), },
|          {}
|      })),
| +    /* "485" = QCryptoBlockInfoLUKSSlot */
|      QLIT_QDICT(((QLitDictEntry[]) {
|          { "members", QLIT_QLIST(((QLitObject[]) {
|              QLIT_QDICT(((QLitDictEntry[]) {

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20180827213943.33524-3-eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Rebased, update to qapi-code-gen.txt corrected]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/devel/qapi-code-gen.txt |  3 +++
 scripts/qapi/introspect.py   | 27 +++++++++++++++++++++------
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 304c12d0ae..53eaf01f34 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -1428,6 +1428,7 @@ Example:
             { "name", QLIT_QSTR("MY_EVENT"), },
             {}
         })),
+        /* "0" = q_obj_my-command-arg */
         QLIT_QDICT(((QLitDictEntry[]) {
             { "members", QLIT_QLIST(((QLitObject[]) {
                 QLIT_QDICT(((QLitDictEntry[]) {
@@ -1441,6 +1442,7 @@ Example:
             { "name", QLIT_QSTR("0"), },
             {}
         })),
+        /* "1" = UserDefOne */
         QLIT_QDICT(((QLitDictEntry[]) {
             { "members", QLIT_QLIST(((QLitObject[]) {
                 QLIT_QDICT(((QLitDictEntry[]) {
@@ -1460,6 +1462,7 @@ Example:
             { "name", QLIT_QSTR("1"), },
             {}
         })),
+        /* "2" = q_empty */
         QLIT_QDICT(((QLitDictEntry[]) {
             { "members", QLIT_QLIST(((QLitObject[]) {
                 {}
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 43e81a0693..67d6106f77 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -19,12 +19,17 @@ def to_qlit(obj, level=0, suppress_first_indent=False):
         return level * 4 * ' '
 
     if isinstance(obj, tuple):
-        ifobj, ifcond = obj
-        ret = gen_if(ifcond)
+        ifobj, extra = obj
+        ifcond = extra.get('if')
+        comment = extra.get('comment')
+        ret = ''
+        if comment:
+            ret += indent(level) + '/* %s */\n' % comment
+        if ifcond:
+            ret += gen_if(ifcond)
         ret += to_qlit(ifobj, level)
-        endif = gen_endif(ifcond)
-        if endif:
-            ret += '\n' + endif
+        if ifcond:
+            ret += '\n' + gen_endif(ifcond)
         return ret
 
     ret = ''
@@ -137,11 +142,21 @@ const QLitObject %(c_name)s = %(c_string)s;
         return self._name(typ.name)
 
     def _gen_qlit(self, name, mtype, obj, ifcond):
+        extra = {}
         if mtype not in ('command', 'event', 'builtin', 'array'):
+            if not self._unmask:
+                # Output a comment to make it easy to map masked names
+                # back to the source when reading the generated output.
+                extra['comment'] = '"%s" = %s' % (self._name(name), name)
             name = self._name(name)
         obj['name'] = name
         obj['meta-type'] = mtype
-        self._qlits.append((obj, ifcond))
+        if ifcond:
+            extra['if'] = ifcond
+        if extra:
+            self._qlits.append((obj, extra))
+        else:
+            self._qlits.append(obj)
 
     def _gen_member(self, member):
         ret = {'name': member.name, 'type': self._use_type(member.type)}
-- 
2.17.1

      parent reply	other threads:[~2018-08-28 19:20 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-28 19:10 [Qemu-devel] [PULL 0/6] QAPI patches for 2018-08-28 Markus Armbruster
2018-08-28 19:10 ` [Qemu-devel] [PULL 1/6] qapi: Fix build_params() for empty parameter list Markus Armbruster
2018-08-28 19:10 ` [Qemu-devel] [PULL 2/6] qapi: Drop qapi_event_send_FOO()'s Error ** argument Markus Armbruster
2018-08-28 19:10 ` [Qemu-devel] [PULL 3/6] qapi: Emit a blank line before dummy declaration Markus Armbruster
2018-08-28 19:10 ` [Qemu-devel] [PULL 4/6] qapi: Update docs for generator changes since commit 9ee86b85267 Markus Armbruster
2018-08-28 19:10 ` [Qemu-devel] [PULL 5/6] qapi: Minor introspect.py cleanups Markus Armbruster
2018-08-28 19:10 ` Markus Armbruster [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180828191048.29806-7-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.