On 9/25/20 9:00 AM, Markus Armbruster wrote:
Eduardo Habkost <ehabk...@redhat.com> writes:
On Tue, Sep 22, 2020 at 05:00:47PM -0400, John Snow wrote:
The edge case is that if the name is '', this expression returns a
string instead of a bool, which violates our declared type.
Signed-off-by: John Snow <js...@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 9898d513ae..cb2b2655c3 100644
--- a/scripts/qapi/gen.py
+++ b/scripts/qapi/gen.py
@@ -251,7 +251,7 @@ def __init__(self, prefix, what, user_blurb, builtin_blurb,
pydoc):
@staticmethod
def _is_user_module(name):
- return name and not name.startswith('./')
+ return name is not None and not name.startswith('./')
This changes behavior if name=='', and I guess this is OK, but
I'm not sure.
@name is either
(1) A module pathname relative to the main module
This is a module defined by the user.
(2) system module name, starting with './'
This is a named system module. We currently have two: './init' in
commands.py, and and './emit' in events.py.
(3) None
This is the (nameless) system module for built-in stuff. It
predates (2). Using './builtin' would probably be better now.
Yes please! This would help simplify Optional[str] to str in many
places, and removes doubt as to what "None" might imply.
Let's queue that idea up as a cleanup for after this typing series.
Note that (1) and (2) are disjoint: relative pathnames do not begin with
'./'.
name='' is not possible, because '' is not a valid pathname.
I miss documentation on `visit_module()`,
`visit_include()`, and `_is_user_module()`. I don't know what
`name` means here, and what is a "user module".
Valid complaints! The code is subtle in places, without helping its
readers along with comments or doc strings.
@staticmethod
def _is_builtin_module(name):
--
2.26.2
For now, I've done the simpler thing and wrapped the return in
bool(...), but we will be able to do much more cleanups if we eliminate
the possibility of "None" module names later. I'll get it all at once.
I'm adding it to my python TODO: https://gitlab.com/jsnow/qemu/-/issues/8
--js
