Marc-André Lureau <marcandre.lur...@redhat.com> 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 <marcandre.lur...@redhat.com> > --- > 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): > +@contextmanager > +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))