|
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, SequenceEATSPACE = '\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 % kwdsYour 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.
[Prev in Thread] | Current Thread | [Next in Thread] |