[Qemu-devel] [PATCH 0/2] qapi: easier debugging of introspection file

[Qemu-devel] [PATCH 0/2] qapi: easier debugging of introspection file

[Eric Blake]

When inspecting the generated qapi-introspect.c (for debugging,
or to see what QAPI changes are user visible vs. internal only),
the fact that we've intentionally masked names from the QMP client
makes it harder to tie back generated code back to the original
QAPI .json files.  We have a -u switch to qapi-gen for temporarily
bypassing the name masking, but that's a rather heavy-handed
tactic just for some temporary debugging.  Better is to just make
the generated file include strategic comments.

Eric Blake (2):
  qapi: Add comments to aid debugging generated introspection
  qapi: Drop qapi-gen --unmask option

 scripts/qapi-gen.py        |  5 +----
 scripts/qapi/introspect.py | 19 ++++++++++---------
 2 files changed, 11 insertions(+), 13 deletions(-)

-- 
2.14.4

[Qemu-devel] [PATCH 1/2] qapi: Add comments to aid debugging generated introspection

[Eric Blake]

We consciously chose in commit 1a9a507b to hide QAPI type names
from the generated introspection output, but added a command line
option -u to unmask the type name when doing a debug build.  At
that time, we generated a monolithic C string, so there was no
better way to do things (we could not really inject 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.  But this switch has also made it easier
to interject strategic comments.  That commit also forgot to
delete some now-stale comments about long generated line lengths.

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 to the introspection data.  But this is still a lot
more convenient than having to regenerate the file with the
unmask operation temporarily turned on.

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

| @@ -14927,6 +15410,7 @@
|          {}
|      })),
|      QLIT_QDICT(((QLitDictEntry[]) {
| +        /* QCryptoBlockInfoLUKSSlot */
|          { "members", QLIT_QLIST(((QLitObject[]) {
|              QLIT_QDICT(((QLitDictEntry[]) {
|                  { "name", QLIT_QSTR("active") },
|

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi/introspect.py | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 6ad198ae5b5..b37160292bc 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -35,10 +35,14 @@ def to_qlit(obj, level=0, suppress_first_indent=False):
     elif isinstance(obj, dict):
         elts = []
         for key, value in sorted(obj.items()):
+            if key == 'comment':
+                continue
             elts.append(indent(level + 1) + '{ %s, %s }' %
                         (to_c_string(key), to_qlit(value, level + 1, True)))
         elts.append(indent(level + 1) + '{}')
         ret += 'QLIT_QDICT(((QLitDictEntry[]) {\n'
+        if obj.get('comment'):
+            ret += indent(level + 1) + '/* %s */\n' % obj['comment']
         ret += ',\n'.join(elts) + '\n'
         ret += indent(level) + '}))'
     elif isinstance(obj, bool):
@@ -78,7 +82,6 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor):
         for typ in self._used_types:
             typ.visit(self)
         # generate C
-        # TODO can generate awfully long lines
         name = c_name(self._prefix, protect=False) + 'qmp_schema_qlit'
         self._genh.add(mcgen('''
 #include "qapi/qmp/qlit.h"
@@ -118,8 +121,8 @@ const QLitObject %(c_name)s = %(c_string)s;
         if typ not in self._used_types:
             self._used_types.append(typ)
         # Clients should examine commands and events, not types.  Hide
-        # type names to reduce the temptation.  Also saves a few
-        # characters.
+        # type names as integers to reduce the temptation, but provide
+        # comments for debugging aid.
         if isinstance(typ, QAPISchemaBuiltinType):
             return typ.name
         if isinstance(typ, QAPISchemaArrayType):
@@ -128,6 +131,8 @@ const QLitObject %(c_name)s = %(c_string)s;

     def _gen_qlit(self, name, mtype, obj):
         if mtype not in ('command', 'event', 'builtin', 'array'):
+            if not self._unmask:
+                obj['comment'] = name
             name = self._name(name)
         obj['name'] = name
         obj['meta-type'] = mtype
-- 
2.14.4

Re: [Qemu-devel] [PATCH 1/2] qapi: Add comments to aid debugging generated introspection

[Eric Blake]

On 06/29/2018 02:55 PM, Eric Blake wrote:
> We consciously chose in commit 1a9a507b to hide QAPI type names
> from the generated introspection output, but added a command line
> option -u to unmask the type name when doing a debug build.  At
> that time, we generated a monolithic C string, so there was no
> better way to do things (we could not really inject 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.  But this switch has also made it easier
> to interject strategic comments.  That commit also forgot to
> delete some now-stale comments about long generated line lengths.
> 
> 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 to the introspection data.  But this is still a lot
> more convenient than having to regenerate the file with the
> unmask operation temporarily turned on.
...

> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   scripts/qapi/introspect.py | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)

Misses a change to docs/devel/qapi-code.gen.tx:

diff --git i/docs/devel/qapi-code-gen.txt w/docs/devel/qapi-code-gen.txt
index 88a70e4d451..0c98c7d7b6c 100644
--- i/docs/devel/qapi-code-gen.txt
+++ w/docs/devel/qapi-code-gen.txt
@@ -1392,6 +1392,7 @@ Example:
              { }
          })),
          QLIT_QDICT(((QLitDictEntry[]) {
+            /* q_empty */
              { "members", QLIT_QLIST(((QLitObject[]) {
                  { }
              })) },

> @@ -118,8 +121,8 @@ const QLitObject %(c_name)s = %(c_string)s;
>           if typ not in self._used_types:
>               self._used_types.append(typ)
>           # Clients should examine commands and events, not types.  Hide
> -        # type names to reduce the temptation.  Also saves a few
> -        # characters.
> +        # type names as integers to reduce the temptation, but provide
> +        # comments for debugging aid.

When I wrote the patch, I assumed the final sentence of the comment was 
talking about fewer characters in the generated .c file (I nuked it, 
since adding comments makes for a longer, not shorter, generated .c 
file); but now that I'm writing this email, I see it could also refer to 
fewer characters sent over the wire in the QMP command (which length is 
unchanged by this patch, but was a 13KiB savings compared to the 
unmasked version, per commit 1a9a507b).  So feel free to bikeshed this 
comment.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 1/2] qapi: Add comments to aid debugging generated introspection

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 06/29/2018 02:55 PM, Eric Blake wrote:
>> We consciously chose in commit 1a9a507b to hide QAPI type names
>> from the generated introspection output, but added a command line
>> option -u to unmask the type name when doing a debug build.  At
>> that time, we generated a monolithic C string, so there was no
>> better way to do things (we could not really inject 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.  But this switch has also made it easier
>> to interject strategic comments.  That commit also forgot to
>> delete some now-stale comments about long generated line lengths.
>>
>> 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 to the introspection data.  But this is still a lot
>> more convenient than having to regenerate the file with the
>> unmask operation temporarily turned on.
> ...
>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
>>   scripts/qapi/introspect.py | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>
> Misses a change to docs/devel/qapi-code.gen.tx:
>
> diff --git i/docs/devel/qapi-code-gen.txt w/docs/devel/qapi-code-gen.txt
> index 88a70e4d451..0c98c7d7b6c 100644
> --- i/docs/devel/qapi-code-gen.txt
> +++ w/docs/devel/qapi-code-gen.txt
> @@ -1392,6 +1392,7 @@ Example:
>              { }
>          })),
>          QLIT_QDICT(((QLitDictEntry[]) {
> +            /* q_empty */
>              { "members", QLIT_QLIST(((QLitObject[]) {
>                  { }
>              })) },

Yes.

>> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
>> index 6ad198ae5b5..b37160292bc 100644
>> --- a/scripts/qapi/introspect.py
>> +++ b/scripts/qapi/introspect.py
>> @@ -35,10 +35,14 @@ def to_qlit(obj, level=0, suppress_first_indent=False):
>>      elif isinstance(obj, dict):
>>          elts = []
>>          for key, value in sorted(obj.items()):
>> +            if key == 'comment':
>> +                continue
>>              elts.append(indent(level + 1) + '{ %s, %s }' %
>>                          (to_c_string(key), to_qlit(value, level + 1, True)))
>>          elts.append(indent(level + 1) + '{}')
>>          ret += 'QLIT_QDICT(((QLitDictEntry[]) {\n'
>> +        if obj.get('comment'):
>> +            ret += indent(level + 1) + '/* %s */\n' % obj['comment']
>>          ret += ',\n'.join(elts) + '\n'
>>          ret += indent(level) + '}))'
>>      elif isinstance(obj, bool):

to_qlit() converts from a natural representation of JSON in Python to
JSON text.

Your patch breaks it for JSON objects that have a member named
'comment'.

Related work: Marc-André's "[PATCH v6 09/15] qapi-introspect: add
preprocessor conditions to generated QLit" uses tuples to augment JSON.
He uses them for conditionals, but the technique could be generalized,
say from his (jobj, ifcond) to (jobj, { 'if': ifcond, '#': comment }.

>> @@ -78,7 +82,6 @@ class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor):
>>          for typ in self._used_types:
>>              typ.visit(self)
>>          # generate C
>> -        # TODO can generate awfully long lines
>>          name = c_name(self._prefix, protect=False) + 'qmp_schema_qlit'
>>          self._genh.add(mcgen('''
>>  #include "qapi/qmp/qlit.h"
>> @@ -118,8 +121,8 @@ const QLitObject %(c_name)s = %(c_string)s;
>>           if typ not in self._used_types:
>>               self._used_types.append(typ)
>>          # Clients should examine commands and events, not types.  Hide
>> -        # type names to reduce the temptation.  Also saves a few
>> -        # characters.
>> +        # type names as integers to reduce the temptation, but provide
>> +        # comments for debugging aid.
>
> When I wrote the patch, I assumed the final sentence of the comment
> was talking about fewer characters in the generated .c file (I nuked
> it, since adding comments makes for a longer, not shorter, generated
> .c file); but now that I'm writing this email, I see it could also
> refer to fewer characters sent over the wire in the QMP command (which

Yes, that was my intent.

> length is unchanged by this patch, but was a 13KiB savings compared to
> the unmasked version, per commit 1a9a507b).  So feel free to bikeshed
> this comment.

I think I'd merely clarify the comment's last sentence by adding "on the
wire", ...

>>          if isinstance(typ, QAPISchemaBuiltinType):
>>              return typ.name
>>          if isinstance(typ, QAPISchemaArrayType):
>> @@ -128,6 +131,8 @@ const QLitObject %(c_name)s = %(c_string)s;
>> 
>>      def _gen_qlit(self, name, mtype, obj):
>>          if mtype not in ('command', 'event', 'builtin', 'array'):
>> +            if not self._unmask:

... and perhaps briefly explain here why we generate a comment.

>> +                obj['comment'] = name
>>              name = self._name(name)
>>          obj['name'] = name
>>          obj['meta-type'] = mtype

Re: [Qemu-devel] [PATCH 1/2] qapi: Add comments to aid debugging generated introspection

[Eric Blake]

On 07/02/2018 01:43 PM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 06/29/2018 02:55 PM, Eric Blake wrote:
>>> We consciously chose in commit 1a9a507b to hide QAPI type names
>>> from the generated introspection output, but added a command line
>>> option -u to unmask the type name when doing a debug build.  At
>>> that time, we generated a monolithic C string, so there was no
>>> better way to do things (we could not really inject 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.  But this switch has also made it easier
>>> to interject strategic comments.  That commit also forgot to
>>> delete some now-stale comments about long generated line lengths.
>>>
>>> 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 to the introspection data.  But this is still a lot
>>> more convenient than having to regenerate the file with the
>>> unmask operation temporarily turned on.
>> ...

> to_qlit() converts from a natural representation of JSON in Python to
> JSON text.
> 
> Your patch breaks it for JSON objects that have a member named
> 'comment'.

Hmm.  None do, presently.  And I don't see where it would be permitted 
in the QAPI schema for query-qmp-schema.

> 
> Related work: Marc-André's "[PATCH v6 09/15] qapi-introspect: add
> preprocessor conditions to generated QLit" uses tuples to augment JSON.
> He uses them for conditionals, but the technique could be generalized,
> say from his (jobj, ifcond) to (jobj, { 'if': ifcond, '#': comment }.

Yeah, I can probably rebase on top of that (and at this point, it's late 
enough to probably have missed 3.0 anyways, so I'm not time-crunched for 
getting it done right now).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 1/2] qapi: Add comments to aid debugging generated introspection

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 07/02/2018 01:43 PM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>
>>> On 06/29/2018 02:55 PM, Eric Blake wrote:
>>>> We consciously chose in commit 1a9a507b to hide QAPI type names
>>>> from the generated introspection output, but added a command line
>>>> option -u to unmask the type name when doing a debug build.  At
>>>> that time, we generated a monolithic C string, so there was no
>>>> better way to do things (we could not really inject 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.  But this switch has also made it easier
>>>> to interject strategic comments.  That commit also forgot to
>>>> delete some now-stale comments about long generated line lengths.
>>>>
>>>> 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 to the introspection data.  But this is still a lot
>>>> more convenient than having to regenerate the file with the
>>>> unmask operation temporarily turned on.
>>> ...
>
>> to_qlit() converts from a natural representation of JSON in Python to
>> JSON text.
>>
>> Your patch breaks it for JSON objects that have a member named
>> 'comment'.
>
> Hmm.  None do, presently.  And I don't see where it would be permitted
> in the QAPI schema for query-qmp-schema.

So far, to_qlit() is completely oblivious of the schema; it just
crunches JSON.  I'd like to keep it that way, if practical.

>> Related work: Marc-André's "[PATCH v6 09/15] qapi-introspect: add
>> preprocessor conditions to generated QLit" uses tuples to augment JSON.
>> He uses them for conditionals, but the technique could be generalized,
>> say from his (jobj, ifcond) to (jobj, { 'if': ifcond, '#': comment }.
>
> Yeah, I can probably rebase on top of that (and at this point, it's
> late enough to probably have missed 3.0 anyways, so I'm not
> time-crunched for getting it done right now).

It's a lovely improvement for developers, but it'll be just as lovely in
3.1 :)

Re: [Qemu-devel] [PATCH 1/2] qapi: Add comments to aid debugging generated introspection

[Eric Blake]

On 06/29/2018 02:55 PM, Eric Blake wrote:

> The generated qapi-introspect.c changes only with the addition
> of comments, such as:
> 
> | @@ -14927,6 +15410,7 @@
> |          {}
> |      })),
> |      QLIT_QDICT(((QLitDictEntry[]) {
> | +        /* QCryptoBlockInfoLUKSSlot */
> |          { "members", QLIT_QLIST(((QLitObject[]) {
> |              QLIT_QDICT(((QLitDictEntry[]) {
> |                  { "name", QLIT_QSTR("active") },

---
...

> @@ -128,6 +131,8 @@ const QLitObject %(c_name)s = %(c_string)s;
> 
>       def _gen_qlit(self, name, mtype, obj):
>           if mtype not in ('command', 'event', 'builtin', 'array'):
> +            if not self._unmask:
> +                obj['comment'] = name

If desired, we could change this to:

if not self._unmask:
     obj['comment'] = '"%s" = %s' % (self._name(name), name)

to produce comments like /* "0" = q_empty */

for even easier debugging (if I see a line containing "ret-type", 
QLIT_QSTR("0"), then I can grep for '"0" =' to directly look up the type 
name, rather than having to grep for 'name.*"0"' then scrolling back to 
find the associated comment).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH 2/2] qapi: Drop qapi-gen --unmask option

[Eric Blake]

Now that we have useful access to the type name as a comment
in the generated qapi-introspect.c, we don't need to regenerate
code with a temporary -u option just to get at type names.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 scripts/qapi-gen.py        |  5 +----
 scripts/qapi/introspect.py | 12 ++++--------
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/scripts/qapi-gen.py b/scripts/qapi-gen.py
index 3d98ca2e0c6..6ec7e481b1b 100755
--- a/scripts/qapi-gen.py
+++ b/scripts/qapi-gen.py
@@ -26,9 +26,6 @@ def main(argv):
                         help="write output to directory OUTPUT_DIR")
     parser.add_argument('-p', '--prefix', action='store', default='',
                         help="prefix for symbols")
-    parser.add_argument('-u', '--unmask-non-abi-names', action='store_true',
-                        dest='unmask',
-                        help="expose non-ABI names in introspection")
     parser.add_argument('schema', action='store')
     args = parser.parse_args()

@@ -49,7 +46,7 @@ def main(argv):
     gen_visit(schema, args.output_dir, args.prefix, args.builtins)
     gen_commands(schema, args.output_dir, args.prefix)
     gen_events(schema, args.output_dir, args.prefix)
-    gen_introspect(schema, args.output_dir, args.prefix, args.unmask)
+    gen_introspect(schema, args.output_dir, args.prefix)
     gen_doc(schema, args.output_dir, args.prefix)


diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index b37160292bc..8dd4136c0af 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -58,11 +58,10 @@ def to_c_string(string):

 class QAPISchemaGenIntrospectVisitor(QAPISchemaMonolithicCVisitor):

-    def __init__(self, prefix, unmask):
+    def __init__(self, prefix):
         QAPISchemaMonolithicCVisitor.__init__(
             self, prefix, 'qapi-introspect',
             ' * QAPI/QMP schema introspection', __doc__)
-        self._unmask = unmask
         self._schema = None
         self._qlits = []
         self._used_types = []
@@ -104,8 +103,6 @@ const QLitObject %(c_name)s = %(c_string)s;
         return not isinstance(entity, QAPISchemaType)

     def _name(self, name):
-        if self._unmask:
-            return name
         if name not in self._name_map:
             self._name_map[name] = '%d' % len(self._name_map)
         return self._name_map[name]
@@ -131,8 +128,7 @@ const QLitObject %(c_name)s = %(c_string)s;

     def _gen_qlit(self, name, mtype, obj):
         if mtype not in ('command', 'event', 'builtin', 'array'):
-            if not self._unmask:
-                obj['comment'] = name
+            obj['comment'] = name
             name = self._name(name)
         obj['name'] = name
         obj['meta-type'] = mtype
@@ -188,7 +184,7 @@ const QLitObject %(c_name)s = %(c_string)s;
         self._gen_qlit(name, 'event', {'arg-type': self._use_type(arg_type)})


-def gen_introspect(schema, output_dir, prefix, opt_unmask):
-    vis = QAPISchemaGenIntrospectVisitor(prefix, opt_unmask)
+def gen_introspect(schema, output_dir, prefix):
+    vis = QAPISchemaGenIntrospectVisitor(prefix)
     schema.visit(vis)
     vis.write(output_dir)
-- 
2.14.4

Re: [Qemu-devel] [PATCH 2/2] qapi: Drop qapi-gen --unmask option

[Eduardo Habkost]

On Fri, Jun 29, 2018 at 02:55:44PM -0500, Eric Blake wrote:
> Now that we have useful access to the type name as a comment
> in the generated qapi-introspect.c, we don't need to regenerate
> code with a temporary -u option just to get at type names.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [Qemu-devel] [PATCH 2/2] qapi: Drop qapi-gen --unmask option

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> Now that we have useful access to the type name as a comment
> in the generated qapi-introspect.c, we don't need to regenerate
> code with a temporary -u option just to get at type names.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>

I occasionally feed output of query-qmp-schema to ad hoc Python scripts.
My last one searched for object types containing non-string members
(directly or indirectly).  -u remains useful there, and having to
recompile is not a problem.  I'd prefer to keep it.

Re: [Qemu-devel] [PATCH 2/2] qapi: Drop qapi-gen --unmask option

[Eric Blake]

On 07/02/2018 01:48 PM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> Now that we have useful access to the type name as a comment
>> in the generated qapi-introspect.c, we don't need to regenerate
>> code with a temporary -u option just to get at type names.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> I occasionally feed output of query-qmp-schema to ad hoc Python scripts.
> My last one searched for object types containing non-string members
> (directly or indirectly).  -u remains useful there, and having to
> recompile is not a problem.  I'd prefer to keep it.

Would it be any easier to argue for the removal of -u if the generated 
output resembled:

{ "ret-type", QLIT_QSTR("1") /* Foo */ },

that is, sticking in the comment everywhere the type name is mangled to 
an integer, rather than just once per type?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 2/2] qapi: Drop qapi-gen --unmask option

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 07/02/2018 01:48 PM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>
>>> Now that we have useful access to the type name as a comment
>>> in the generated qapi-introspect.c, we don't need to regenerate
>>> code with a temporary -u option just to get at type names.
>>>
>>> Signed-off-by: Eric Blake <eblake@redhat.com>
>>
>> I occasionally feed output of query-qmp-schema to ad hoc Python scripts.
>> My last one searched for object types containing non-string members
>> (directly or indirectly).  -u remains useful there, and having to
>> recompile is not a problem.  I'd prefer to keep it.
>
> Would it be any easier to argue for the removal of -u if the generated
> output resembled:
>
> { "ret-type", QLIT_QSTR("1") /* Foo */ },
>
> that is, sticking in the comment everywhere the type name is mangled
> to an integer, rather than just once per type?

Doesn't help with my use case.  Let me elaborate on it.

My ad hoc Python script gets the output of query-qmp-schema like this:

    pipe = subprocess.Popen('socat %s STDIO' % qmp_socket,
                            shell=True,
                            stdin=subprocess.PIPE, stdout=subprocess.PIPE)
    (out, err) = pipe.communicate('{ "execute": "qmp_capabilities" }\n'
                                  '{ "execute": "query-qmp-schema" }\n')
    # three lines: greeting, output of qmp_capabilities, query-qmp-schema
    return json.loads(out.split('\n')[2])['return']

This returns an abstract syntax tree, represented in Python the obvious
way.  I can then explore it in Python, say search for object types with
certain properties.  For example, commit 2860b2b2cb8:

    Thus, the flaw puts an artificial restriction on the QAPI schema: we
    can't have potentially empty objects and arrays within
    BlockdevOptions, except when they're optional and "empty" has the same
    meaning as "absent".

--> Our QAPI schema satisfies this restriction (I checked), but it's a
    trap for the unwary, and a temptation to employ awkward workarounds
    for the wary.  Let's get rid of it.

I checked with a Python script that read the schema as shown above.
Without -u, I'd have to revert the identifier hiding.  I could certainly
write some more Python to read the mapping from the generated C, but
that feels like busy work.

Re: [Qemu-devel] [PATCH 2/2] qapi: Drop qapi-gen --unmask option

[Eric Blake]

On 07/03/2018 12:51 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 07/02/2018 01:48 PM, Markus Armbruster wrote:
>>> Eric Blake <eblake@redhat.com> writes:
>>>
>>>> Now that we have useful access to the type name as a comment
>>>> in the generated qapi-introspect.c, we don't need to regenerate
>>>> code with a temporary -u option just to get at type names.
>>>>

>      # three lines: greeting, output of qmp_capabilities, query-qmp-schema
>      return json.loads(out.split('\n')[2])['return']
> 
> This returns an abstract syntax tree, represented in Python the obvious
> way.  I can then explore it in Python, say search for object types with
> certain properties.  For example, commit 2860b2b2cb8:
> 
>      Thus, the flaw puts an artificial restriction on the QAPI schema: we
>      can't have potentially empty objects and arrays within
>      BlockdevOptions, except when they're optional and "empty" has the same
>      meaning as "absent".
> 
> --> Our QAPI schema satisfies this restriction (I checked), but it's a
>      trap for the unwary, and a temptation to employ awkward workarounds
>      for the wary.  Let's get rid of it.
> 
> I checked with a Python script that read the schema as shown above.
> Without -u, I'd have to revert the identifier hiding.  I could certainly
> write some more Python to read the mapping from the generated C, but
> that feels like busy work.

Good argument. Okay, I'm dropping this patch, and tweaking the other 
patch commit message to explain that -u is still useful, even with the 
addition of comment aids.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org