diff mbox series

[2/9] qapi: move gen_if/gen_endif to IfCond

Message ID 20201015165255.1573897-3-marcandre.lureau@redhat.com
State New
Headers show
Series qapi: untie 'if' conditions from C preprocessor | expand

Commit Message

Marc-André Lureau Oct. 15, 2020, 4:52 p.m. UTC 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Move the generating function to the IfCond class.
(avoid cluttering and potential missuse of global functions, allow
access to private members etc)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 scripts/qapi/common.py     | 22 ++++++++++------------
 scripts/qapi/gen.py        |  6 ++----
 scripts/qapi/introspect.py |  6 ++----
 scripts/qapi/types.py      | 22 ++++++++++------------
 scripts/qapi/visit.py      | 14 ++++++--------
 5 files changed, 30 insertions(+), 40 deletions(-)

Comments

John Snow Oct. 27, 2020, 9:32 p.m. UTC | #1 
On 10/15/20 12:52 PM, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Move the generating function to the IfCond class.
> (avoid cluttering and potential missuse of global functions, allow
> access to private members etc)
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   scripts/qapi/common.py     | 22 ++++++++++------------
>   scripts/qapi/gen.py        |  6 ++----
>   scripts/qapi/introspect.py |  6 ++----
>   scripts/qapi/types.py      | 22 ++++++++++------------
>   scripts/qapi/visit.py      | 14 ++++++--------
>   5 files changed, 30 insertions(+), 40 deletions(-)
> 

Seems straightforward enough, though I guess your 'Ifcond' object 
becomes something more of a code generator class than a pure abstract 
representation by absorbing C generation functions, yeah?
Marc-André Lureau Nov. 3, 2020, 7:54 p.m. UTC | #2 
Hi

On Wed, Oct 28, 2020 at 1:32 AM John Snow <jsnow@redhat.com> wrote:

> On 10/15/20 12:52 PM, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > Move the generating function to the IfCond class.
> > (avoid cluttering and potential missuse of global functions, allow
> > access to private members etc)
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   scripts/qapi/common.py     | 22 ++++++++++------------
> >   scripts/qapi/gen.py        |  6 ++----
> >   scripts/qapi/introspect.py |  6 ++----
> >   scripts/qapi/types.py      | 22 ++++++++++------------
> >   scripts/qapi/visit.py      | 14 ++++++--------
> >   5 files changed, 30 insertions(+), 40 deletions(-)
> >
>
> Seems straightforward enough, though I guess your 'Ifcond' object
> becomes something more of a code generator class than a pure abstract
> representation by absorbing C generation functions, yeah?
>

Right, in its current form (
https://github.com/elmarco/qemu/blob/qapi-rs/scripts/qapi/common.py#L283),
it has  "def gen_if(self) -> str" & "def gen_endif(self) -> str" (for C),
as well as "def gen_rs_cfg(self) -> str" (for Rust).

I didn't bother doing a visitor or other fancy design, as it's easy to
iterate later if needed, imho.
diff mbox series

Patch

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 59e6a400da..58ddec3bc5 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -209,20 +209,18 @@  class IfCond:
             return NotImplemented
         return self.ifcond == other.ifcond
 
-
-def gen_if(c: IfCond) -> str:
-    ret = ''
-    for ifc in c.ifcond:
-        ret += mcgen('''
+    def gen_if(self) -> str:
+        ret = ''
+        for ifc in self.ifcond:
+            ret += mcgen('''
 #if %(cond)s
 ''', cond=ifc)
-    return ret
-
+        return ret
 
-def gen_endif(c: IfCond) -> str:
-    ret = ''
-    for ifc in reversed(c.ifcond):
-        ret += mcgen('''
+    def gen_endif(self) -> str:
+        ret = ''
+        for ifc in reversed(self.ifcond):
+            ret += mcgen('''
 #endif /* %(cond)s */
 ''', cond=ifc)
-    return ret
+        return ret
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index b35ccb201f..5a8c66e628 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -26,8 +26,6 @@  from .common import (
     IfCond,
     c_fname,
     c_name,
-    gen_endif,
-    gen_if,
     guardend,
     guardstart,
     mcgen,
@@ -92,9 +90,9 @@  def _wrap_ifcond(ifcond: IfCond, before: str, after: str) -> str:
     if added[0] == '\n':
         out += '\n'
         added = added[1:]
-    out += gen_if(ifcond)
+    out += ifcond.gen_if()
     out += added
-    out += gen_endif(ifcond)
+    out += ifcond.gen_endif()
     return out
 
 
diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
index 103b8515eb..d97dea8e4e 100644
--- a/scripts/qapi/introspect.py
+++ b/scripts/qapi/introspect.py
@@ -23,8 +23,6 @@  from typing import (
 from .common import (
     IfCond,
     c_name,
-    gen_endif,
-    gen_if,
     mcgen,
 )
 from .gen import QAPISchemaMonolithicCVisitor
@@ -77,10 +75,10 @@  def _tree_to_qlit(obj: TreeNode, level: int = 0,
         if obj.comment:
             ret += indent(level) + '/* %s */\n' % obj.comment
         if obj.ifcond:
-            ret += gen_if(obj.ifcond)
+            ret += obj.ifcond.gen_if()
         ret += _tree_to_qlit(obj.data, level)
         if obj.ifcond:
-            ret += '\n' + gen_endif(obj.ifcond)
+            ret += '\n' + obj.ifcond.gen_endif()
         return ret
 
     ret = ''
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index c15613d13b..66ba2f62e4 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -19,8 +19,6 @@  from .common import (
     IfCond,
     c_enum_const,
     c_name,
-    gen_endif,
-    gen_if,
     mcgen,
 )
 from .gen import QAPISchemaModularCVisitor, ifcontext
@@ -51,13 +49,13 @@  const QEnumLookup %(c_name)s_lookup = {
 ''',
                 c_name=c_name(name))
     for memb in members:
-        ret += gen_if(memb.ifcond)
+        ret += memb.ifcond.gen_if()
         index = c_enum_const(name, memb.name, prefix)
         ret += mcgen('''
         [%(index)s] = "%(name)s",
 ''',
                      index=index, name=memb.name)
-        ret += gen_endif(memb.ifcond)
+        ret += memb.ifcond.gen_endif()
 
     ret += mcgen('''
     },
@@ -81,12 +79,12 @@  typedef enum %(c_name)s {
                 c_name=c_name(name))
 
     for memb in enum_members:
-        ret += gen_if(memb.ifcond)
+        ret += memb.ifcond.gen_if()
         ret += mcgen('''
     %(c_enum)s,
 ''',
                      c_enum=c_enum_const(name, memb.name, prefix))
-        ret += gen_endif(memb.ifcond)
+        ret += memb.ifcond.gen_endif()
 
     ret += mcgen('''
 } %(c_name)s;
@@ -126,7 +124,7 @@  struct %(c_name)s {
 def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
     ret = ''
     for memb in members:
-        ret += gen_if(memb.ifcond)
+        ret += memb.ifcond.gen_if()
         if memb.optional:
             ret += mcgen('''
     bool has_%(c_name)s;
@@ -136,7 +134,7 @@  def gen_struct_members(members: List[QAPISchemaObjectTypeMember]) -> str:
     %(c_type)s %(c_name)s;
 ''',
                      c_type=memb.type.c_type(), c_name=c_name(memb.name))
-        ret += gen_endif(memb.ifcond)
+        ret += memb.ifcond.gen_endif()
     return ret
 
 
@@ -159,7 +157,7 @@  def gen_object(name: str, ifcond: IfCond,
     ret += mcgen('''
 
 ''')
-    ret += gen_if(ifcond)
+    ret += ifcond.gen_if()
     ret += mcgen('''
 struct %(c_name)s {
 ''',
@@ -193,7 +191,7 @@  struct %(c_name)s {
     ret += mcgen('''
 };
 ''')
-    ret += gen_endif(ifcond)
+    ret += ifcond.gen_endif()
 
     return ret
 
@@ -220,13 +218,13 @@  def gen_variants(variants: QAPISchemaVariants) -> str:
     for var in variants.variants:
         if var.type.name == 'q_empty':
             continue
-        ret += gen_if(var.ifcond)
+        ret += var.ifcond.gen_if()
         ret += mcgen('''
         %(c_type)s %(c_name)s;
 ''',
                      c_type=var.type.c_unboxed_type(),
                      c_name=c_name(var.name))
-        ret += gen_endif(var.ifcond)
+        ret += var.ifcond.gen_endif()
 
     ret += mcgen('''
     } u;
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index d10840bf7b..b199e75946 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -19,8 +19,6 @@  from .common import (
     IfCond,
     c_enum_const,
     c_name,
-    gen_endif,
-    gen_if,
     indent,
     mcgen,
 )
@@ -78,7 +76,7 @@  bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
                      c_type=base.c_name())
 
     for memb in members:
-        ret += gen_if(memb.ifcond)
+        ret += memb.ifcond.gen_if()
         if memb.optional:
             ret += mcgen('''
     if (visit_optional(v, "%(name)s", &obj->has_%(c_name)s)) {
@@ -97,7 +95,7 @@  bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
             ret += mcgen('''
     }
 ''')
-        ret += gen_endif(memb.ifcond)
+        ret += memb.ifcond.gen_endif()
 
     if variants:
         tag_member = variants.tag_member
@@ -111,7 +109,7 @@  bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
         for var in variants.variants:
             case_str = c_enum_const(tag_member.type.name, var.name,
                                     tag_member.type.prefix)
-            ret += gen_if(var.ifcond)
+            ret += var.ifcond.gen_if()
             if var.type.name == 'q_empty':
                 # valid variant and nothing to do
                 ret += mcgen('''
@@ -127,7 +125,7 @@  bool visit_type_%(c_name)s_members(Visitor *v, %(c_name)s *obj, Error **errp)
                              case=case_str,
                              c_type=var.type.c_name(), c_name=c_name(var.name))
 
-            ret += gen_endif(var.ifcond)
+            ret += var.ifcond.gen_endif()
         ret += mcgen('''
     default:
         abort();
@@ -213,7 +211,7 @@  bool visit_type_%(c_name)s(Visitor *v, const char *name,
                 c_name=c_name(name))
 
     for var in variants.variants:
-        ret += gen_if(var.ifcond)
+        ret += var.ifcond.gen_if()
         ret += mcgen('''
     case %(case)s:
 ''',
@@ -239,7 +237,7 @@  bool visit_type_%(c_name)s(Visitor *v, const char *name,
         ret += mcgen('''
         break;
 ''')
-        ret += gen_endif(var.ifcond)
+        ret += var.ifcond.gen_endif()
 
     ret += mcgen('''
     case QTYPE_NONE: