[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 07/15] qapi: add #if/#endif helpers
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v6 07/15] qapi: add #if/#endif helpers |
Date: |
Tue, 03 Jul 2018 13:53:15 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Marc-André Lureau <address@hidden> writes:
> Add helpers to wrap generated code with #if/#endif lines.
>
> Add QAPIGenCSnippet class to write C snippet code, make QAPIGenC
> inherit from it, for full C files with copyright headers etc.
It's called QAPIGenCCode now. Can touch up when I apply, along with
another instance below.
Leaves the reader wondering *why* you need to splice QAPIGenCCode
between QAPIGen and QAPIGenC. Becomes clear only in PATCH 10.
Providing the class now reduces code churn, but you should explain why.
Perhaps:
A later patch wants to use QAPIGen for generating C snippets rather
than full C files with copyright headers etc. Splice in class
QAPIGenCCode between QAPIGen and QAPIGenC.
> Add a 'with' statement context manager that will be used to wrap
> generator visitor methods. The manager will check if code was
> generated before adding #if/#endif lines on QAPIGenCSnippet
> objects. Used in the following patches.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
> scripts/qapi/common.py | 101 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 97 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
> index 1b56065a80..44eaf25850 100644
> --- a/scripts/qapi/common.py
> +++ b/scripts/qapi/common.py
> @@ -12,6 +12,7 @@
> # See the COPYING file in the top-level directory.
>
> from __future__ import print_function
> +from contextlib import contextmanager
> import errno
> import os
> import re
> @@ -1974,6 +1975,40 @@ def guardend(name):
> name=guardname(name))
>
>
> +def gen_if(ifcond):
> + ret = ''
> + for ifc in ifcond:
> + ret += mcgen('''
> +#if %(cond)s
> +''', cond=ifc)
> + return ret
> +
> +
> +def gen_endif(ifcond):
> + ret = ''
> + for ifc in reversed(ifcond):
> + ret += mcgen('''
> +#endif /* %(cond)s */
> +''', cond=ifc)
> + return ret
> +
> +
> +def wrap_ifcond(ifcond, before, after):
Looks like a helper method. Name it _wrap_ifcond?
> + if ifcond is None or before == after:
> + return after
The before == after part suppresses empty conditionals. Suggest
return after # suppress empty #if ... #endif
The ifcond is None part is suppresses "non-conditionals". How can this
happen? If it can't, let's drop this part.
Another non-conditional is []. See below.
> +
> + assert after.startswith(before)
> + out = before
> + added = after[len(before):]
> + if added[0] == '\n':
> + out += '\n'
> + added = added[1:]
The conditional adjusts vertical space around #if. This might need
tweaking, but let's not worry about that now.
> + out += gen_if(ifcond)
> + out += added
> + out += gen_endif(ifcond)
There's no similar adjustment for #endif. Again, let's not worry about
that now.
> + return out
> +
> +
Since gen_if([]) and gen_endif([]) return '', wrap_ifcond([], before,
after) returns after. Okay.
> def gen_enum_lookup(name, values, prefix=None):
> ret = mcgen('''
>
> @@ -2064,6 +2099,7 @@ class QAPIGen(object):
> def __init__(self):
> self._preamble = ''
> self._body = ''
> + self._start_if = None
>
> def preamble_add(self, text):
> self._preamble += text
> @@ -2071,6 +2107,25 @@ class QAPIGen(object):
> def add(self, text):
> self._body += text
>
> + def start_if(self, ifcond):
> + assert self._start_if is None
> +
Let's drop this blank line.
> + self._start_if = (ifcond, self._body, self._preamble)
It's now obvious that you can't nest .start_if() ... .endif(). Good.
> +
> + def _wrap_ifcond(self):
> + pass
> +
> + def end_if(self):
> + assert self._start_if
> +
Let's drop this blank line.
> + self._wrap_ifcond()
> + self._start_if = None
> +
> + def get_content(self, fname=None):
> + assert self._start_if is None
> + return (self._top(fname) + self._preamble + self._body
> + + self._bottom(fname))
> +
> def _top(self, fname):
> return ''
>
Note for later: all this code has no effect unless ._wrap_ifcond() is
overridden.
> @@ -2091,8 +2146,7 @@ class QAPIGen(object):
> f = open(fd, 'r+', encoding='utf-8')
> else:
> f = os.fdopen(fd, 'r+')
> - text = (self._top(fname) + self._preamble + self._body
> - + self._bottom(fname))
> + text = self.get_content(fname)
> oldtext = f.read(len(text) + 1)
> if text != oldtext:
> f.seek(0)
> @@ -2101,10 +2155,49 @@ class QAPIGen(object):
> f.close()
>
>
> -class QAPIGenC(QAPIGen):
> address@hidden
> +def ifcontext(ifcond, *args):
> + """A 'with' statement context manager to wrap with start_if()/end_if()
>
> - def __init__(self, blurb, pydoc):
> + *args: variable length argument list of QAPIGen
Perhaps: any number of QAPIGen objects
> +
> + Example::
> +
> + with ifcontext(ifcond, self._genh, self._genc):
> + modify self._genh and self._genc ...
> +
> + Is equivalent to calling::
> +
> + self._genh.start_if(ifcond)
> + self._genc.start_if(ifcond)
> + modify self._genh and self._genc ...
> + self._genh.end_if()
> + self._genc.end_if()
> +
Can we drop this blank line?
> + """
> + for arg in args:
> + arg.start_if(ifcond)
> + yield
> + for arg in args:
> + arg.end_if()
> +
> +
> +class QAPIGenCCode(QAPIGen):
> +
> + def __init__(self):
> QAPIGen.__init__(self)
> +
> + def _wrap_ifcond(self):
> + self._body = wrap_ifcond(self._start_if[0],
> + self._start_if[1], self._body)
> + self._preamble = wrap_ifcond(self._start_if[0],
> + self._start_if[2], self._preamble)
Can you explain why you put the machinery for conditionals in QAPIGen
and not QAPIGenCCode?
> +
> +
> +class QAPIGenC(QAPIGenCCode):
> +
> + def __init__(self, blurb, pydoc):
> + QAPIGenCCode.__init__(self)
> self._blurb = blurb
> self._copyright = '\n * '.join(re.findall(r'^Copyright .*', pydoc,
> re.MULTILINE))
- Re: [Qemu-devel] [PATCH v6 07/15] qapi: add #if/#endif helpers,
Markus Armbruster <=