[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 06/22] qapi/parser: assert get_expr returns object in outer l
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 06/22] qapi/parser: assert get_expr returns object in outer loop |
Date: |
Sun, 25 Apr 2021 09:23:33 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> get_expr can return many things, depending on where it is used. In the
> outer parsing loop, we expect and require it to return a dict.
>
> (It's (maybe) a bit involved to teach mypy that when nested is False,
> this is already always True. I'll look into it later, maybe.)
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
> scripts/qapi/parser.py | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index c75434e75a5..6b443b1247e 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -78,6 +78,8 @@ def _parse(self):
> continue
>
> expr = self.get_expr(False)
> + assert isinstance(expr, dict) # Guaranteed when nested=False
> +
> if 'include' in expr:
> self.reject_expr_doc(cur_doc)
> if len(expr) != 1:
> @@ -278,6 +280,7 @@ def get_values(self):
> self.accept()
>
> def get_expr(self, nested):
> + # TODO: Teach mypy that nested=False means the retval is a Dict.
> if self.tok != '{' and not nested:
> raise QAPIParseError(self, "expected '{'")
> if self.tok == '{':
The better place to assert a post condition would be ...
self.accept()
expr = self.get_members()
elif self.tok == '[':
self.accept()
expr = self.get_values()
elif self.tok in "'tf":
expr = self.val
self.accept()
else:
raise QAPIParseError(
self, "expected '{', '[', string, or boolean")
... here.
return expr
But then it may not help mypy over the hump, which is the whole point of
the patch.
Alternative ways to skin this cat:
* Split get_object() off get_expr().
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index ca5e8e18e0..c79b3c7d08 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -262,9 +262,12 @@ def get_values(self):
raise QAPIParseError(self, "expected ',' or ']'")
self.accept()
- def get_expr(self, nested):
- if self.tok != '{' and not nested:
+ def get_object(self):
+ if self.tok != '{':
raise QAPIParseError(self, "expected '{'")
+ return self.get_expr()
+
+ def get_expr(self):
if self.tok == '{':
self.accept()
expr = self.get_members()
* Shift "top-level expression must be dict" up:
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index ca5e8e18e0..ee8cbf3531 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -68,7 +68,10 @@ def __init__(self, fname, previously_included=None,
incl_info=None):
self.docs.append(cur_doc)
continue
- expr = self.get_expr(False)
+ expr = self.get_expr()
+ if not isinstance(expr, OrderedDict):
+ raise QAPISemError(
+ info, "top-level expression must be an object")
if 'include' in expr:
self.reject_expr_doc(cur_doc)
if len(expr) != 1:
@@ -262,9 +265,7 @@ def get_values(self):
raise QAPIParseError(self, "expected ',' or ']'")
self.accept()
- def get_expr(self, nested):
- if self.tok != '{' and not nested:
- raise QAPIParseError(self, "expected '{'")
+ def get_expr(self):
if self.tok == '{':
self.accept()
expr = self.get_members()
* Shift it further, into expr.py:
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 496f7e0333..0a83c493a0 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -600,7 +600,10 @@ def check_exprs(exprs: List[_JSONObject]) ->
List[_JSONObject]:
"""
for expr_elem in exprs:
# Expression
- assert isinstance(expr_elem['expr'], dict)
+ if not isinstance(expr_elem['expr'], dict):
+ raise QAPISemError(
+ info, "top-level expression must be an object")
+
for key in expr_elem['expr'].keys():
assert isinstance(key, str)
expr: _JSONObject = expr_elem['expr']
Shifting it up would be closer to qapi-code-gen.txt than what we have
now.
All observations, no demands.
- Re: [PATCH 03/22] qapi/source: Remove line number from QAPISourceInfo initializer, (continued)
[PATCH 01/22] qapi/parser: Don't try to handle file errors, John Snow, 2021/04/21
[PATCH 04/22] qapi/parser: factor parsing routine into method, John Snow, 2021/04/21
[PATCH 08/22] qapi/parser: Use @staticmethod where appropriate, John Snow, 2021/04/21
[PATCH 06/22] qapi/parser: assert get_expr returns object in outer loop, John Snow, 2021/04/21
- Re: [PATCH 06/22] qapi/parser: assert get_expr returns object in outer loop,
Markus Armbruster <=
[PATCH 09/22] qapi: add match_nofail helper, John Snow, 2021/04/21
[PATCH 05/22] qapi/parser: Assert lexer value is a string, John Snow, 2021/04/21
[PATCH 07/22] qapi/parser: assert object keys are strings, John Snow, 2021/04/21