diff mbox series

[v4,6/6] qapi: remove JSON value FIXME

Message ID 20230215000011.1725012-7-jsnow@redhat.com (mailing list archive)
State New, archived
Headers show
Series qapi: static typing conversion, pt5c | expand

Commit Message

John Snow Feb. 15, 2023, midnight UTC
With the two major JSON-ish type hierarchies clarified for distinct
purposes; QAPIExpression for parsed expressions and JSONValue for
introspection data, remove this FIXME as no longer an action item.

A third JSON-y data type, _ExprValue, is not meant to represent JSON in
the abstract but rather only the possible legal return values from a
single function, get_expr(). It isn't appropriate to attempt to merge it
with either of the above two types.

In theory, it may be possible to define a completely agnostic
one-size-fits-all JSON type hierarchy that any other user could borrow -
in practice, it's tough to wrangle the differences between invariant,
covariant and contravariant types: input and output parameters demand
different properties of such a structure.

However, QAPIExpression serves to authoritatively type user input to the
QAPI parser, while JSONValue serves to authoritatively type qapi
generator *output* to be served back to client users at runtime via
QMP. The AST for these two types are different and cannot be wholly
merged into a unified syntax.

They could, in theory, share some JSON primitive definitions. In
practice, this is currently more trouble than it's worth with mypy's
current expressive power. As such, declare this "done enough for now".

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/parser.py | 4 ----
 1 file changed, 4 deletions(-)

Comments

Markus Armbruster Feb. 15, 2023, 11:13 a.m. UTC | #1
John Snow <jsnow@redhat.com> writes:

> With the two major JSON-ish type hierarchies clarified for distinct
> purposes; QAPIExpression for parsed expressions and JSONValue for
> introspection data, remove this FIXME as no longer an action item.
>
> A third JSON-y data type, _ExprValue, is not meant to represent JSON in
> the abstract but rather only the possible legal return values from a
> single function, get_expr(). It isn't appropriate to attempt to merge it
> with either of the above two types.
>
> In theory, it may be possible to define a completely agnostic
> one-size-fits-all JSON type hierarchy that any other user could borrow -
> in practice, it's tough to wrangle the differences between invariant,
> covariant and contravariant types: input and output parameters demand
> different properties of such a structure.
>
> However, QAPIExpression serves to authoritatively type user input to the
> QAPI parser, while JSONValue serves to authoritatively type qapi
> generator *output* to be served back to client users at runtime via
> QMP. The AST for these two types are different and cannot be wholly
> merged into a unified syntax.
>
> They could, in theory, share some JSON primitive definitions. In
> practice, this is currently more trouble than it's worth with mypy's
> current expressive power. As such, declare this "done enough for now".
>
> Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>
diff mbox series

Patch

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index d570086e1a9..878f90b4583 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -42,10 +42,6 @@ 
 _ExprValue = Union[List[object], Dict[str, object], str, bool]
 
 
-# FIXME: Consolidate and centralize definitions for _ExprValue and
-# JSONValue; currently scattered across several modules.
-
-
 class QAPIExpression(Dict[str, object]):
     # pylint: disable=too-few-public-methods
     def __init__(self,