Re: [Qemu-devel] [PATCH v2 19/54] qapi: add #if/#endif helpers

[Marc-André Lureau]

On Tue, Sep 5, 2017 at 11:32 AM, Markus Armbruster <address@hidden> wrote:
> Marc-André Lureau <address@hidden> writes:
>
>> Add helpers to generate #if/#endif and wrap visitor methods generating
>> code. Used in the following patches.
>>
>> Signed-off-by: Marc-André Lureau <address@hidden>
>> ---
>>  scripts/qapi.py | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 51 insertions(+)
>>
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index 86845a28f9..52099332f1 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -1892,6 +1892,57 @@ def guardend(name):
>>                   name=guardname(name))
>>
>>
>> +def gen_if(ifcond):
>> +    if not ifcond:
>> +        return ''
>> +    if isinstance(ifcond, str):
>> +        ifcond = [ifcond]
>> +    # Not using mcgen() because we don't want indent or \n stipped
>
> s/stipped/stripped/ I guess.
>
> Which newline exactly don't you want to strip?

My first attempt at using mcgen() was a fail.

>
> Here's what mcgen() currently does:
>
> * Strip one leading newline, so that this works:
>
>       mcgen('''
>   line 1
>   line 2
>   ''')
>
> * Interpolate keywords
>
> * Indent by @indent_level spaces
>
> * Strip @eatspace.
>

So we don't need much from mcgen(), especailly, we don't want indent
on preprocessor lines. I have added a patch to fix that. And now, I
switch to mcgen() for more consistency.

>> +    ret = '\n'
>> +    for ifc in ifcond:
>> +        ret += '#if %s\n' % ifc
>> +    ret += '\n'
>> +    return ret
>> +
>> +
>> +def gen_endif(ifcond):
>> +    if not ifcond:
>> +        return ''
>> +    if isinstance(ifcond, str):
>> +        ifcond = [ifcond]
>> +    # Not using mcgen() because we don't want indent or \n stipped
>
> Likewise.
>
>> +    ret = '\n'
>> +    for ifc in reversed(ifcond):
>> +        ret += '#endif /* %s */\n' % ifc
>> +    ret += '\n'
>> +    return ret
>
> I guess factoring out the common parts of gen_if() and gen_endif() would
> make the code longer and more complex.
>
>> +
>> +
>> +# Wrap a method to add #if / #endif to generated code, only if some
>> +# code was generated.
>> +# self must have 'if_members' listing the attributes to wrap.
>> +# The last argument of the wrapped function must be the 'ifcond'.
>> +def ifcond_decorator(func):
>> +
>> +    def func_wrapper(self, *args, **kwargs):
>> +        ifcond = args[-1]
>> +        save = {}
>> +        for mem in self.if_members:
>> +            save[mem] = getattr(self, mem)
>> +        func(self, *args, **kwargs)
>> +        for mem, val in save.items():
>> +            newval = getattr(self, mem)
>> +            if newval != val:
>> +                assert newval.startswith(val)
>> +                newval = newval[len(val):]
>> +                val += gen_if(ifcond)
>> +                val += newval
>> +                val += gen_endif(ifcond)
>> +            setattr(self, mem, val)
>> +
>> +    return func_wrapper
>> +
>> +
>>  def gen_enum_lookup(name, values, prefix=None):
>>      ret = mcgen('''
>
> My gut feeling is "too clever by half", but i'm reserving judgement
> until after review of its use.
>

Well easy to change later on too, I'd not focus on this for now.

-- 
Marc-André Lureau