John Snow <js...@redhat.com> writes:
> This is an immutable, named, typed tuple. It's arguably nicer than
> arbitrary dicts for passing data around when using strict typing.
>
> This patch turns parser.exprs into a list of ParsedExpressions instead,
> and adjusts expr.py to match.
>
> This allows the types we specify in parser.py to be "remembered" all the
> way through expr.py and into schema.py. Several assertions around
> packing and unpacking this data can be removed as a result.
>
> Signed-off-by: John Snow <js...@redhat.com>
> ---
> scripts/qapi/expr.py | 29 +++++++++--------------------
> scripts/qapi/parser.py | 29 ++++++++++++++++++-----------
> scripts/qapi/schema.py | 6 +++---
> 3 files changed, 30 insertions(+), 34 deletions(-)
>
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index d01543006d8..293f830fe9d 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -44,7 +44,7 @@
>
> from .common import c_name
> from .error import QAPISemError
> -from .parser import QAPIDoc
> +from .parser import ParsedExpression
> from .source import QAPISourceInfo
>
>
> @@ -595,29 +595,17 @@ def check_event(expr: _JSONObject, info:
> QAPISourceInfo) -> None:
> check_type(args, info, "'data'", allow_dict=not boxed)
>
>
> -def check_expr(expr_elem: _JSONObject) -> None:
> +def check_expr(pexpr: ParsedExpression) -> None:
> """
> - Validate and normalize a parsed QAPI schema expression.
> + Validate and normalize a `ParsedExpression`.
Doc comment style question: what's our attitude towards repeating (parts
of) the function signature in its doc comment?
In general, I dislike comments that merely rephrase code as prose.
Untyped Python builds a habit of mentioning the types in the doc
comment. That's additional information. In typed Python, it's
rephrased information.
Keeping the old line would be just fine with me.
>
> - :param expr_elem: The parsed expression to normalize and validate.
> + :param pexpr: The parsed expression to normalize and validate.
>
> :raise QAPISemError: When this expression fails validation.
> - :return: None, ``expr`` is normalized in-place as needed.
> + :return: None, ``pexpr`` is normalized in-place as needed.
> """
> - # Expression
> - assert isinstance(expr_elem['expr'], dict)
> - for key in expr_elem['expr'].keys():
> - assert isinstance(key, str)
> - expr: _JSONObject = expr_elem['expr']
> -
> - # QAPISourceInfo
> - assert isinstance(expr_elem['info'], QAPISourceInfo)
> - info: QAPISourceInfo = expr_elem['info']
> -
> - # Optional[QAPIDoc]
> - tmp = expr_elem.get('doc')
> - assert tmp is None or isinstance(tmp, QAPIDoc)
> - doc: Optional[QAPIDoc] = tmp
> + expr = pexpr.expr
> + info = pexpr.info
>
> if 'include' in expr:
> return
> @@ -637,6 +625,7 @@ def check_expr(expr_elem: _JSONObject) -> None:
> info.set_defn(meta, name)
> check_defn_name_str(name, info, meta)
>
> + doc = pexpr.doc
> if doc:
> if doc.symbol != name:
> raise QAPISemError(
> @@ -688,7 +677,7 @@ def check_expr(expr_elem: _JSONObject) -> None:
> check_flags(expr, info)
>
>
> -def check_exprs(exprs: List[_JSONObject]) -> List[_JSONObject]:
> +def check_exprs(exprs: List[ParsedExpression]) -> List[ParsedExpression]:
> """
> Validate and normalize a list of parsed QAPI schema expressions.
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index 1b006cdc133..f897dffbfd4 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -21,6 +21,7 @@
> TYPE_CHECKING,
> Dict,
> List,
> + NamedTuple,
> Optional,
> Set,
> Union,
> @@ -48,6 +49,12 @@
> # several modules.
>
>
> +class ParsedExpression(NamedTuple):
> + expr: TopLevelExpr
> + info: QAPISourceInfo
> + doc: Optional['QAPIDoc']
> +
> +
> class QAPIParseError(QAPISourceError):
> """Error class for all QAPI schema parsing errors."""
> def __init__(self, parser: 'QAPISchemaParser', msg: str):
> @@ -100,7 +107,7 @@ def __init__(self,
> self.line_pos = 0
>
> # Parser output:
> - self.exprs: List[Dict[str, object]] = []
> + self.exprs: List[ParsedExpression] = []
> self.docs: List[QAPIDoc] = []
>
> # Showtime!
> @@ -147,8 +154,7 @@ def _parse(self) -> None:
> "value of 'include' must be a string")
> incl_fname = os.path.join(os.path.dirname(self._fname),
> include)
> - self.exprs.append({'expr': {'include': incl_fname},
> - 'info': info})
> + self._add_expr(OrderedDict({'include': incl_fname}), info)
> exprs_include = self._include(include, info, incl_fname,
> self._included)
> if exprs_include:
> @@ -165,17 +171,18 @@ def _parse(self) -> None:
> for name, value in pragma.items():
> self._pragma(name, value, info)
> else:
> - expr_elem = {'expr': expr,
> - 'info': info}
> - if cur_doc:
> - if not cur_doc.symbol:
> - raise QAPISemError(
> - cur_doc.info, "definition documentation
> required")
> - expr_elem['doc'] = cur_doc
> - self.exprs.append(expr_elem)
> + if cur_doc and not cur_doc.symbol:
> + raise QAPISemError(
> + cur_doc.info, "definition documentation required")
> + self._add_expr(expr, info, cur_doc)
> cur_doc = None
> self.reject_expr_doc(cur_doc)
>
> + def _add_expr(self, expr: TopLevelExpr,
> + info: QAPISourceInfo,
> + doc: Optional['QAPIDoc'] = None) -> None:
> + self.exprs.append(ParsedExpression(expr, info, doc))
> +
> @staticmethod
> def reject_expr_doc(doc: Optional['QAPIDoc']) -> None:
> if doc and doc.symbol:
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index cd8661125cd..89f8e60db36 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -1168,9 +1168,9 @@ def _def_event(self, expr, info, doc):
>
> def _def_exprs(self, exprs):
> for expr_elem in exprs:
Rename @expr_elem to @pexpr for consistency with check_expr()?
> - expr = expr_elem['expr']
> - info = expr_elem['info']
> - doc = expr_elem.get('doc')
> + expr = expr_elem.expr
> + info = expr_elem.info
> + doc = expr_elem.doc
> if 'enum' in expr:
> self._def_enum_type(expr, info, doc)
> elif 'struct' in expr:
