qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 13/37] qapi/common.py: add notational type hints


From: John Snow
Subject: Re: [PATCH 13/37] qapi/common.py: add notational type hints
Date: Fri, 18 Sep 2020 11:24:56 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.11.0

On 9/18/20 7:14 AM, Markus Armbruster wrote:
John Snow <jsnow@redhat.com> writes:

On 9/17/20 10:32 AM, Markus Armbruster wrote:
Question on the subject line: what makes a type hint notational?


My cover letter explains that every time I use this phrase, I mean to
state that "This patch adds exclusively type notations and makes no
functional changes to the runtime operation whatsoever."

i.e. notations-only.

By the time I get to PATCH 13, details explained in the cover letter
have been flushed from my memory.  Moreover, the cover letter won't make
it into Git.  Best to repeat them right in the commit message.  Perhaps:

     qapi/common; Add type hints

     Type hints do not change behavior.


ACK, done.

John Snow <jsnow@redhat.com> writes:

Signed-off-by: John Snow <jsnow@redhat.com>
---
   scripts/qapi/common.py | 27 ++++++++++++++++-----------
   1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 4c079755d3..af01348b35 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -12,6 +12,7 @@
   # See the COPYING file in the top-level directory.
     import re
+from typing import Optional, Union, Sequence
EATSPACE = '\033EATSPACE.'
@@ -22,7 +23,7 @@
   # ENUMName -> ENUM_NAME, EnumName1 -> ENUM_NAME1
   # ENUM_NAME -> ENUM_NAME, ENUM_NAME1 -> ENUM_NAME1, ENUM_Name2 -> ENUM_NAME2
   # ENUM24_Name -> ENUM24_NAME
-def camel_to_upper(value):
+def camel_to_upper(value: str) -> str:
       c_fun_str = c_name(value, False)
       if value.isupper():
           return c_fun_str
@@ -41,7 +42,9 @@ def camel_to_upper(value):
       return new_name.lstrip('_').upper()
-def c_enum_const(type_name, const_name, prefix=None):
+def c_enum_const(type_name: str,
+                 const_name: str,
+                 prefix: Optional[str] = None) -> str:
       if prefix is not None:
           type_name = prefix
       return camel_to_upper(type_name) + '_' + c_name(const_name, 
False).upper()
@@ -56,7 +59,7 @@ def c_enum_const(type_name, const_name, prefix=None):
   # into substrings of a generated C function name.
   # '__a.b_c' -> '__a_b_c', 'x-foo' -> 'x_foo'
   # protect=True: 'int' -> 'q_int'; protect=False: 'int' -> 'int'
-def c_name(name, protect=True):
+def c_name(name: str, protect: bool = True) -> str:
       # ANSI X3J11/88-090, 3.1.1
       c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
                        'default', 'do', 'double', 'else', 'enum', 'extern',
@@ -134,24 +137,24 @@ def pop(self, amount: int = 4) -> int:
     # Generate @code with @kwds interpolated.
   # Obey INDENT level, and strip EATSPACE.
-def cgen(code, **kwds):
+def cgen(code: str, **kwds: Union[str, int]) -> str:
Hmm.
The @kwds values can be anything, provided they match the conversion
specifiers in @code:

       raw = code % kwds
Your type hint adds a restriction that wasn't there before.
Is there a better way?


Maybe there are format-like type annotation tricks you can do to
enforce this, but I did not research them. I tried to resist
"improving" our usage of the old % formatter prematurely. I may do a
wholesale f-string conversion at some point, but not now, it's not
important.

In practice, we pass strings and integers. This typing *is*
artificially restrictive, though. We can declare the type to be "Any"
and allow the function to fail or succeed at runtime if you'd prefer.


I went with the 'object' type in new revisions.

       if INDENT:
           raw, _ = re.subn(r'^(?!(#|$))', str(INDENT), raw, flags=re.MULTILINE)
       return re.sub(re.escape(EATSPACE) + r' *', '', raw)
-def mcgen(code, **kwds):
+def mcgen(code: str, **kwds: Union[str, int]) -> str:
Likewise.


Unresearched idea: It's possible that we can subclass the
string.Formatter class and extend it to perform our special variable
replacements (chomping EATSPACE, etc.)

And *maybe* because it inherits from the standard formatter, we would
benefit from any analysis Mypy performs on such things.

Basically, replace mcgen/cgen with class CFormatter(string.Formatter).

(maybe. assume that none of what I just said will work or is feasible.)

Sounds worth exploring.  No need to do it now, of course.





reply via email to

[Prev in Thread] Current Thread [Next in Thread]