[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 10/17] qapi/gen: Combine ._add_[user|system]_module
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v3 10/17] qapi/gen: Combine ._add_[user|system]_module |
Date: |
Wed, 20 Jan 2021 15:20:41 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> From: Markus Armbruster <armbru@redhat.com>
>
> 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
Is this still accurate?
> 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().
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> scripts/qapi/commands.py | 2 +-
> scripts/qapi/events.py | 2 +-
> scripts/qapi/gen.py | 20 +++++++-------------
> 3 files changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/scripts/qapi/commands.py b/scripts/qapi/commands.py
> index fc5fe27c472..49111663394 100644
> --- a/scripts/qapi/commands.py
> +++ b/scripts/qapi/commands.py
> @@ -286,7 +286,7 @@ def _begin_user_module(self, name: str) -> None:
> 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 26faa829898..079c666ec69 100644
> --- a/scripts/qapi/events.py
> +++ b/scripts/qapi/events.py
> @@ -191,7 +191,7 @@ def _begin_user_module(self, name: str) -> None:
> 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 55acd7e080d..b5505685e6e 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -272,22 +272,15 @@ def _module_filename(self, what: str, name: str) -> str:
> self._module_basename(what, name))
>
> def _add_module(self, name: str, blurb: str) -> None:
> + if QAPISchemaModule.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._genc, self._genh = self._module[name]
>
> - def _add_user_module(self, name: str, blurb: str) -> None:
> - assert QAPISchemaModule.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:
> - assert QAPISchemaModule.is_system_module(name)
> - self._add_module(name, blurb)
> -
> def write(self, output_dir: str, opt_builtins: bool = False) -> None:
> for name in self._module:
> if QAPISchemaModule.is_builtin_module(name) and not opt_builtins:
> @@ -303,9 +296,9 @@ def _begin_user_module(self, name: str) -> None:
> pass
>
> def visit_module(self, module: QAPISchemaModule) -> None:
> - if module.system_module:
> + if module.builtin_module:
Looks like you're fixing a slip-up in PATCH 06. If yes, squash. If no,
I'm confused.
> if self._builtin_blurb:
> - self._add_system_module(module.name, self._builtin_blurb)
> + self._add_module(module.name, self._builtin_blurb)
> self._begin_builtin_module()
> else:
> # The built-in module has not been created. No code may
> @@ -313,7 +306,8 @@ def visit_module(self, module: QAPISchemaModule) -> None:
> self._genc = None
> self._genh = None
> else:
> - self._add_user_module(module.name, self._user_blurb)
> + assert module.user_module, "Unexpected system module"
The string provides no value.
> + self._add_module(module.name, self._user_blurb)
> self._begin_user_module(module.name)
>
> def visit_include(self, name: str, info: QAPISourceInfo) -> None:
- Re: [PATCH v3 05/17] qapi: pass QAPISchemaModule to visit_module instead of str, (continued)
[PATCH v3 07/17] qapi/gen: Replace ._begin_system_module(), John Snow, 2021/01/19
[PATCH v3 11/17] qapi: centralize the built-in module name definition, John Snow, 2021/01/19
[PATCH v3 04/17] qapi/gen: inline _wrap_ifcond into end_if(), John Snow, 2021/01/19
[PATCH v3 12/17] qapi/gen: write _genc/_genh access shims, John Snow, 2021/01/19
[PATCH v3 16/17] qapi: type 'info' as Optional[QAPISourceInfo], John Snow, 2021/01/19
[PATCH v3 10/17] qapi/gen: Combine ._add_[user|system]_module, John Snow, 2021/01/19
- Re: [PATCH v3 10/17] qapi/gen: Combine ._add_[user|system]_module,
Markus Armbruster <=
[PATCH v3 02/17] qapi/events: fix visit_event typing, John Snow, 2021/01/19
[PATCH v3 09/17] qapi: use './builtin' as the built-in module name, John Snow, 2021/01/19
[PATCH v3 15/17] qapi/gen: Drop support for QAPIGen without a file name, John Snow, 2021/01/19
[PATCH v3 13/17] qapi/gen: Support for switching to another module temporarily, John Snow, 2021/01/19
[PATCH v3 14/17] qapi/commands: Simplify command registry generation, John Snow, 2021/01/19
[PATCH v3 17/17] qapi: enable strict-optional checks, John Snow, 2021/01/19