| Message ID | 20230209184758.1017863-8-jsnow@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: static typing conversion, pt5c | expand |
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 The comment you remove talks about _ExprValue, not QAPIExpression. > introspection data, remove this FIXME as no longer an action item. > > 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. As such, it's simply more > trouble than it's worth. I think there's a weightier counter-argument struggling to get out. As I explained under v2's cover letter, the two types represent things that are only superficially similar. _ExprValue is the obvious stupid abstract syntax tree for the QAPI schema language, with str and bool leaves (QAPI doesn't support floating-point numbers), OrderedDict and list inner nodes. It is used for parser output. QAPIExpression augments _ExprValue, adding a QAPISourceInfo (identifying the expression's source) and a QAPIDoc (the expressions documentation). It is used to represent QAPI top-level expressions. JSONValue is an annotated JSON abstract syntax tree. QAPIExpression and _ExprValue are related to it only insofar as the QAPI schema language is (rather loosely) patterned after JSON. Moreover, the two ASTs serve different purposes. QAPIExpression and _ExprValue represent *input*: they are produced by a parser and consumed by a semantic analyzer. JSONValue represents *output*: it is produced within a backend so we can separate the JSON text formatting aspect. Consolidating these two ASTs makes no sense. Suggest to work this argument into your commit message. > So, declare this "done enough for now". Agree. > Signed-off-by: John Snow <jsnow@redhat.com> > --- > scripts/qapi/parser.py | 4 ---- > 1 file changed, 4 deletions(-) > > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py > index c165bd3912c..b5afdd703e7 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. > - > - > # 3.6 workaround: can be removed when Python 3.7+ is our required version. > if TYPE_CHECKING: > _UserDict = UserDict[str, object]
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py index c165bd3912c..b5afdd703e7 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. - - # 3.6 workaround: can be removed when Python 3.7+ is our required version. if TYPE_CHECKING: _UserDict = UserDict[str, object]
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. 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. As such, it's simply more trouble than it's worth. So, declare this "done enough for now". Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/parser.py | 4 ---- 1 file changed, 4 deletions(-)