All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] Drop support for QAPIGen without a file name
@ 2020-12-18 20:53 Markus Armbruster
  2020-12-18 20:53 ` [PATCH 01/11] qapi/commands: assert arg_type is not None Markus Armbruster
                   ` (10 more replies)
  0 siblings, 11 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-12-18 20:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, jsnow, ehabkost, crosa

John Snow posted

    [PATCH 09/12] qapi/gen: move write method to QAPIGenC, make fname a str

    QAPIGenC and QAPIGenH in particular depend on fname being defined, but
    we have a usage of QAPIGenCCode that isn't intended to be associated
    with a particular file.

    No problem, move the write method down to the class that actually needs
    it, and keep QAPIGenCCode more abstract.

    Signed-off-by: John Snow <jsnow@redhat.com>
    ---
     scripts/qapi/commands.py |  2 +-
     scripts/qapi/gen.py      | 54 ++++++++++++++++++++--------------------
     2 files changed, 28 insertions(+), 28 deletions(-)

There is just one user of QAPIGen without a file name, and it's
awkward.  Let's get rid of it.

Since my work to get rid of it depends on parts of John's series, and
I'm pressed for time, I include the parts I need in my series.  John,
feel free to pick this into your complete series.

John Snow (6):
  qapi/commands: assert arg_type is not None
  qapi/events: fix visit_event typing
  qapi/main: handle theoretical None-return from re.match()
  qapi/gen: assert that _start_if is not None in _wrap_ifcond
  qapi/gen: use './builtin' for the built-in module name
  qapi/gen: write _genc/_genh access shims

Markus Armbruster (5):
  qapi/gen: Replace ._begin_system_module()
  qapi/gen: Expose a single module name space
  qapi/gen: Support for switching to another module temporarily
  qapi/commands: Simplify command registry generation
  qapi/gen: Drop support for QAPIGen without a file name

 scripts/qapi/commands.py | 60 ++++++++++++++++-----------------
 scripts/qapi/events.py   | 14 ++++----
 scripts/qapi/gen.py      | 72 +++++++++++++++++++++++-----------------
 scripts/qapi/main.py     |  2 ++
 scripts/qapi/types.py    |  2 +-
 scripts/qapi/visit.py    |  2 +-
 6 files changed, 82 insertions(+), 70 deletions(-)

-- 
2.26.2



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/11] qapi/commands: assert arg_type is not None
  2020-12-18 20:53 [PATCH 00/11] Drop support for QAPIGen without a file name Markus Armbruster
@ 2020-12-18 20:53 ` Markus Armbruster
  2020-12-18 20:53 ` [PATCH 02/11] qapi/events: fix visit_event typing Markus Armbruster
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-12-18 20:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, jsnow, ehabkost, crosa

From: John Snow <jsnow@redhat.com>

when boxed is true, expr.py asserts that we must have
arguments. Ultimately, this should mean that if boxed is True, that
arg_type should be defined. Mypy cannot infer this, and does not support
'stateful' type inference, e.g.:

```
if x:
    assert y is not None

...

if x:
    y.etc()
```

does not work, because mypy does not statefully remember the conditional
assertion in the second block. Help mypy out by creating a new local
that it can track more easily.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20201217015927.197287-2-jsnow@redhat.com>
---
 scripts/qapi/commands.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 50978090b4..71744f48a3 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -126,6 +126,9 @@ def gen_marshal(name: str,
                 boxed: bool,
                 ret_type: Optional[QAPISchemaType]) -> str:
     have_args = boxed or (arg_type and not arg_type.is_empty())
+    if have_args:
+        assert arg_type is not None
+        arg_type_c_name = arg_type.c_name()
 
     ret = mcgen('''
 
@@ -147,7 +150,7 @@ def gen_marshal(name: str,
         ret += mcgen('''
     %(c_name)s arg = {0};
 ''',
-                     c_name=arg_type.c_name())
+                     c_name=arg_type_c_name)
 
     ret += mcgen('''
 
@@ -163,7 +166,7 @@ def gen_marshal(name: str,
         ok = visit_check_struct(v, errp);
     }
 ''',
-                     c_arg_type=arg_type.c_name())
+                     c_arg_type=arg_type_c_name)
     else:
         ret += mcgen('''
     ok = visit_check_struct(v, errp);
@@ -193,7 +196,7 @@ out:
         ret += mcgen('''
     visit_type_%(c_arg_type)s_members(v, &arg, NULL);
 ''',
-                     c_arg_type=arg_type.c_name())
+                     c_arg_type=arg_type_c_name)
 
     ret += mcgen('''
     visit_end_struct(v, NULL);
-- 
2.26.2



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 02/11] qapi/events: fix visit_event typing
  2020-12-18 20:53 [PATCH 00/11] Drop support for QAPIGen without a file name Markus Armbruster
  2020-12-18 20:53 ` [PATCH 01/11] qapi/commands: assert arg_type is not None Markus Armbruster
@ 2020-12-18 20:53 ` Markus Armbruster
  2020-12-18 20:53 ` [PATCH 03/11] qapi/main: handle theoretical None-return from re.match() Markus Armbruster
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-12-18 20:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, jsnow, ehabkost, crosa

From: John Snow <jsnow@redhat.com>

Actually, the arg_type can indeed be Optional.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20201217015927.197287-3-jsnow@redhat.com>
---
 scripts/qapi/events.py | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 599f3d1f56..9851653b9d 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -12,7 +12,7 @@ This work is licensed under the terms of the GNU GPL, version 2.
 See the COPYING file in the top-level directory.
 """
 
-from typing import List
+from typing import List, Optional
 
 from .common import c_enum_const, c_name, mcgen
 from .gen import QAPISchemaModularCVisitor, build_params, ifcontext
@@ -27,7 +27,7 @@ from .types import gen_enum, gen_enum_lookup
 
 
 def build_event_send_proto(name: str,
-                           arg_type: QAPISchemaObjectType,
+                           arg_type: Optional[QAPISchemaObjectType],
                            boxed: bool) -> str:
     return 'void qapi_event_send_%(c_name)s(%(param)s)' % {
         'c_name': c_name(name.lower()),
@@ -35,7 +35,7 @@ def build_event_send_proto(name: str,
 
 
 def gen_event_send_decl(name: str,
-                        arg_type: QAPISchemaObjectType,
+                        arg_type: Optional[QAPISchemaObjectType],
                         boxed: bool) -> str:
     return mcgen('''
 
@@ -78,7 +78,7 @@ def gen_param_var(typ: QAPISchemaObjectType) -> str:
 
 
 def gen_event_send(name: str,
-                   arg_type: QAPISchemaObjectType,
+                   arg_type: Optional[QAPISchemaObjectType],
                    boxed: bool,
                    event_enum_name: str,
                    event_emit: str) -> str:
@@ -99,6 +99,7 @@ def gen_event_send(name: str,
                 proto=build_event_send_proto(name, arg_type, boxed))
 
     if have_args:
+        assert arg_type is not None
         ret += mcgen('''
     QObject *obj;
     Visitor *v;
@@ -114,6 +115,7 @@ def gen_event_send(name: str,
                  name=name)
 
     if have_args:
+        assert arg_type is not None
         ret += mcgen('''
     v = qobject_output_visitor_new(&obj);
 ''')
@@ -214,7 +216,7 @@ void %(event_emit)s(%(event_enum)s event, QDict *qdict);
                     info: QAPISourceInfo,
                     ifcond: List[str],
                     features: List[QAPISchemaFeature],
-                    arg_type: QAPISchemaObjectType,
+                    arg_type: Optional[QAPISchemaObjectType],
                     boxed: bool) -> None:
         with ifcontext(ifcond, self._genh, self._genc):
             self._genh.add(gen_event_send_decl(name, arg_type, boxed))
-- 
2.26.2



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 03/11] qapi/main: handle theoretical None-return from re.match()
  2020-12-18 20:53 [PATCH 00/11] Drop support for QAPIGen without a file name Markus Armbruster
  2020-12-18 20:53 ` [PATCH 01/11] qapi/commands: assert arg_type is not None Markus Armbruster
  2020-12-18 20:53 ` [PATCH 02/11] qapi/events: fix visit_event typing Markus Armbruster
@ 2020-12-18 20:53 ` Markus Armbruster
  2020-12-18 20:54 ` [PATCH 04/11] qapi/gen: assert that _start_if is not None in _wrap_ifcond Markus Armbruster
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-12-18 20:53 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, jsnow, ehabkost, crosa

From: John Snow <jsnow@redhat.com>

Mypy cannot understand that this match can never be None, so help it
along.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20201217015927.197287-4-jsnow@redhat.com>
---
 scripts/qapi/main.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
index 42517210b8..703e7ed1ed 100644
--- a/scripts/qapi/main.py
+++ b/scripts/qapi/main.py
@@ -23,6 +23,8 @@ from .visit import gen_visit
 
 def invalid_prefix_char(prefix: str) -> Optional[str]:
     match = re.match(r'([A-Za-z_.-][A-Za-z0-9_.-]*)?', prefix)
+    # match cannot be None, but mypy cannot infer that.
+    assert match is not None
     if match.end() != len(prefix):
         return prefix[match.end()]
     return None
-- 
2.26.2



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 04/11] qapi/gen: assert that _start_if is not None in _wrap_ifcond
  2020-12-18 20:53 [PATCH 00/11] Drop support for QAPIGen without a file name Markus Armbruster
                   ` (2 preceding siblings ...)
  2020-12-18 20:53 ` [PATCH 03/11] qapi/main: handle theoretical None-return from re.match() Markus Armbruster
@ 2020-12-18 20:54 ` Markus Armbruster
  2020-12-18 20:54 ` [PATCH 05/11] qapi/gen: use './builtin' for the built-in module name Markus Armbruster
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-12-18 20:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, jsnow, ehabkost, crosa

From: John Snow <jsnow@redhat.com>

We already assert this in end_if, but that's opaque to mypy. Do it in
_wrap_ifcond instead. Same effect at runtime, but mypy can now infer
the type in _wrap_ifcond's body.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20201217015927.197287-5-jsnow@redhat.com>
---
 scripts/qapi/gen.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index b40f18eee3..a6dc991b1d 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -130,11 +130,11 @@ class QAPIGenCCode(QAPIGen):
         self._start_if = (ifcond, self._body, self._preamble)
 
     def end_if(self) -> None:
-        assert self._start_if
         self._wrap_ifcond()
         self._start_if = None
 
     def _wrap_ifcond(self) -> None:
+        assert self._start_if
         self._body = _wrap_ifcond(self._start_if[0],
                                   self._start_if[1], self._body)
         self._preamble = _wrap_ifcond(self._start_if[0],
-- 
2.26.2



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 05/11] qapi/gen: use './builtin' for the built-in module name
  2020-12-18 20:53 [PATCH 00/11] Drop support for QAPIGen without a file name Markus Armbruster
                   ` (3 preceding siblings ...)
  2020-12-18 20:54 ` [PATCH 04/11] qapi/gen: assert that _start_if is not None in _wrap_ifcond Markus Armbruster
@ 2020-12-18 20:54 ` Markus Armbruster
  2020-12-18 20:54 ` [PATCH 06/11] qapi/gen: write _genc/_genh access shims Markus Armbruster
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-12-18 20:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, jsnow, ehabkost, crosa

From: John Snow <jsnow@redhat.com>

Use this in preference to 'None', which helps remove some edge cases in
the typing.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20201217015927.197287-6-jsnow@redhat.com>
---
 scripts/qapi/gen.py | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index a6dc991b1d..476d0adac4 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -245,23 +245,23 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
         self._pydoc = pydoc
         self._genc: Optional[QAPIGenC] = None
         self._genh: Optional[QAPIGenH] = None
-        self._module: Dict[Optional[str], Tuple[QAPIGenC, QAPIGenH]] = {}
+        self._module: Dict[str, Tuple[QAPIGenC, QAPIGenH]] = {}
         self._main_module: Optional[str] = None
 
     @staticmethod
-    def _is_user_module(name: Optional[str]) -> bool:
-        return bool(name and not name.startswith('./'))
+    def _is_user_module(name: str) -> bool:
+        return not name.startswith('./')
 
     @staticmethod
-    def _is_builtin_module(name: Optional[str]) -> bool:
-        return not name
+    def _is_builtin_module(name: str) -> bool:
+        return name == './builtin'
 
-    def _module_dirname(self, name: Optional[str]) -> str:
+    def _module_dirname(self, name: str) -> str:
         if self._is_user_module(name):
             return os.path.dirname(name)
         return ''
 
-    def _module_basename(self, what: str, name: Optional[str]) -> str:
+    def _module_basename(self, what: str, name: str) -> str:
         ret = '' if self._is_builtin_module(name) else self._prefix
         if self._is_user_module(name):
             basename = os.path.basename(name)
@@ -269,15 +269,15 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
             if name != self._main_module:
                 ret += '-' + os.path.splitext(basename)[0]
         else:
-            name = name[2:] if name else 'builtin'
-            ret += re.sub(r'-', '-' + name + '-', what)
+            assert name.startswith('./')
+            ret += re.sub(r'-', '-' + name[2:] + '-', what)
         return ret
 
-    def _module_filename(self, what: str, name: Optional[str]) -> str:
+    def _module_filename(self, what: str, name: str) -> str:
         return os.path.join(self._module_dirname(name),
                             self._module_basename(what, name))
 
-    def _add_module(self, name: Optional[str], blurb: str) -> None:
+    def _add_module(self, name: str, blurb: str) -> None:
         basename = self._module_filename(self._what, name)
         genc = QAPIGenC(basename + '.c', blurb, self._pydoc)
         genh = QAPIGenH(basename + '.h', blurb, self._pydoc)
@@ -290,8 +290,8 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
             self._main_module = name
         self._add_module(name, blurb)
 
-    def _add_system_module(self, name: Optional[str], blurb: str) -> None:
-        self._add_module(name and './' + name, blurb)
+    def _add_system_module(self, name: str, blurb: str) -> None:
+        self._add_module('./' + name, blurb)
 
     def write(self, output_dir: str, opt_builtins: bool = False) -> None:
         for name in self._module:
@@ -310,7 +310,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
     def visit_module(self, name: Optional[str]) -> None:
         if name is None:
             if self._builtin_blurb:
-                self._add_system_module(None, self._builtin_blurb)
+                self._add_system_module('builtin', self._builtin_blurb)
                 self._begin_system_module(name)
             else:
                 # The built-in module has not been created.  No code may
-- 
2.26.2



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 06/11] qapi/gen: write _genc/_genh access shims
  2020-12-18 20:53 [PATCH 00/11] Drop support for QAPIGen without a file name Markus Armbruster
                   ` (4 preceding siblings ...)
  2020-12-18 20:54 ` [PATCH 05/11] qapi/gen: use './builtin' for the built-in module name Markus Armbruster
@ 2020-12-18 20:54 ` Markus Armbruster
  2020-12-18 20:54 ` [PATCH 07/11] qapi/gen: Replace ._begin_system_module() Markus Armbruster
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-12-18 20:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, jsnow, ehabkost, crosa

From: John Snow <jsnow@redhat.com>

Many places assume they can access these fields without checking them
first to ensure they are defined. Eliminating the _genc and _genh fields
and replacing them with functional properties that check for correct
state can ease the typing overhead by eliminating the Optional[T] return
type.

Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20201217015927.197287-9-jsnow@redhat.com>
---
 scripts/qapi/gen.py | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 476d0adac4..12ea98fb61 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -243,11 +243,20 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
         self._user_blurb = user_blurb
         self._builtin_blurb = builtin_blurb
         self._pydoc = pydoc
-        self._genc: Optional[QAPIGenC] = None
-        self._genh: Optional[QAPIGenH] = None
+        self._current_module: Optional[str] = None
         self._module: Dict[str, Tuple[QAPIGenC, QAPIGenH]] = {}
         self._main_module: Optional[str] = None
 
+    @property
+    def _genc(self) -> QAPIGenC:
+        assert self._current_module is not None
+        return self._module[self._current_module][0]
+
+    @property
+    def _genh(self) -> QAPIGenH:
+        assert self._current_module is not None
+        return self._module[self._current_module][1]
+
     @staticmethod
     def _is_user_module(name: str) -> bool:
         return not name.startswith('./')
@@ -282,7 +291,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
         genc = QAPIGenC(basename + '.c', blurb, self._pydoc)
         genh = QAPIGenH(basename + '.h', blurb, self._pydoc)
         self._module[name] = (genc, genh)
-        self._genc, self._genh = self._module[name]
+        self._current_module = name
 
     def _add_user_module(self, name: str, blurb: str) -> None:
         assert self._is_user_module(name)
@@ -315,8 +324,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
             else:
                 # The built-in module has not been created.  No code may
                 # be generated.
-                self._genc = None
-                self._genh = None
+                self._current_module = None
         else:
             self._add_user_module(name, self._user_blurb)
             self._begin_user_module(name)
-- 
2.26.2



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 07/11] qapi/gen: Replace ._begin_system_module()
  2020-12-18 20:53 [PATCH 00/11] Drop support for QAPIGen without a file name Markus Armbruster
                   ` (5 preceding siblings ...)
  2020-12-18 20:54 ` [PATCH 06/11] qapi/gen: write _genc/_genh access shims Markus Armbruster
@ 2020-12-18 20:54 ` Markus Armbruster
  2020-12-18 20:54 ` [PATCH 08/11] qapi/gen: Expose a single module name space Markus Armbruster
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-12-18 20:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, jsnow, ehabkost, crosa

QAPISchemaModularCVisitor._begin_system_module() is actually just for
the builtin module.  Rename it to ._begin_builtin_module() and drop
its useless @name parameter.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/gen.py   | 4 ++--
 scripts/qapi/types.py | 2 +-
 scripts/qapi/visit.py | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 12ea98fb61..67635b6a26 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -310,7 +310,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
             genc.write(output_dir)
             genh.write(output_dir)
 
-    def _begin_system_module(self, name: None) -> None:
+    def _begin_builtin_module(self) -> None:
         pass
 
     def _begin_user_module(self, name: str) -> None:
@@ -320,7 +320,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
         if name is None:
             if self._builtin_blurb:
                 self._add_system_module('builtin', self._builtin_blurb)
-                self._begin_system_module(name)
+                self._begin_builtin_module()
             else:
                 # The built-in module has not been created.  No code may
                 # be generated.
diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py
index 2b4916cdaa..dbf58c0b91 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -271,7 +271,7 @@ class QAPISchemaGenTypeVisitor(QAPISchemaModularCVisitor):
             prefix, 'qapi-types', ' * Schema-defined QAPI types',
             ' * Built-in QAPI types', __doc__)
 
-    def _begin_system_module(self, name: None) -> None:
+    def _begin_builtin_module(self) -> None:
         self._genc.preamble_add(mcgen('''
 #include "qemu/osdep.h"
 #include "qapi/dealloc-visitor.h"
diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py
index 339f152152..568ba35592 100644
--- a/scripts/qapi/visit.py
+++ b/scripts/qapi/visit.py
@@ -305,7 +305,7 @@ class QAPISchemaGenVisitVisitor(QAPISchemaModularCVisitor):
             prefix, 'qapi-visit', ' * Schema-defined QAPI visitors',
             ' * Built-in QAPI visitors', __doc__)
 
-    def _begin_system_module(self, name: None) -> None:
+    def _begin_builtin_module(self) -> None:
         self._genc.preamble_add(mcgen('''
 #include "qemu/osdep.h"
 #include "qapi/error.h"
-- 
2.26.2



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 08/11] qapi/gen: Expose a single module name space
  2020-12-18 20:53 [PATCH 00/11] Drop support for QAPIGen without a file name Markus Armbruster
                   ` (6 preceding siblings ...)
  2020-12-18 20:54 ` [PATCH 07/11] qapi/gen: Replace ._begin_system_module() Markus Armbruster
@ 2020-12-18 20:54 ` Markus Armbruster
  2020-12-18 20:54 ` [PATCH 09/11] qapi/gen: Support for switching to another module temporarily Markus Armbruster
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-12-18 20:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, jsnow, ehabkost, crosa

QAPISchemaModularCVisitor attempts to encapsulate the way it splits
the module name space between user modules (name can't start with
'./') and system modules (name is None or starts with './') by
providing separate ._add_user_module() and ._add_system_module(),
where the latter prepends './' to names other than None.

Not worthwhile.  Dumb down to a single ._add_module(), and have the
callers take care of giving system modules names starting with './'.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/commands.py |  2 +-
 scripts/qapi/events.py   |  2 +-
 scripts/qapi/gen.py      | 16 +++++-----------
 3 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 71744f48a3..4911166339 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -286,7 +286,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
                              types=types))
 
     def visit_end(self) -> None:
-        self._add_system_module('init', ' * QAPI Commands initialization')
+        self._add_module('./init', ' * QAPI Commands initialization')
         self._genh.add(mcgen('''
 #include "qapi/qmp/dispatch.h"
 
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 9851653b9d..079c666ec6 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -191,7 +191,7 @@ class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor):
                              types=types))
 
     def visit_end(self) -> None:
-        self._add_system_module('emit', ' * QAPI Events emission')
+        self._add_module('./emit', ' * QAPI Events emission')
         self._genc.preamble_add(mcgen('''
 #include "qemu/osdep.h"
 #include "%(prefix)sqapi-emit-events.h"
diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index 67635b6a26..d9f8bac9aa 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -287,21 +287,15 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
                             self._module_basename(what, name))
 
     def _add_module(self, name: str, blurb: str) -> None:
+        if self._is_user_module(name):
+            if self._main_module is None:
+                self._main_module = name
         basename = self._module_filename(self._what, name)
         genc = QAPIGenC(basename + '.c', blurb, self._pydoc)
         genh = QAPIGenH(basename + '.h', blurb, self._pydoc)
         self._module[name] = (genc, genh)
         self._current_module = name
 
-    def _add_user_module(self, name: str, blurb: str) -> None:
-        assert self._is_user_module(name)
-        if self._main_module is None:
-            self._main_module = name
-        self._add_module(name, blurb)
-
-    def _add_system_module(self, name: str, blurb: str) -> None:
-        self._add_module('./' + name, blurb)
-
     def write(self, output_dir: str, opt_builtins: bool = False) -> None:
         for name in self._module:
             if self._is_builtin_module(name) and not opt_builtins:
@@ -319,14 +313,14 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
     def visit_module(self, name: Optional[str]) -> None:
         if name is None:
             if self._builtin_blurb:
-                self._add_system_module('builtin', self._builtin_blurb)
+                self._add_module('./builtin', self._builtin_blurb)
                 self._begin_builtin_module()
             else:
                 # The built-in module has not been created.  No code may
                 # be generated.
                 self._current_module = None
         else:
-            self._add_user_module(name, self._user_blurb)
+            self._add_module(name, self._user_blurb)
             self._begin_user_module(name)
 
     def visit_include(self, name: str, info: QAPISourceInfo) -> None:
-- 
2.26.2



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 09/11] qapi/gen: Support for switching to another module temporarily
  2020-12-18 20:53 [PATCH 00/11] Drop support for QAPIGen without a file name Markus Armbruster
                   ` (7 preceding siblings ...)
  2020-12-18 20:54 ` [PATCH 08/11] qapi/gen: Expose a single module name space Markus Armbruster
@ 2020-12-18 20:54 ` Markus Armbruster
  2021-01-15  2:07   ` John Snow
  2020-12-18 20:54 ` [PATCH 10/11] qapi/commands: Simplify command registry generation Markus Armbruster
  2020-12-18 20:54 ` [PATCH 11/11] qapi/gen: Drop support for QAPIGen without a file name Markus Armbruster
  10 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2020-12-18 20:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, jsnow, ehabkost, crosa

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/gen.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index d9f8bac9aa..cb00229f5d 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -15,6 +15,7 @@ from contextlib import contextmanager
 import os
 import re
 from typing import (
+    ContextManager,
     Dict,
     Iterator,
     List,
@@ -296,6 +297,13 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor):
         self._module[name] = (genc, genh)
         self._current_module = name
 
+    @contextmanager
+    def _temp_module(self, name: str) -> ContextManager[None]:
+        old_module = self._current_module
+        self._current_module = name
+        yield
+        self._current_module = old_module
+
     def write(self, output_dir: str, opt_builtins: bool = False) -> None:
         for name in self._module:
             if self._is_builtin_module(name) and not opt_builtins:
-- 
2.26.2



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 10/11] qapi/commands: Simplify command registry generation
  2020-12-18 20:53 [PATCH 00/11] Drop support for QAPIGen without a file name Markus Armbruster
                   ` (8 preceding siblings ...)
  2020-12-18 20:54 ` [PATCH 09/11] qapi/gen: Support for switching to another module temporarily Markus Armbruster
@ 2020-12-18 20:54 ` Markus Armbruster
  2021-01-15  2:15   ` John Snow
  2020-12-18 20:54 ` [PATCH 11/11] qapi/gen: Drop support for QAPIGen without a file name Markus Armbruster
  10 siblings, 1 reply; 14+ messages in thread
From: Markus Armbruster @ 2020-12-18 20:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, jsnow, ehabkost, crosa

QAPISchemaGenCommandVisitor.visit_command() needs to generate the
marshalling function into the current module, and also generate its
registration into the ./init system module.  The latter is done
somewhat awkwardly: .__init__() creates a QAPIGenCCode that will not
be written out, each .visit_command() adds its registration to it, and
.visit_end() copies its contents into the ./init module it creates.

Instead provide the means to temporarily switch to another module.
Create the ./init module in .visit_begin(), and generate its initial
part.  Add registrations to it in .visit_command().  Finish it in
.visit_end().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/commands.py | 49 ++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 27 deletions(-)

diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
index 4911166339..396485cc1a 100644
--- a/scripts/qapi/commands.py
+++ b/scripts/qapi/commands.py
@@ -23,7 +23,6 @@ from typing import (
 from .common import c_name, mcgen
 from .gen import (
     QAPIGenC,
-    QAPIGenCCode,
     QAPISchemaModularCVisitor,
     build_params,
     ifcontext,
@@ -237,28 +236,11 @@ def gen_register_command(name: str,
     return ret
 
 
-def gen_registry(registry: str, prefix: str) -> str:
-    ret = mcgen('''
-
-void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds)
-{
-    QTAILQ_INIT(cmds);
-
-''',
-                c_prefix=c_name(prefix, protect=False))
-    ret += registry
-    ret += mcgen('''
-}
-''')
-    return ret
-
-
 class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
     def __init__(self, prefix: str):
         super().__init__(
             prefix, 'qapi-commands',
             ' * Schema-defined QAPI/QMP commands', None, __doc__)
-        self._regy = QAPIGenCCode(None)
         self._visited_ret_types: Dict[QAPIGenC, Set[QAPISchemaType]] = {}
 
     def _begin_user_module(self, name: str) -> None:
@@ -285,7 +267,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
 ''',
                              types=types))
 
-    def visit_end(self) -> None:
+    def visit_begin(self, schema) -> None:
         self._add_module('./init', ' * QAPI Commands initialization')
         self._genh.add(mcgen('''
 #include "qapi/qmp/dispatch.h"
@@ -293,13 +275,24 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
 void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
 ''',
                              c_prefix=c_name(self._prefix, protect=False)))
-        self._genc.preamble_add(mcgen('''
+        self._genc.add(mcgen('''
 #include "qemu/osdep.h"
 #include "%(prefix)sqapi-commands.h"
 #include "%(prefix)sqapi-init-commands.h"
+
+void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds)
+{
+    QTAILQ_INIT(cmds);
+
 ''',
-                                      prefix=self._prefix))
-        self._genc.add(gen_registry(self._regy.get_content(), self._prefix))
+                             prefix=self._prefix,
+                             c_prefix=c_name(self._prefix, protect=False)))
+
+    def visit_end(self) -> None:
+        with self._temp_module('./init'):
+            self._genc.add(mcgen('''
+}
+'''))
 
     def visit_command(self,
                       name: str,
@@ -324,15 +317,17 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds);
         if ret_type and ret_type not in self._visited_ret_types[self._genc]:
             self._visited_ret_types[self._genc].add(ret_type)
             with ifcontext(ret_type.ifcond,
-                           self._genh, self._genc, self._regy):
+                           self._genh, self._genc):
                 self._genc.add(gen_marshal_output(ret_type))
-        with ifcontext(ifcond, self._genh, self._genc, self._regy):
+        with ifcontext(ifcond, self._genh, self._genc):
             self._genh.add(gen_command_decl(name, arg_type, boxed, ret_type))
             self._genh.add(gen_marshal_decl(name))
             self._genc.add(gen_marshal(name, arg_type, boxed, ret_type))
-            self._regy.add(gen_register_command(name, success_response,
-                                                allow_oob, allow_preconfig,
-                                                coroutine))
+        with self._temp_module('./init'):
+            with ifcontext(ifcond, self._genh, self._genc):
+                self._genc.add(gen_register_command(name, success_response,
+                                                    allow_oob, allow_preconfig,
+                                                    coroutine))
 
 
 def gen_commands(schema: QAPISchema,
-- 
2.26.2



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 11/11] qapi/gen: Drop support for QAPIGen without a file name
  2020-12-18 20:53 [PATCH 00/11] Drop support for QAPIGen without a file name Markus Armbruster
                   ` (9 preceding siblings ...)
  2020-12-18 20:54 ` [PATCH 10/11] qapi/commands: Simplify command registry generation Markus Armbruster
@ 2020-12-18 20:54 ` Markus Armbruster
  10 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2020-12-18 20:54 UTC (permalink / raw)
  To: qemu-devel; +Cc: marcandre.lureau, jsnow, ehabkost, crosa

The previous commit removed the only user of QAPIGen(None).  Tighten
the type hint.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 scripts/qapi/gen.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
index cb00229f5d..8716689450 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -37,7 +37,7 @@ from .source import QAPISourceInfo
 
 
 class QAPIGen:
-    def __init__(self, fname: Optional[str]):
+    def __init__(self, fname: str):
         self.fname = fname
         self._preamble = ''
         self._body = ''
-- 
2.26.2



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 09/11] qapi/gen: Support for switching to another module temporarily
  2020-12-18 20:54 ` [PATCH 09/11] qapi/gen: Support for switching to another module temporarily Markus Armbruster
@ 2021-01-15  2:07   ` John Snow
  0 siblings, 0 replies; 14+ messages in thread
From: John Snow @ 2021-01-15  2:07 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: marcandre.lureau, ehabkost, crosa

On 12/18/20 3:54 PM, Markus Armbruster wrote:
> +    @contextmanager
> +    def _temp_module(self, name: str) -> ContextManager[None]:

Doesn't quite typecheck; we want Iterator[None] -- I think we're typing 
the function that is yet-to-be-decorated -- mypy will handle typing the 
resulting function.

--js



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 10/11] qapi/commands: Simplify command registry generation
  2020-12-18 20:54 ` [PATCH 10/11] qapi/commands: Simplify command registry generation Markus Armbruster
@ 2021-01-15  2:15   ` John Snow
  0 siblings, 0 replies; 14+ messages in thread
From: John Snow @ 2021-01-15  2:15 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: marcandre.lureau, ehabkost, crosa

On 12/18/20 3:54 PM, Markus Armbruster wrote:
> QAPISchemaGenCommandVisitor.visit_command() needs to generate the
> marshalling function into the current module, and also generate its
> registration into the ./init system module.  The latter is done
> somewhat awkwardly: .__init__() creates a QAPIGenCCode that will not
> be written out, each .visit_command() adds its registration to it, and
> .visit_end() copies its contents into the ./init module it creates.
> 
> Instead provide the means to temporarily switch to another module.
> Create the ./init module in .visit_begin(), and generate its initial
> part.  Add registrations to it in .visit_command().  Finish it in
> .visit_end().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   scripts/qapi/commands.py | 49 ++++++++++++++++++----------------------
>   1 file changed, 22 insertions(+), 27 deletions(-)
> 
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index 4911166339..396485cc1a 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -23,7 +23,6 @@ from typing import (
>   from .common import c_name, mcgen
>   from .gen import (
>       QAPIGenC,
> -    QAPIGenCCode,
>       QAPISchemaModularCVisitor,
>       build_params,
>       ifcontext,
> @@ -237,28 +236,11 @@ def gen_register_command(name: str,
>       return ret
>   
>   
> -def gen_registry(registry: str, prefix: str) -> str:
> -    ret = mcgen('''
> -
> -void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds)
> -{
> -    QTAILQ_INIT(cmds);
> -
> -''',
> -                c_prefix=c_name(prefix, protect=False))
> -    ret += registry
> -    ret += mcgen('''
> -}
> -''')
> -    return ret
> -
> -
>   class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
>       def __init__(self, prefix: str):
>           super().__init__(
>               prefix, 'qapi-commands',
>               ' * Schema-defined QAPI/QMP commands', None, __doc__)
> -        self._regy = QAPIGenCCode(None)
>           self._visited_ret_types: Dict[QAPIGenC, Set[QAPISchemaType]] = {}
>   
>       def _begin_user_module(self, name: str) -> None:
> @@ -285,7 +267,7 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
>   ''',
>                                types=types))
>   
> -    def visit_end(self) -> None:
> +    def visit_begin(self, schema) -> None:

visit_begin(self, schema: QAPISchema) -> None: ...

:~)



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2021-01-15  2:17 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-18 20:53 [PATCH 00/11] Drop support for QAPIGen without a file name Markus Armbruster
2020-12-18 20:53 ` [PATCH 01/11] qapi/commands: assert arg_type is not None Markus Armbruster
2020-12-18 20:53 ` [PATCH 02/11] qapi/events: fix visit_event typing Markus Armbruster
2020-12-18 20:53 ` [PATCH 03/11] qapi/main: handle theoretical None-return from re.match() Markus Armbruster
2020-12-18 20:54 ` [PATCH 04/11] qapi/gen: assert that _start_if is not None in _wrap_ifcond Markus Armbruster
2020-12-18 20:54 ` [PATCH 05/11] qapi/gen: use './builtin' for the built-in module name Markus Armbruster
2020-12-18 20:54 ` [PATCH 06/11] qapi/gen: write _genc/_genh access shims Markus Armbruster
2020-12-18 20:54 ` [PATCH 07/11] qapi/gen: Replace ._begin_system_module() Markus Armbruster
2020-12-18 20:54 ` [PATCH 08/11] qapi/gen: Expose a single module name space Markus Armbruster
2020-12-18 20:54 ` [PATCH 09/11] qapi/gen: Support for switching to another module temporarily Markus Armbruster
2021-01-15  2:07   ` John Snow
2020-12-18 20:54 ` [PATCH 10/11] qapi/commands: Simplify command registry generation Markus Armbruster
2021-01-15  2:15   ` John Snow
2020-12-18 20:54 ` [PATCH 11/11] qapi/gen: Drop support for QAPIGen without a file name Markus Armbruster

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.