Michael Roth <mdr...@linux.vnet.ibm.com> writes:
> Quoting Markus Armbruster (2018-02-11 03:35:43)
>> These classes encapsulate accumulating and writing output.
>>
>> Convert C code generation to QAPIGenC and QAPIGenH. The conversion is
>> rather shallow: most of the output accumulation is not converted.
>> Left for later.
>>
>> The indentation machinery uses a single global variable indent_level,
>> even though we generally interleave creation of a .c and its .h. It
>> should become instance variable of QAPIGenC. Also left for later.
>>
>> Documentation generation isn't converted, and QAPIGenDoc isn't used.
>> This will change shortly.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> Reviewed-by: Eric Blake <ebl...@redhat.com>
>> Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>
> 2 minor nits below, but in any case:
>
> Reviewed-by: Michael Roth <mdr...@linux.vnet.ibm.com>
>
>> ---
>> scripts/qapi-commands.py | 23 +++++------
>> scripts/qapi-event.py | 22 ++++++-----
>> scripts/qapi-introspect.py | 18 +++++----
>> scripts/qapi-types.py | 22 ++++++-----
>> scripts/qapi-visit.py | 22 ++++++-----
>> scripts/qapi.py | 99
>> +++++++++++++++++++++++++---------------------
>> 6 files changed, 112 insertions(+), 94 deletions(-)
>>
>> diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
>> index c3aa52fce1..8d38ade076 100644
>> --- a/scripts/qapi-commands.py
>> +++ b/scripts/qapi-commands.py
>> @@ -260,12 +260,10 @@ blurb = '''
>> * Schema-defined QAPI/QMP commands
>> '''
>>
>> -(fdef, fdecl) = open_output(output_dir, do_c, do_h, prefix,
>> - 'qmp-marshal.c', 'qmp-commands.h',
>> - blurb, __doc__)
>> -
>> -fdef.write(mcgen('''
>> +genc = QAPIGenC(blurb, __doc__)
>> +genh = QAPIGenH(blurb, __doc__)
>>
>> +genc.add(mcgen('''
>> #include "qemu/osdep.h"
>> #include "qemu-common.h"
>> #include "qemu/module.h"
>> @@ -280,20 +278,23 @@ fdef.write(mcgen('''
>> #include "%(prefix)sqmp-commands.h"
>>
>> ''',
>> - prefix=prefix))
>> + prefix=prefix))
>>
>> -fdecl.write(mcgen('''
>> +genh.add(mcgen('''
>> #include "%(prefix)sqapi-types.h"
>> #include "qapi/qmp/dispatch.h"
>>
>> void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
>> ''',
>> - prefix=prefix, c_prefix=c_name(prefix, protect=False)))
>> + prefix=prefix, c_prefix=c_name(prefix, protect=False)))
>>
>> schema = QAPISchema(input_file)
>> vis = QAPISchemaGenCommandVisitor()
>> schema.visit(vis)
>> -fdef.write(vis.defn)
>> -fdecl.write(vis.decl)
>> +genc.add(vis.defn)
>> +genh.add(vis.decl)
>>
>> -close_output(fdef, fdecl)
>> +if do_c:
>> + genc.write(output_dir, prefix + 'qmp-marshal.c')
>> +if do_h:
>> + genh.write(output_dir, prefix + 'qmp-commands.h')
>> diff --git a/scripts/qapi-event.py b/scripts/qapi-event.py
>> index edb9ddb650..bd7a9be3dc 100644
>> --- a/scripts/qapi-event.py
>> +++ b/scripts/qapi-event.py
>> @@ -176,11 +176,10 @@ blurb = '''
>> * Schema-defined QAPI/QMP events
>> '''
>>
>> -(fdef, fdecl) = open_output(output_dir, do_c, do_h, prefix,
>> - 'qapi-event.c', 'qapi-event.h',
>> - blurb, __doc__)
>> +genc = QAPIGenC(blurb, __doc__)
>> +genh = QAPIGenH(blurb, __doc__)
>>
>> -fdef.write(mcgen('''
>> +genc.add(mcgen('''
>> #include "qemu/osdep.h"
>> #include "qemu-common.h"
>> #include "%(prefix)sqapi-event.h"
>> @@ -191,21 +190,24 @@ fdef.write(mcgen('''
>> #include "qapi/qmp-event.h"
>>
>> ''',
>> - prefix=prefix))
>> + prefix=prefix))
>>
>> -fdecl.write(mcgen('''
>> +genh.add(mcgen('''
>> #include "qapi/util.h"
>> #include "%(prefix)sqapi-types.h"
>>
>> ''',
>> - prefix=prefix))
>> + prefix=prefix))
>>
>> event_enum_name = c_name(prefix + 'QAPIEvent', protect=False)
>>
>> schema = QAPISchema(input_file)
>> vis = QAPISchemaGenEventVisitor()
>> schema.visit(vis)
>> -fdef.write(vis.defn)
>> -fdecl.write(vis.decl)
>> +genc.add(vis.defn)
>> +genh.add(vis.decl)
>>
>> -close_output(fdef, fdecl)
>> +if do_c:
>> + genc.write(output_dir, prefix + 'qapi-event.c')
>> +if do_h:
>> + genh.write(output_dir, prefix + 'qapi-event.h')
>> diff --git a/scripts/qapi-introspect.py b/scripts/qapi-introspect.py
>> index ebe8706f41..3d65690fe3 100644
>> --- a/scripts/qapi-introspect.py
>> +++ b/scripts/qapi-introspect.py
>> @@ -181,21 +181,23 @@ blurb = '''
>> * QAPI/QMP schema introspection
>> '''
>>
>> -(fdef, fdecl) = open_output(output_dir, do_c, do_h, prefix,
>> - 'qmp-introspect.c', 'qmp-introspect.h',
>> - blurb, __doc__)
>> +genc = QAPIGenC(blurb, __doc__)
>> +genh = QAPIGenH(blurb, __doc__)
>>
>> -fdef.write(mcgen('''
>> +genc.add(mcgen('''
>> #include "qemu/osdep.h"
>> #include "%(prefix)sqmp-introspect.h"
>>
>> ''',
>> - prefix=prefix))
>> + prefix=prefix))
>>
>> schema = QAPISchema(input_file)
>> vis = QAPISchemaGenIntrospectVisitor(opt_unmask)
>> schema.visit(vis)
>> -fdef.write(vis.defn)
>> -fdecl.write(vis.decl)
>> +genc.add(vis.defn)
>> +genh.add(vis.decl)
>>
>> -close_output(fdef, fdecl)
>> +if do_c:
>> + genc.write(output_dir, prefix + 'qmp-introspect.c')
>> +if do_h:
>> + genh.write(output_dir, prefix + 'qmp-introspect.h')
>> diff --git a/scripts/qapi-types.py b/scripts/qapi-types.py
>> index 4db8424da1..c0ac879beb 100644
>> --- a/scripts/qapi-types.py
>> +++ b/scripts/qapi-types.py
>> @@ -180,7 +180,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaVisitor):
>> self.decl = ''
>> self.defn = ''
>> self._fwdecl = ''
>> - self._btin = guardstart('QAPI_TYPES_BUILTIN')
>> + self._btin = '\n' + guardstart('QAPI_TYPES_BUILTIN')
>
> Minor nit, but if we compensate for guardstart() change here, shouldn't
> we do the same in QAPISchemaGenVisitVisitor? Both are cosmetic (though
> Visit is in less need since it has extra an extra newline already,
> but changing one and not the other to compensate here makes the patch
> appear less mechanical, and the resulting formatting fix-up gets dropped
> later in the series anyway)
Yes, it's better to keep the two consistent.
>> def visit_end(self):
>> self.decl = self._fwdecl + self.decl
>
> <snip>
>
>> +class QAPIGenDoc(QAPIGen):
>> + def _top(self, fname):
>> + return (QAPIGen._top(self, fname)
>> + + '@c AUTOMATICALLY GENERATED, DO NOT MODIFY\n\n')
>
> The whitespace change in
> "qapi/types qapi/visit: Generate built-in stuff into separate files" should
> probably be squashed in here:
>
> class QAPIGenDoc(QAPIGen):
> +
> def _top(self, fname):
> return (QAPIGen._top(self, fname)
> + '@c AUTOMATICALLY GENERATED, DO NOT MODIFY\n\n')
Yes.
