Hi
On Thu, Dec 7, 2017 at 3:10 PM, Markus Armbruster <arm...@redhat.com> wrote:
> Marc-André Lureau <marcandre.lur...@redhat.com> writes:
>
>> Add helpers to wrap generated code with #if/#endif lines.
>>
>> Add a function decorator that will be used to wrap visitor methods.
>> The decorator will check if code was generated before adding #if/#endif
>> lines. Used in the following patches.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>> ---
>> scripts/qapi.py | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 55 insertions(+)
>>
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index 2a8e60e975..94b735d8d6 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -1897,6 +1897,61 @@ def guardend(name):
>> name=guardname(name))
>>
>>
>> +def gen_if(ifcond):
>> + if not ifcond:
>> + return ''
>> + if isinstance(ifcond, str):
>> + ifcond = [ifcond]
>
> Perhaps we should take this normalization step in the QAPISchema
> constructors.
Yes, it's not very convenient though, I added a TODO note
>
>> + ret = ''
>> + for ifc in ifcond:
>> + ret += mcgen('''
>> +#if %(cond)s
>> +''', cond=ifc)
>> + return ret
>> +
>> +
>> +def gen_endif(ifcond):
>> + if not ifcond:
>> + return ''
>> + if isinstance(ifcond, str):
>> + ifcond = [ifcond]
>> + ret = ''
>> + for ifc in reversed(ifcond):
>> + ret += mcgen('''
>> +#endif /* %(cond)s */
>> +''', cond=ifc)
>> + return ret
>> +
>> +
>> +# Wrap a method to add #if / #endif to generated code, only if some
>> +# code was generated. The method must have an 'ifcond' argument.
>> +# self must have 'if_members' listing the attributes to wrap.
>> +def ifcond_decorator(func):
>> +
>> + def func_wrapper(self, *args, **kwargs):
>> + import inspect
>
> Is hiding imports in function a good idea?
I believe it's best to restrict the import to the scope it is being
used, especially if it's very specific to that place. Some code do
that already.
>
>> + idx = inspect.getargspec(func).args.index('ifcond')
>> + ifcond = args[idx - 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):]
>> + if newval[0] == '\n':
>> + val += '\n'
>> + newval = newval[1:]
>> + 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 still "too clever by half", but i'm reserving
> judgement until after review of its use, and exploration of
> alternatives.
>
Could easily be done as a follow-up.
thanks
--
Marc-André Lureau
