Re: [PATCH 13/37] qapi/common.py: add notational type hints
On 9/18/20 7:14 AM, Markus Armbruster wrote:
John Snow 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 writes:
Signed-off-by: John Snow
---
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.
Re: [PATCH 13/37] qapi/common.py: add notational type hints
John Snow 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.
>> John Snow writes:
>>
>>> Signed-off-by: John Snow
>>> ---
>>> 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.
>
>>> 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.
Re: [PATCH 13/37] qapi/common.py: add notational type hints
On 9/17/20 2:18 PM, John Snow wrote: Your type hint adds a restriction that wasn't there before. Is there a better way? I've settled on using the `object` type for now, which is slightly more restrictive than `Any`. `Any` and `object` both allow any type of argument, but `Any` effectively creates a gradually typed boundary in which you are implicitly casting to whatever typed boundary it enters next. `object` is an explicit cast to the most abstracted type; any further usage that is not supported the base object will be rejected. This is nice, because if someone adds a call at or below the mcgen level, mypy will certainly complain that they are assuming too much about the type -- and they would be! The rest of my series will now take this approach: prefer `object` to `Any`. --js
Re: [PATCH 13/37] qapi/common.py: add notational type hints
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.
John Snow writes:
Signed-off-by: John Snow
---
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.
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.)
--js
Re: [PATCH 13/37] qapi/common.py: add notational type hints
Question on the subject line: what makes a type hint notational?
John Snow writes:
> Signed-off-by: John Snow
> ---
> 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?
> 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.
> if code[0] == '\n':
> code = code[1:]
> return cgen(code, **kwds)
>
>
> -def c_fname(filename):
> +def c_fname(filename: str) -> str:
> return re.sub(r'[^A-Za-z0-9_]', '_', filename)
>
>
> -def guardstart(name):
> +def guardstart(name: str) -> str:
> return mcgen('''
> #ifndef %(name)s
> #define %(name)s
> @@ -160,7 +163,7 @@ def guardstart(name):
> name=c_fname(name).upper())
>
>
> -def guardend(name):
> +def guardend(name: str) -> str:
> return mcgen('''
>
> #endif /* %(name)s */
> @@ -168,7 +171,7 @@ def guardend(name):
> name=c_fname(name).upper())
>
>
> -def gen_if(ifcond):
> +def gen_if(ifcond: Sequence[str]) -> str:
> ret = ''
> for ifc in ifcond:
> ret += mcgen('''
> @@ -177,7 +180,7 @@ def gen_if(ifcond):
> return ret
>
>
> -def gen_endif(ifcond):
> +def gen_endif(ifcond: Sequence[str]) -> str:
> ret = ''
> for ifc in reversed(ifcond):
> ret += mcgen('''
> @@ -186,7 +189,9 @@ def gen_endif(ifcond):
> return ret
>
>
> -def build_params(arg_type, boxed, extra=None):
> +def build_params(arg_type,
> + boxed: bool,
> + extra: Optional[str] = None) -> str:
> ret = ''
> sep = ''
> if boxed:
[PATCH 13/37] qapi/common.py: add notational type hints
Signed-off-by: John Snow
---
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:
raw = code % kwds
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:
if code[0] == '\n':
code = code[1:]
return cgen(code, **kwds)
-def c_fname(filename):
+def c_fname(filename: str) -> str:
return re.sub(r'[^A-Za-z0-9_]', '_', filename)
-def guardstart(name):
+def guardstart(name: str) -> str:
return mcgen('''
#ifndef %(name)s
#define %(name)s
@@ -160,7 +163,7 @@ def guardstart(name):
name=c_fname(name).upper())
-def guardend(name):
+def guardend(name: str) -> str:
return mcgen('''
#endif /* %(name)s */
@@ -168,7 +171,7 @@ def guardend(name):
name=c_fname(name).upper())
-def gen_if(ifcond):
+def gen_if(ifcond: Sequence[str]) -> str:
ret = ''
for ifc in ifcond:
ret += mcgen('''
@@ -177,7 +180,7 @@ def gen_if(ifcond):
return ret
-def gen_endif(ifcond):
+def gen_endif(ifcond: Sequence[str]) -> str:
ret = ''
for ifc in reversed(ifcond):
ret += mcgen('''
@@ -186,7 +189,9 @@ def gen_endif(ifcond):
return ret
-def build_params(arg_type, boxed, extra=None):
+def build_params(arg_type,
+ boxed: bool,
+ extra: Optional[str] = None) -> str:
ret = ''
sep = ''
if boxed:
--
2.26.2
