qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH RFC 03/21] qapi: New classes QAPIGenC, QAPIGenH,


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH RFC 03/21] qapi: New classes QAPIGenC, QAPIGenH, QAPIGenDoc
Date: Tue, 06 Feb 2018 08:28:28 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux)

Eric Blake <address@hidden> writes:

> On 02/03/2018 02:49 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>> 
>>> On 02/02/2018 07:03 AM, Markus Armbruster wrote:
>>>> 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.
>>>>
>
>>>>  schema = QAPISchema(input_file)
>>>> -gen = QAPISchemaGenEventVisitor()
>>>> -schema.visit(gen)
>>>> -fdef.write(gen.defn)
>>>> -fdecl.write(gen.decl)
>>>> +vis = QAPISchemaGenEventVisitor()
>>>> +schema.visit(vis)
>>>> +genc.body(vis.defn)
>>>> +genh.body(vis.decl)
>>>
>>> I don't know if it is worth a sentence in the commit message that the
>>> visitor variable is renamed from 'gen' to 'vis' for less confusion with
>>> the new class instances 'genc' and 'genh'.
>> 
>> Did the rename give you pause when reviewing?
>
> Enough to question whether it was intentional, since it wasn't mentioned
> in the commit message (I obviously figured out that it was intentional
> and useful, but the fact that I even pointed it out meant that I did
> pause during the review).

Okay, I'll try to make it clearer.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]