| Message ID | 20180321115211.17937-9-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: add #if pre-processor conditions to generated code | expand |
Marc-André Lureau <marcandre.lureau@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. > > 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.lureau@redhat.com> > --- > scripts/qapi/common.py | 82 +++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 78 insertions(+), 4 deletions(-) > > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > index 47efe79758..60c1d0a783 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 > @@ -1964,6 +1965,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. We'll see later how we end up with them. The ifcond is None part is suppresses "non-conditionals". I wonder how that can happen. Another non-conditional is []. Can that happen? Questions like these indicate the function needs a contract. > + > + 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 > + > + > def gen_enum_lookup(name, values, prefix=None): > ret = mcgen(''' > > @@ -2054,6 +2089,7 @@ class QAPIGen(object): > def __init__(self): > self._preamble = '' > self._body = '' > + self._ifcond = None > > def preamble_add(self, text): > self._preamble += text > @@ -2061,6 +2097,23 @@ class QAPIGen(object): > def add(self, text): > self._body += text > > + def start_if(self, ifcond): What are @ifcond's legal values? In particular, is it okay to pass None? If not, then we have an easy way to check whether we're between .start_if() and .end_if(): ._ifcond is not None. > + self._ifcond = ifcond What if self._ifcond is not None? Can happen only when you call .start_if within .start_if() ... .endif(). If that's not supposed to work, let's assert self._ifcond is None here, and add a function contract spelling out the restriction. > + self._start_if_body = self._body > + self._start_if_preamble = self._preamble > + > + def _wrap_ifcond(self): > + pass > + > + def end_if(self): If .start_if() doesn't take None, then we can catch misuse of .end_if() by asserting ._ifcond is not None here. > + self._wrap_ifcond() > + self._ifcond = None I'd zap ._start_if_body and ._start_if_preamble here. Note for later: use of .start_if() ... .end_if() has no effect unless ._wrap_ifcond() is overridden. > + > + def get_content(self, fname=None): > + assert self._ifcond is None > + return (self._top(fname) + self._preamble + self._body > + + self._bottom(fname)) > + > def _top(self, fname): > return '' > > @@ -2078,8 +2131,7 @@ class QAPIGen(object): > raise > fd = os.open(pathname, os.O_RDWR | os.O_CREAT, 0o666) > 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) > @@ -2088,10 +2140,32 @@ class QAPIGen(object): > f.close() > > > -class QAPIGenC(QAPIGen): > +@contextmanager > +def ifcontext(ifcond, *args): > + saved = [] > + for arg in args: > + arg.start_if(ifcond) > + yield > + for arg in args: > + arg.end_if() @saved is write-only. How to use the function isn't immediately obvious[*]; I had to glance at later patches to be sure. Please add a function comment. > > - def __init__(self, blurb, pydoc): > + > +class QAPIGenCSnippet(QAPIGen): > + > + def __init__(self): > QAPIGen.__init__(self) > + > + def _wrap_ifcond(self): > + self._body = wrap_ifcond(self._ifcond, > + self._start_if_body, self._body) > + self._preamble = wrap_ifcond(self._ifcond, > + self._start_if_preamble, self._preamble) > + > + > +class QAPIGenC(QAPIGenCSnippet): > + > + def __init__(self, blurb, pydoc): > + QAPIGenCSnippet.__init__(self) > self._blurb = blurb > self._copyright = '\n * '.join(re.findall(r'^Copyright .*', pydoc, > re.MULTILINE)) Why do you need the intermediary class QAPIGenCSnippet? As far as I can see, only one subclass overrides ._wrap_ifcond(). Why is the general machinery in the superclass, where it does nothing, and not in the subclass that makes it do something? [*] Compared to the decorator you had in v4, it's simple as pie. There *had* to be a better way, and it looks like you found one. Appreciated!
Marc-André Lureau <marcandre.lureau@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. > > 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.lureau@redhat.com> > --- > scripts/qapi/common.py | 82 +++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 78 insertions(+), 4 deletions(-) > > diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py > index 47efe79758..60c1d0a783 100644 > --- a/scripts/qapi/common.py > +++ b/scripts/qapi/common.py [...] > @@ -2061,6 +2097,23 @@ class QAPIGen(object): > def add(self, text): > self._body += text > > + def start_if(self, ifcond): > + self._ifcond = ifcond > + self._start_if_body = self._body > + self._start_if_preamble = self._preamble pylint gripes: +W:2102, 8: Attribute '_start_if_body' defined outside __init__ (attribute-defined-outside-init) +W:2103, 8: Attribute '_start_if_preamble' defined outside __init__ (attribute-defined-outside-init) We generally define in .__init__(), except when we want to catch premature use, such as in PATCH 05. Let's define these two in .__init__(). [...]
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py index 47efe79758..60c1d0a783 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 @@ -1964,6 +1965,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): + if ifcond is None or before == after: + return after + + assert after.startswith(before) + out = before + added = after[len(before):] + if added[0] == '\n': + out += '\n' + added = added[1:] + out += gen_if(ifcond) + out += added + out += gen_endif(ifcond) + return out + + def gen_enum_lookup(name, values, prefix=None): ret = mcgen(''' @@ -2054,6 +2089,7 @@ class QAPIGen(object): def __init__(self): self._preamble = '' self._body = '' + self._ifcond = None def preamble_add(self, text): self._preamble += text @@ -2061,6 +2097,23 @@ class QAPIGen(object): def add(self, text): self._body += text + def start_if(self, ifcond): + self._ifcond = ifcond + self._start_if_body = self._body + self._start_if_preamble = self._preamble + + def _wrap_ifcond(self): + pass + + def end_if(self): + self._wrap_ifcond() + self._ifcond = None + + def get_content(self, fname=None): + assert self._ifcond is None + return (self._top(fname) + self._preamble + self._body + + self._bottom(fname)) + def _top(self, fname): return '' @@ -2078,8 +2131,7 @@ class QAPIGen(object): raise fd = os.open(pathname, os.O_RDWR | os.O_CREAT, 0o666) 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) @@ -2088,10 +2140,32 @@ class QAPIGen(object): f.close() -class QAPIGenC(QAPIGen): +@contextmanager +def ifcontext(ifcond, *args): + saved = [] + for arg in args: + arg.start_if(ifcond) + yield + for arg in args: + arg.end_if() - def __init__(self, blurb, pydoc): + +class QAPIGenCSnippet(QAPIGen): + + def __init__(self): QAPIGen.__init__(self) + + def _wrap_ifcond(self): + self._body = wrap_ifcond(self._ifcond, + self._start_if_body, self._body) + self._preamble = wrap_ifcond(self._ifcond, + self._start_if_preamble, self._preamble) + + +class QAPIGenC(QAPIGenCSnippet): + + def __init__(self, blurb, pydoc): + QAPIGenCSnippet.__init__(self) self._blurb = blurb self._copyright = '\n * '.join(re.findall(r'^Copyright .*', pydoc, re.MULTILINE))
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. 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.lureau@redhat.com> --- scripts/qapi/common.py | 82 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 78 insertions(+), 4 deletions(-)