* [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 related [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 related [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 related [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 related [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 related [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 related [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 related [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 related [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 related [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 related [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 related [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.