Re: [PATCH v3 2/9] qapi: move gen_if/gen_endif to QAPISchemaIfCond

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 4/29/21 9:40 AM, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Move the generating function to the QAPISchemaIfCond class.
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

[...]

> Tested-by: John Snow <jsnow@redhat.com>
>
> Seems fine, though I'm a lot less sure of baking the C-specific stuff
> right into the class -- I want a bit more distance to the C output 
> instead of less.
>
> (Though I do admit that I'm quite fond of centralizing things into
> classes like this myself. I think if we want to add Rust, Go, Python
> and other generators that it won't scale the way we want it to.)

I share this concern.

In a compiler, we commonly have one or more intermediate
representations, and several separate tasks that work on an IR.

The classical organisation is to define a "thin" IR data type, and have
the tasks work on that.

If you are (or your implementation language is) religious about OO, keep
a task separate gets a bit more complicated.  The visitor pattern is a
common solution.  The temptation to just put it in the IR class and call
it a day is always there, of course.

schema.py is my attempt at writing compiler in an OO way.  It's not 100%
pure.  For instance, there is QAPISchemaEntity.c_name().  Note that most
of the actual work is done *outside* schema.py, in function c_name().
The separation could certainly be cleaner.  But this is simple, and it
feels tolerably clean to me.

However, such things do add up.  At some point, we need to stop, think,
and maybe refactor.  I haven't looked at the patch, so I can't say
whether this patch gets us to this point already.