* [PATCH 0/5] qapi: Another round of minor fixes and cleanups
@ 2021-09-08 4:54 Markus Armbruster
2021-09-08 4:54 ` [PATCH 1/5] qapi: Fix a botched type annotation Markus Armbruster
` (6 more replies)
0 siblings, 7 replies; 10+ messages in thread
From: Markus Armbruster @ 2021-09-08 4:54 UTC (permalink / raw)
To: qemu-devel; +Cc: marcandre.lureau, jsnow, michael.roth
Markus Armbruster (5):
qapi: Fix a botched type annotation
qapi: Drop Indentation.__bool__()
qapi: Bury some unused code in class Indentation
tests/qapi-schema: Cover 'not' condition with empty argument
qapi: Fix bogus error for 'if': { 'not': '' }
scripts/qapi/common.py | 19 ++++++-------------
scripts/qapi/expr.py | 21 +++++++++++++--------
tests/qapi-schema/bad-if-not.err | 2 ++
tests/qapi-schema/bad-if-not.json | 3 +++
tests/qapi-schema/bad-if-not.out | 0
tests/qapi-schema/meson.build | 1 +
6 files changed, 25 insertions(+), 21 deletions(-)
create mode 100644 tests/qapi-schema/bad-if-not.err
create mode 100644 tests/qapi-schema/bad-if-not.json
create mode 100644 tests/qapi-schema/bad-if-not.out
--
2.31.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/5] qapi: Fix a botched type annotation
2021-09-08 4:54 [PATCH 0/5] qapi: Another round of minor fixes and cleanups Markus Armbruster
@ 2021-09-08 4:54 ` Markus Armbruster
2021-09-08 4:54 ` [PATCH 2/5] qapi: Drop Indentation.__bool__() Markus Armbruster
` (5 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2021-09-08 4:54 UTC (permalink / raw)
To: qemu-devel; +Cc: marcandre.lureau, jsnow, michael.roth
Mypy is unhappy:
$ mypy --config-file=scripts/qapi/mypy.ini `git-ls-files scripts/qapi/\*py`
scripts/qapi/common.py:208: error: Function is missing a return type annotation
scripts/qapi/common.py:227: error: Returning Any from function declared to return "str"
Messed up in commit ccea6a8637 "qapi: Factor common recursion out of
cgen_ifcond(), docgen_ifcond()". Tidy up.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi/common.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 5f8f76e5b2..c4d11b9637 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -205,7 +205,8 @@ def gen_ifcond(ifcond: Optional[Union[str, Dict[str, Any]]],
cond_fmt: str, not_fmt: str,
all_operator: str, any_operator: str) -> str:
- def do_gen(ifcond: Union[str, Dict[str, Any]], need_parens: bool):
+ def do_gen(ifcond: Union[str, Dict[str, Any]],
+ need_parens: bool) -> str:
if isinstance(ifcond, str):
return cond_fmt % ifcond
assert isinstance(ifcond, dict) and len(ifcond) == 1
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] qapi: Drop Indentation.__bool__()
2021-09-08 4:54 [PATCH 0/5] qapi: Another round of minor fixes and cleanups Markus Armbruster
2021-09-08 4:54 ` [PATCH 1/5] qapi: Fix a botched type annotation Markus Armbruster
@ 2021-09-08 4:54 ` Markus Armbruster
2021-09-13 18:29 ` Eric Blake
2021-09-08 4:54 ` [PATCH 3/5] qapi: Bury some unused code in class Indentation Markus Armbruster
` (4 subsequent siblings)
6 siblings, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2021-09-08 4:54 UTC (permalink / raw)
To: qemu-devel; +Cc: marcandre.lureau, jsnow, michael.roth
Intentation.__bool__() is not worth its keep: it has just one user,
which can just as well check .__str__() instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi/common.py | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index c4d11b9637..1d62c27fb7 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -142,10 +142,6 @@ def __str__(self) -> str:
"""Return the current indentation as a string of spaces."""
return ' ' * self._level
- def __bool__(self) -> bool:
- """True when there is a non-zero indentation."""
- return bool(self._level)
-
def increase(self, amount: int = 4) -> None:
"""Increase the indentation level by ``amount``, default 4."""
self._level += amount
@@ -169,8 +165,9 @@ def cgen(code: str, **kwds: object) -> str:
Obey `indent`, and strip `EATSPACE`.
"""
raw = code % kwds
- if indent:
- raw = re.sub(r'^(?!(#|$))', str(indent), raw, flags=re.MULTILINE)
+ pfx = str(indent)
+ if pfx:
+ raw = re.sub(r'^(?!(#|$))', pfx, raw, flags=re.MULTILINE)
return re.sub(re.escape(EATSPACE) + r' *', '', raw)
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 3/5] qapi: Bury some unused code in class Indentation
2021-09-08 4:54 [PATCH 0/5] qapi: Another round of minor fixes and cleanups Markus Armbruster
2021-09-08 4:54 ` [PATCH 1/5] qapi: Fix a botched type annotation Markus Armbruster
2021-09-08 4:54 ` [PATCH 2/5] qapi: Drop Indentation.__bool__() Markus Armbruster
@ 2021-09-08 4:54 ` Markus Armbruster
2021-09-08 4:54 ` [PATCH 4/5] tests/qapi-schema: Cover 'not' condition with empty argument Markus Armbruster
` (3 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2021-09-08 4:54 UTC (permalink / raw)
To: qemu-devel; +Cc: marcandre.lureau, jsnow, michael.roth
.__int__() has never been used. Drop it.
.decrease() raises ArithmeticError when asked to decrease indentation
level below zero. Nothing catches it. It's a programming error.
Dumb down to assert.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi/common.py | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/scripts/qapi/common.py b/scripts/qapi/common.py
index 1d62c27fb7..489273574a 100644
--- a/scripts/qapi/common.py
+++ b/scripts/qapi/common.py
@@ -132,9 +132,6 @@ class Indentation:
def __init__(self, initial: int = 0) -> None:
self._level = initial
- def __int__(self) -> int:
- return self._level
-
def __repr__(self) -> str:
return "{}({:d})".format(type(self).__name__, self._level)
@@ -148,9 +145,7 @@ def increase(self, amount: int = 4) -> None:
def decrease(self, amount: int = 4) -> None:
"""Decrease the indentation level by ``amount``, default 4."""
- if self._level < amount:
- raise ArithmeticError(
- f"Can't remove {amount:d} spaces from {self!r}")
+ assert amount <= self._level
self._level -= amount
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] tests/qapi-schema: Cover 'not' condition with empty argument
2021-09-08 4:54 [PATCH 0/5] qapi: Another round of minor fixes and cleanups Markus Armbruster
` (2 preceding siblings ...)
2021-09-08 4:54 ` [PATCH 3/5] qapi: Bury some unused code in class Indentation Markus Armbruster
@ 2021-09-08 4:54 ` Markus Armbruster
2021-09-08 4:54 ` [PATCH 5/5] qapi: Fix bogus error for 'if': { 'not': '' } Markus Armbruster
` (2 subsequent siblings)
6 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2021-09-08 4:54 UTC (permalink / raw)
To: qemu-devel; +Cc: marcandre.lureau, jsnow, michael.roth
We flag this, but the error message is bogus:
bad-if-not.json:2: 'if' condition [] of struct is useless
The next commit will fix it.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
tests/qapi-schema/bad-if-not.err | 2 ++
tests/qapi-schema/bad-if-not.json | 3 +++
tests/qapi-schema/bad-if-not.out | 0
tests/qapi-schema/meson.build | 1 +
4 files changed, 6 insertions(+)
create mode 100644 tests/qapi-schema/bad-if-not.err
create mode 100644 tests/qapi-schema/bad-if-not.json
create mode 100644 tests/qapi-schema/bad-if-not.out
diff --git a/tests/qapi-schema/bad-if-not.err b/tests/qapi-schema/bad-if-not.err
new file mode 100644
index 0000000000..b3acdd679a
--- /dev/null
+++ b/tests/qapi-schema/bad-if-not.err
@@ -0,0 +1,2 @@
+bad-if-not.json: In struct 'TestIfStruct':
+bad-if-not.json:2: 'if' condition [] of struct is useless
diff --git a/tests/qapi-schema/bad-if-not.json b/tests/qapi-schema/bad-if-not.json
new file mode 100644
index 0000000000..9fdaacc47b
--- /dev/null
+++ b/tests/qapi-schema/bad-if-not.json
@@ -0,0 +1,3 @@
+# check 'if not' with empy argument
+{ 'struct': 'TestIfStruct', 'data': { 'foo': 'int' },
+ 'if': { 'not': '' } }
diff --git a/tests/qapi-schema/bad-if-not.out b/tests/qapi-schema/bad-if-not.out
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/tests/qapi-schema/meson.build b/tests/qapi-schema/meson.build
index 4697c070bc..6b2a4ce41a 100644
--- a/tests/qapi-schema/meson.build
+++ b/tests/qapi-schema/meson.build
@@ -43,6 +43,7 @@ schemas = [
'bad-if-key.json',
'bad-if-keys.json',
'bad-if-list.json',
+ 'bad-if-not.json',
'bad-type-bool.json',
'bad-type-dict.json',
'bad-type-int.json',
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] qapi: Fix bogus error for 'if': { 'not': '' }
2021-09-08 4:54 [PATCH 0/5] qapi: Another round of minor fixes and cleanups Markus Armbruster
` (3 preceding siblings ...)
2021-09-08 4:54 ` [PATCH 4/5] tests/qapi-schema: Cover 'not' condition with empty argument Markus Armbruster
@ 2021-09-08 4:54 ` Markus Armbruster
2021-09-08 13:27 ` Markus Armbruster
2021-09-08 6:48 ` [PATCH 0/5] qapi: Another round of minor fixes and cleanups Marc-André Lureau
2021-09-13 7:57 ` Markus Armbruster
6 siblings, 1 reply; 10+ messages in thread
From: Markus Armbruster @ 2021-09-08 4:54 UTC (permalink / raw)
To: qemu-devel; +Cc: marcandre.lureau, jsnow, michael.roth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi/expr.py | 21 +++++++++++++--------
tests/qapi-schema/bad-if-not.err | 2 +-
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index b62f0a3640..ad3732c7f0 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -293,17 +293,22 @@ def _check_if(cond: Union[str, object]) -> None:
info,
"'if' condition of %s has conflicting keys" % source)
- oper, operands = next(iter(cond.items()))
+ if 'not' in cond:
+ _check_if(cond['not'])
+ elif 'all' in cond:
+ _check_infix('all', cond['all'])
+ else:
+ _check_infix('any', cond['any'])
+
+ def _check_infix(operator: str, operands: object):
+ if not isinstance(operands, list):
+ raise QAPISemError(
+ info,
+ "'%s' condition of %s must be an array"
+ % (operator, source))
if not operands:
raise QAPISemError(
info, "'if' condition [] of %s is useless" % source)
-
- if oper == "not":
- _check_if(operands)
- return
- if oper in ("all", "any") and not isinstance(operands, list):
- raise QAPISemError(
- info, "'%s' condition of %s must be an array" % (oper, source))
for operand in operands:
_check_if(operand)
diff --git a/tests/qapi-schema/bad-if-not.err b/tests/qapi-schema/bad-if-not.err
index b3acdd679a..b33f5e16b8 100644
--- a/tests/qapi-schema/bad-if-not.err
+++ b/tests/qapi-schema/bad-if-not.err
@@ -1,2 +1,2 @@
bad-if-not.json: In struct 'TestIfStruct':
-bad-if-not.json:2: 'if' condition [] of struct is useless
+bad-if-not.json:2: 'if' condition '' of struct is not a valid identifier
--
2.31.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/5] qapi: Another round of minor fixes and cleanups
2021-09-08 4:54 [PATCH 0/5] qapi: Another round of minor fixes and cleanups Markus Armbruster
` (4 preceding siblings ...)
2021-09-08 4:54 ` [PATCH 5/5] qapi: Fix bogus error for 'if': { 'not': '' } Markus Armbruster
@ 2021-09-08 6:48 ` Marc-André Lureau
2021-09-13 7:57 ` Markus Armbruster
6 siblings, 0 replies; 10+ messages in thread
From: Marc-André Lureau @ 2021-09-08 6:48 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Michael Roth, John Snow, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 967 bytes --]
Hi
On Wed, Sep 8, 2021 at 8:54 AM Markus Armbruster <armbru@redhat.com> wrote:
> Markus Armbruster (5):
> qapi: Fix a botched type annotation
> qapi: Drop Indentation.__bool__()
> qapi: Bury some unused code in class Indentation
> tests/qapi-schema: Cover 'not' condition with empty argument
> qapi: Fix bogus error for 'if': { 'not': '' }
>
> scripts/qapi/common.py | 19 ++++++-------------
> scripts/qapi/expr.py | 21 +++++++++++++--------
> tests/qapi-schema/bad-if-not.err | 2 ++
> tests/qapi-schema/bad-if-not.json | 3 +++
> tests/qapi-schema/bad-if-not.out | 0
> tests/qapi-schema/meson.build | 1 +
> 6 files changed, 25 insertions(+), 21 deletions(-)
> create mode 100644 tests/qapi-schema/bad-if-not.err
> create mode 100644 tests/qapi-schema/bad-if-not.json
> create mode 100644 tests/qapi-schema/bad-if-not.out
>
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[-- Attachment #2: Type: text/html, Size: 1451 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 5/5] qapi: Fix bogus error for 'if': { 'not': '' }
2021-09-08 4:54 ` [PATCH 5/5] qapi: Fix bogus error for 'if': { 'not': '' } Markus Armbruster
@ 2021-09-08 13:27 ` Markus Armbruster
0 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2021-09-08 13:27 UTC (permalink / raw)
To: qemu-devel; +Cc: marcandre.lureau, jsnow, michael.roth
Markus Armbruster <armbru@redhat.com> writes:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> scripts/qapi/expr.py | 21 +++++++++++++--------
> tests/qapi-schema/bad-if-not.err | 2 +-
> 2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index b62f0a3640..ad3732c7f0 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -293,17 +293,22 @@ def _check_if(cond: Union[str, object]) -> None:
> info,
> "'if' condition of %s has conflicting keys" % source)
>
> - oper, operands = next(iter(cond.items()))
> + if 'not' in cond:
> + _check_if(cond['not'])
> + elif 'all' in cond:
> + _check_infix('all', cond['all'])
> + else:
> + _check_infix('any', cond['any'])
> +
> + def _check_infix(operator: str, operands: object):
> + if not isinstance(operands, list):
> + raise QAPISemError(
> + info,
> + "'%s' condition of %s must be an array"
> + % (operator, source))
> if not operands:
> raise QAPISemError(
> info, "'if' condition [] of %s is useless" % source)
> -
> - if oper == "not":
> - _check_if(operands)
> - return
> - if oper in ("all", "any") and not isinstance(operands, list):
> - raise QAPISemError(
> - info, "'%s' condition of %s must be an array" % (oper, source))
> for operand in operands:
> _check_if(operand)
>
> diff --git a/tests/qapi-schema/bad-if-not.err b/tests/qapi-schema/bad-if-not.err
> index b3acdd679a..b33f5e16b8 100644
> --- a/tests/qapi-schema/bad-if-not.err
> +++ b/tests/qapi-schema/bad-if-not.err
> @@ -1,2 +1,2 @@
> bad-if-not.json: In struct 'TestIfStruct':
> -bad-if-not.json:2: 'if' condition [] of struct is useless
> +bad-if-not.json:2: 'if' condition '' of struct is not a valid identifier
Squashing in this fixup:
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index ad3732c7f0..90bde501b0 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -300,7 +300,7 @@ def _check_if(cond: Union[str, object]) -> None:
else:
_check_infix('any', cond['any'])
- def _check_infix(operator: str, operands: object):
+ def _check_infix(operator: str, operands: object) -> None:
if not isinstance(operands, list):
raise QAPISemError(
info,
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 0/5] qapi: Another round of minor fixes and cleanups
2021-09-08 4:54 [PATCH 0/5] qapi: Another round of minor fixes and cleanups Markus Armbruster
` (5 preceding siblings ...)
2021-09-08 6:48 ` [PATCH 0/5] qapi: Another round of minor fixes and cleanups Marc-André Lureau
@ 2021-09-13 7:57 ` Markus Armbruster
6 siblings, 0 replies; 10+ messages in thread
From: Markus Armbruster @ 2021-09-13 7:57 UTC (permalink / raw)
To: qemu-devel; +Cc: marcandre.lureau, jsnow, michael.roth
Queued.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] qapi: Drop Indentation.__bool__()
2021-09-08 4:54 ` [PATCH 2/5] qapi: Drop Indentation.__bool__() Markus Armbruster
@ 2021-09-13 18:29 ` Eric Blake
0 siblings, 0 replies; 10+ messages in thread
From: Eric Blake @ 2021-09-13 18:29 UTC (permalink / raw)
To: Markus Armbruster; +Cc: marcandre.lureau, jsnow, qemu-devel, michael.roth
On Wed, Sep 08, 2021 at 06:54:25AM +0200, Markus Armbruster wrote:
> Intentation.__bool__() is not worth its keep: it has just one user,
Indentation
> which can just as well check .__str__() instead.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> scripts/qapi/common.py | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-09-13 18:33 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-08 4:54 [PATCH 0/5] qapi: Another round of minor fixes and cleanups Markus Armbruster
2021-09-08 4:54 ` [PATCH 1/5] qapi: Fix a botched type annotation Markus Armbruster
2021-09-08 4:54 ` [PATCH 2/5] qapi: Drop Indentation.__bool__() Markus Armbruster
2021-09-13 18:29 ` Eric Blake
2021-09-08 4:54 ` [PATCH 3/5] qapi: Bury some unused code in class Indentation Markus Armbruster
2021-09-08 4:54 ` [PATCH 4/5] tests/qapi-schema: Cover 'not' condition with empty argument Markus Armbruster
2021-09-08 4:54 ` [PATCH 5/5] qapi: Fix bogus error for 'if': { 'not': '' } Markus Armbruster
2021-09-08 13:27 ` Markus Armbruster
2021-09-08 6:48 ` [PATCH 0/5] qapi: Another round of minor fixes and cleanups Marc-André Lureau
2021-09-13 7:57 ` Markus Armbruster
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).