From a253b3eb9a194a0b2e8b55095ce5f28c2d5cfa11 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 1 Feb 2021 14:37:35 -0500 Subject: qapi/gen: inline _wrap_ifcond into end_if() We assert _start_if is not None in end_if, but that's opaque to mypy. By inlining _wrap_ifcond, that constraint becomes provable to mypy. Signed-off-by: John Snow Message-Id: <20210201193747.2169670-5-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/gen.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'scripts/qapi/gen.py') diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index b40f18e..3d81b90 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -130,15 +130,12 @@ 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 is not None self._body = _wrap_ifcond(self._start_if[0], self._start_if[1], self._body) self._preamble = _wrap_ifcond(self._start_if[0], self._start_if[2], self._preamble) + self._start_if = None def get_content(self) -> str: assert self._start_if is None -- cgit v1.1 From 98967c248c4c01085af2ff49ed3d378f79019902 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 1 Feb 2021 14:37:36 -0500 Subject: qapi: centralize is_[user|system|builtin]_module methods Define what a module is and define what kind of a module it is once and for all, in one place. Signed-off-by: John Snow Message-Id: <20210201193747.2169670-6-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/gen.py | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) (limited to 'scripts/qapi/gen.py') diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index 3d81b90..2aec6d3 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -31,7 +31,11 @@ from .common import ( guardstart, mcgen, ) -from .schema import QAPISchemaObjectType, QAPISchemaVisitor +from .schema import ( + QAPISchemaModule, + QAPISchemaObjectType, + QAPISchemaVisitor, +) from .source import QAPISourceInfo @@ -246,21 +250,14 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor): self._main_module: Optional[str] = None @staticmethod - def _is_user_module(name: Optional[str]) -> bool: - return bool(name and not name.startswith('./')) - - @staticmethod - def _is_builtin_module(name: Optional[str]) -> bool: - return not name - - def _module_dirname(self, name: Optional[str]) -> str: - if self._is_user_module(name): + def _module_dirname(name: Optional[str]) -> str: + if QAPISchemaModule.is_user_module(name): return os.path.dirname(name) return '' def _module_basename(self, what: str, name: Optional[str]) -> str: - ret = '' if self._is_builtin_module(name) else self._prefix - if self._is_user_module(name): + ret = '' if QAPISchemaModule.is_builtin_module(name) else self._prefix + if QAPISchemaModule.is_user_module(name): basename = os.path.basename(name) ret += what if name != self._main_module: @@ -282,7 +279,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor): self._genc, self._genh = self._module[name] def _add_user_module(self, name: str, blurb: str) -> None: - assert self._is_user_module(name) + assert QAPISchemaModule.is_user_module(name) if self._main_module is None: self._main_module = name self._add_module(name, blurb) @@ -292,7 +289,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor): 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: + if QAPISchemaModule.is_builtin_module(name) and not opt_builtins: continue (genc, genh) = self._module[name] genc.write(output_dir) -- cgit v1.1 From f3a705928a7b1825678ff510843702652bc15f1a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 1 Feb 2021 14:37:37 -0500 Subject: qapi/gen: Replace ._begin_system_module() QAPISchemaModularCVisitor._begin_system_module() is actually just for the builtin module. Rename it to ._begin_builtin_module() and drop its useless @name parameter. Clarify conditionals in visit_module to make this clear. Signed-off-by: Markus Armbruster Signed-off-by: John Snow Message-Id: <20210201193747.2169670-7-jsnow@redhat.com> --- scripts/qapi/gen.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'scripts/qapi/gen.py') diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index 2aec6d3..aaed78e 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -295,23 +295,24 @@ 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: pass def visit_module(self, name: Optional[str]) -> None: - if name is None: + if QAPISchemaModule.is_builtin_module(name): if self._builtin_blurb: - self._add_system_module(None, self._builtin_blurb) - self._begin_system_module(name) + self._add_system_module(name, self._builtin_blurb) + self._begin_builtin_module() else: # The built-in module has not been created. No code may # be generated. self._genc = None self._genh = None else: + assert QAPISchemaModule.is_user_module(name) self._add_user_module(name, self._user_blurb) self._begin_user_module(name) -- cgit v1.1 From 12893a8ea7163e871abae05b5a42cdce1ad45225 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 1 Feb 2021 14:37:38 -0500 Subject: qapi: use explicitly internal module names QAPISchemaModularCVisitor._add_system_module() prefixes './' to its name argument to make it a module name. Pass the module name instead. This will allow us to coalesce the methods to add modules later on. Signed-off-by: John Snow Message-Id: <20210201193747.2169670-8-jsnow@redhat.com> Reviewed-by: Markus Armbruster [Commit message reworded] Signed-off-by: Markus Armbruster --- scripts/qapi/gen.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'scripts/qapi/gen.py') diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index aaed78e..da9d4d2 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -285,7 +285,8 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor): self._add_module(name, blurb) def _add_system_module(self, name: Optional[str], blurb: str) -> None: - self._add_module(name and './' + name, blurb) + 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: -- cgit v1.1 From e2bbc4eaa7f0d8efb8f49705bae0fecd3356f417 Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 1 Feb 2021 14:37:39 -0500 Subject: qapi: use './builtin' as the built-in module name Use './builtin' as the built-in module name instead of None. Clarify the typing that this is now always a string. Signed-off-by: John Snow Message-Id: <20210201193747.2169670-9-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/gen.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'scripts/qapi/gen.py') diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index da9d4d2..9352d79 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -246,16 +246,16 @@ 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 _module_dirname(name: Optional[str]) -> str: + def _module_dirname(name: str) -> str: if QAPISchemaModule.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 QAPISchemaModule.is_builtin_module(name) else self._prefix if QAPISchemaModule.is_user_module(name): basename = os.path.basename(name) @@ -263,15 +263,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 QAPISchemaModule.is_system_module(name) + 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) @@ -284,7 +284,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor): self._main_module = name self._add_module(name, blurb) - def _add_system_module(self, name: Optional[str], blurb: str) -> None: + def _add_system_module(self, name: str, blurb: str) -> None: assert QAPISchemaModule.is_system_module(name) self._add_module(name, blurb) @@ -302,7 +302,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor): def _begin_user_module(self, name: str) -> None: pass - def visit_module(self, name: Optional[str]) -> None: + def visit_module(self, name: str) -> None: if QAPISchemaModule.is_builtin_module(name): if self._builtin_blurb: self._add_system_module(name, self._builtin_blurb) -- cgit v1.1 From 4ab0ff6da0828e6499d633a4801f4e6b6e96d21b Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 1 Feb 2021 14:37:40 -0500 Subject: qapi/gen: Combine ._add_[user|system]_module With callers to _add_system_module now explicitly using the './' prefix to indicate a system module, there is no longer any reason to have separate interfaces for adding system vs user modules; use a unified interface that differentiates based on the name. Signed-off-by: Markus Armbruster Signed-off-by: John Snow Message-Id: <20210201193747.2169670-10-jsnow@redhat.com> --- scripts/qapi/gen.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) (limited to 'scripts/qapi/gen.py') diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index 9352d79..8ded0f7 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -272,22 +272,15 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor): 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: @@ -305,7 +298,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor): def visit_module(self, name: str) -> None: if QAPISchemaModule.is_builtin_module(name): if self._builtin_blurb: - self._add_system_module(name, self._builtin_blurb) + self._add_module(name, self._builtin_blurb) self._begin_builtin_module() else: # The built-in module has not been created. No code may @@ -314,7 +307,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor): self._genh = None else: assert QAPISchemaModule.is_user_module(name) - 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: -- cgit v1.1 From fd9b1603843df86b430083b583157fe0c352901e Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 1 Feb 2021 14:37:42 -0500 Subject: qapi/gen: write _genc/_genh access shims 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 Message-Id: <20210201193747.2169670-12-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/gen.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'scripts/qapi/gen.py') diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index 8ded0f7..b2bb9d1 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -244,11 +244,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 _module_dirname(name: str) -> str: if QAPISchemaModule.is_user_module(name): @@ -279,7 +288,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 write(self, output_dir: str, opt_builtins: bool = False) -> None: for name in self._module: @@ -303,8 +312,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: assert QAPISchemaModule.is_user_module(name) self._add_module(name, self._user_blurb) -- cgit v1.1 From d921d27c1bac0765370a9b9b891f9f0429f4c7c3 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 1 Feb 2021 14:37:43 -0500 Subject: qapi/gen: Support switching to another module temporarily Signed-off-by: Markus Armbruster Signed-off-by: John Snow Message-Id: <20210201193747.2169670-13-jsnow@redhat.com> [Commit message tweaked] --- scripts/qapi/gen.py | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'scripts/qapi/gen.py') diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index b2bb9d1..a0a5df3 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -290,6 +290,13 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor): self._module[name] = (genc, genh) self._current_module = name + @contextmanager + def _temp_module(self, name: str) -> Iterator[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 QAPISchemaModule.is_builtin_module(name) and not opt_builtins: -- cgit v1.1 From cc0747f6b709c197b077cd313f37617fc80c3be1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Mon, 1 Feb 2021 14:37:45 -0500 Subject: qapi/gen: Drop support for QAPIGen without a file name The previous commit removed the only user of QAPIGen(None). Tighten the type hint. Signed-off-by: Markus Armbruster Signed-off-by: John Snow Message-Id: <20210201193747.2169670-15-jsnow@redhat.com> --- scripts/qapi/gen.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'scripts/qapi/gen.py') diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index a0a5df3..ac3d3e6 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -40,7 +40,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 = '' @@ -125,7 +125,7 @@ def build_params(arg_type: Optional[QAPISchemaObjectType], class QAPIGenCCode(QAPIGen): - def __init__(self, fname: Optional[str]): + def __init__(self, fname: str): super().__init__(fname) self._start_if: Optional[Tuple[List[str], str, str]] = None -- cgit v1.1 From 4a82e468e76f67901a7fcaeaf2d31904664658cc Mon Sep 17 00:00:00 2001 From: John Snow Date: Mon, 1 Feb 2021 14:37:46 -0500 Subject: qapi: type 'info' as Optional[QAPISourceInfo] For everything typed so far, type this parameter as Optional[QAPISourceInfo]. In the most generic case, QAPISchemaEntity's info field may be None to represent types that come from built-in definitions. Although some Entity types may not currently have any built-in definitions, it is not easily possible to constrain the type except on an ad-hoc basis using assertions. It's easier and simpler, then, to just say it's always an Optional type. Signed-off-by: John Snow Message-Id: <20210201193747.2169670-16-jsnow@redhat.com> Reviewed-by: Markus Armbruster Signed-off-by: Markus Armbruster --- scripts/qapi/gen.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'scripts/qapi/gen.py') diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py index ac3d3e6..63549cc 100644 --- a/scripts/qapi/gen.py +++ b/scripts/qapi/gen.py @@ -325,7 +325,7 @@ class QAPISchemaModularCVisitor(QAPISchemaVisitor): self._add_module(name, self._user_blurb) self._begin_user_module(name) - def visit_include(self, name: str, info: QAPISourceInfo) -> None: + def visit_include(self, name: str, info: Optional[QAPISourceInfo]) -> None: relname = os.path.relpath(self._module_filename(self._what, name), os.path.dirname(self._genh.fname)) self._genh.preamble_add(mcgen(''' -- cgit v1.1